From 65abe0d666ff21517477b8cde93d8071ef8f8cbf Mon Sep 17 00:00:00 2001 From: Matt Larraz Date: Thu, 14 Jan 2021 16:14:58 -0800 Subject: [PATCH] Deprecate 'master' in favor of 'primary' --- .rubocop_todo.yml | 12 +- CHANGELOG.md | 1 + README.md | 74 +++++------ .../makara_abstract_adapter.rb | 34 +++-- lib/makara/config_parser.rb | 58 +++++--- lib/makara/context.rb | 2 +- lib/makara/logging/subscriber.rb | 2 +- lib/makara/middleware.rb | 2 +- lib/makara/pool.rb | 4 +- lib/makara/proxy.rb | 89 +++++++------ ...ra_abstract_adapter_error_handling_spec.rb | 4 +- .../makara_abstract_adapter_spec.rb | 10 +- .../makara_mysql2_adapter_spec.rb | 20 +-- .../makara_postgis_adapter_spec.rb | 18 +-- .../makara_postgresql_adapter_spec.rb | 18 +-- spec/config_parser_spec.rb | 32 ++--- spec/connection_wrapper_spec.rb | 4 +- spec/context_spec.rb | 2 +- spec/middleware_spec.rb | 6 +- spec/pool_spec.rb | 18 +-- spec/proxy_spec.rb | 124 +++++++++--------- spec/strategies/priority_failover_spec.rb | 4 +- spec/strategies/shard_aware_spec.rb | 32 ++--- spec/support/helpers.rb | 8 +- spec/support/mock_objects.rb | 2 +- spec/support/mysql2_database.yml | 4 +- .../mysql2_database_with_custom_errors.yml | 4 +- spec/support/postgis_database.yml | 4 +- spec/support/postgresql_database.yml | 4 +- spec/support/proxy_extensions.rb | 6 +- 30 files changed, 326 insertions(+), 276 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 038e256c..884e104c 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2021-01-19 22:10:17 UTC using RuboCop version 1.8.1. +# on 2021-01-20 22:15:06 UTC using RuboCop version 1.8.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -71,7 +71,7 @@ Layout/FirstHashElementIndentation: - 'lib/makara/config_parser.rb' - 'spec/strategies/shard_aware_spec.rb' -# Offense count: 4 +# Offense count: 10 # Cop supports --auto-correct. # Configuration parameters: AllowMultipleStyles, EnforcedHashRocketStyle, EnforcedColonStyle, EnforcedLastArgumentHashStyle. # SupportedHashRocketStyles: key, separator, table @@ -243,7 +243,7 @@ Lint/UnusedBlockArgument: Exclude: - 'lib/makara/strategies/priority_failover.rb' -# Offense count: 28 +# Offense count: 25 # Cop supports --auto-correct. # Configuration parameters: AllowUnusedKeywordArguments, IgnoreEmptyMethods, IgnoreNotImplementedMethods. Lint/UnusedMethodArgument: @@ -283,7 +283,7 @@ Metrics/BlockNesting: # Offense count: 4 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 240 + Max: 253 # Offense count: 3 # Configuration parameters: IgnoredMethods. @@ -514,7 +514,7 @@ Style/NumericPredicate: - 'lib/makara/context.rb' - 'lib/makara/proxy.rb' -# Offense count: 2 +# Offense count: 3 # Configuration parameters: AllowedMethods. # AllowedMethods: respond_to_missing? Style/OptionalBooleanParameter: @@ -662,7 +662,7 @@ Style/TrivialAccessors: Style/WordArray: EnforcedStyle: brackets -# Offense count: 25 +# Offense count: 27 # Cop supports --auto-correct. # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https diff --git a/CHANGELOG.md b/CHANGELOG.md index 904665fe..89bc5739 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ All notable changes to this project will be documented in this file. ## Unreleased +- Deprecated the term `master` in favor of `primary` [#290](https://github.com/instacart/makara/pull/290) Matt Larraz - Deprecated the term `slave` in favor of `replica` [#286](https://github.com/instacart/makara/pull/286) Matt Larraz - Drop support for Ruby < 2.5 and ActiveRecord < 5.2 [#281](https://github.com/instacart/makara/pull/281) Matt Larraz diff --git a/README.md b/README.md index e202db5c..d09a3ab9 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ [![Code Climate](https://codeclimate.com/repos/526886a7f3ea00679b00cae6/badges/7905f7a000492a1078f7/gpa.png)](https://codeclimate.com/repos/526886a7f3ea00679b00cae6/feed) -Makara is generic master/replica proxy. It handles the heavy lifting of managing, choosing, blacklisting, and cycling through connections. It comes with an ActiveRecord database adapter implementation. +Makara is generic primary/replica proxy. It handles the heavy lifting of managing, choosing, blacklisting, and cycling through connections. It comes with an ActiveRecord database adapter implementation. ## Installation @@ -37,18 +37,18 @@ Next, you need to decide which methods are proxied and which methods should be s send_to_all :connect, :reconnect, :disconnect, :clear_cache ``` -Assuming you don't need to split requests between a master and a replica, you're done. If you do need to, implement the `needs_master?` method: +Assuming you don't need to split requests between a primary and a replica, you're done. If you do need to, implement the `needs_primary?` method: ```ruby # within MyAwesomeSqlProxy - def needs_master?(method_name, args) + def needs_primary?(method_name, args) return false if args.empty? sql = args.first sql !~ /^select/i end ``` -This implementation will send any request not like "SELECT..." to a master connection. There are more methods you can override and more control over blacklisting - check out the [makara database adapter](lib/active_record/connection_adapters/makara_abstract_adapter.rb) for examples of advanced usage. +This implementation will send any request not like "SELECT..." to a primary connection. There are more methods you can override and more control over blacklisting - check out the [makara database adapter](lib/active_record/connection_adapters/makara_abstract_adapter.rb) for examples of advanced usage. ### Config Parsing @@ -58,13 +58,13 @@ Makara comes with a config parser which will handle providing subconfigs to the Makara handles stickiness by keeping track of which proxies are stuck at any given moment. The context is basically a mapping of proxy ids to the timestamp until which they are stuck. -To handle persistence of context across requests in a Rack app, makara provides a middleware. It lays a cookie named `_mkra_stck` which contains the current context. If the next request is executed before the cookie expires, that given context will be used. If something occurs which naturally requires master on the second request, the context is updated and stored again. +To handle persistence of context across requests in a Rack app, makara provides a middleware. It lays a cookie named `_mkra_stck` which contains the current context. If the next request is executed before the cookie expires, that given context will be used. If something occurs which naturally requires the primary on the second request, the context is updated and stored again. #### Stickiness Impact -When `sticky:true`, once a query as been sent to master, all queries for the rest of the request will also be sent to master. In addition, the cookie described above will be set client side with an expiration defined by time at end of original request + `master_ttl`. As long as the cookie is valid, all requests will send queries to master. +When `sticky:true`, once a query as been sent to the primary, all queries for the rest of the request will also be sent to the primary. In addition, the cookie described above will be set client side with an expiration defined by time at end of original request + `primary_ttl`. As long as the cookie is valid, all requests will send queries to primary. -When `sticky:false`, only queries that need to go to master will go there. Subsequent read queries in the same request will go to replicas. +When `sticky:false`, only queries that need to go to the primary will go there. Subsequent read queries in the same request will go to replicas. #### Releasing stuck connections (clearing context) @@ -82,28 +82,28 @@ Makara::Context.release('redis') ... ``` -A context is local to the curent thread of execution. This will allow you to stick to master safely in a single thread +A context is local to the curent thread of execution. This will allow you to stick to the primary safely in a single thread in systems such as sidekiq, for instance. -#### Forcing Master +#### Forcing Primary -If you need to force master in your app then you can simply invoke stick_to_master! on your connection: +If you need to force the primary in your app then you can simply invoke stick_to_primary! on your connection: ```ruby persist = true # or false, it's true by default -proxy.stick_to_master!(persist) +proxy.stick_to_primary!(persist) ``` -It'll keep the proxy stuck to master for the current request, and if `persist = true` (default), it'll be also stored in the context for subsequent requests, keeping the proxy stuck up to the duration of `master_ttl` configured for the proxy. +It'll keep the proxy stuck to the primary for the current request, and if `persist = true` (default), it'll be also stored in the context for subsequent requests, keeping the proxy stuck up to the duration of `primary_ttl` configured for the proxy. #### Skipping the Stickiness -If you're using the `sticky: true` configuration and you find yourself in a situation where you need to write information through the proxy but you don't want the context to be stuck to master, you should use a `without_sticking` block: +If you're using the `sticky: true` configuration and you find yourself in a situation where you need to write information through the proxy but you don't want the context to be stuck to the primary, you should use a `without_sticking` block: ```ruby proxy.without_sticking do - # do my stuff that would normally cause the proxy to stick to master + # do my stuff that would normally cause the proxy to stick to the primary end ``` @@ -117,23 +117,23 @@ Makara::Logging::Logger.logger = ::Logger.new(STDOUT) ## ActiveRecord Database Adapter -So you've found yourself with an ActiveRecord-based project which is starting to get some traffic and you realize 95% of you DB load is from reads. Well you've come to the right spot. Makara is a great solution to break up that load not only between master and replica but potentially multiple masters and/or multiple replicas. +So you've found yourself with an ActiveRecord-based project which is starting to get some traffic and you realize 95% of you DB load is from reads. Well you've come to the right spot. Makara is a great solution to break up that load not only between primary and replica but potentially multiple primaries and/or multiple replicas. By creating a makara database adapter which simply acts as a proxy we avoid any major complexity surrounding specific database implementations. The makara adapter doesn't care if the underlying connection is mysql, postgresql, etc it simply cares about the sql string being executed. ### What goes where? -In general: Any `SELECT` statements will execute against your replica(s), anything else will go to master. +In general: Any `SELECT` statements will execute against your replica(s), anything else will go to the primary. There are some edge cases: * `SET` operations will be sent to all connections * Execution of specific methods such as `connect!`, `disconnect!`, and `clear_cache!` are invoked on all underlying connections -* Calls inside a transaction will always be sent to the master (otherwise changes from within the transaction could not be read back on most transaction isolation levels) -* Locking reads (e.g. `SELECT ... FOR UPDATE`) will always be sent to the master +* Calls inside a transaction will always be sent to the primary (otherwise changes from within the transaction could not be read back on most transaction isolation levels) +* Locking reads (e.g. `SELECT ... FOR UPDATE`) will always be sent to the primary ### Errors / blacklisting -Whenever a node fails an operation due to a connection issue, it is blacklisted for the amount of time specified in your database.yml. After that amount of time has passed, the connection will begin receiving queries again. If all replica nodes are blacklisted, the master connection will begin receiving read queries as if it were a replica. Once all nodes are blacklisted the error is raised to the application and all nodes are whitelisted. +Whenever a node fails an operation due to a connection issue, it is blacklisted for the amount of time specified in your database.yml. After that amount of time has passed, the connection will begin receiving queries again. If all replica nodes are blacklisted, the primary connection will begin receiving read queries as if it were a replica. Once all nodes are blacklisted the error is raised to the application and all nodes are whitelisted. ### Database.yml @@ -152,15 +152,15 @@ production: id: mysql # the following are default values blacklist_duration: 5 - master_ttl: 5 - master_strategy: round_robin + primary_ttl: 5 + primary_strategy: round_robin sticky: true # list your connections with the override values (they're merged into the top-level config) - # be sure to provide the role if master, role is assumed to be a replica if not provided + # be sure to provide the role if primary, role is assumed to be a replica if not provided connections: - - role: master - host: master.sql.host + - role: primary + host: primary.sql.host - role: replica host: replica1.sql.host - role: replica @@ -174,19 +174,19 @@ Following the adapter choice is all the standard configurations (host, port, ret The makara subconfig sets up the proxy with a few of its own options, then provides the connection list. The makara options are: * id - an identifier for the proxy, used for sticky behaviour and context. The default is to use a MD5 hash of the configuration contents, so if you are setting `sticky` to true, it's a good idea to also set an `id`. Otherwise any stuck connections will be cleared if the configuration changes (as the default MD5 hash id would change as well) * blacklist_duration - the number of seconds a node is blacklisted when a connection failure occurs -* disable_blacklist - do not blacklist node at any error, useful in case of one master +* disable_blacklist - do not blacklist node at any error, useful in case of one primary * sticky - if a node should be stuck to once it's used during a specific context -* master_ttl - how long the master context is persisted. generally, this needs to be longer than any replication lag -* master_strategy - use a different strategy for picking the "current" master node: `failover` will try to keep the same one until it is blacklisted. The default is `round_robin` which will cycle through available ones. +* primary_ttl - how long the primary context is persisted. generally, this needs to be longer than any replication lag +* primary_strategy - use a different strategy for picking the "current" primary node: `failover` will try to keep the same one until it is blacklisted. The default is `round_robin` which will cycle through available ones. * replica_strategy - use a different strategy for picking the "current" replica node: `failover` will try to keep the same one until it is blacklisted. The default is `round_robin` which will cycle through available ones. * connection_error_matchers - array of custom error matchers you want to be handled gracefully by Makara (as in, errors matching these regexes will result in blacklisting the connection as opposed to raising directly). -Connection definitions contain any extra node-specific configurations. If the node should behave as a master you must provide `role: master`. Any previous configurations can be overridden within a specific node's config. Nodes can also contain weights if you'd like to balance usage based on hardware specifications. Optionally, you can provide a name attribute which will be used in sql logging. +Connection definitions contain any extra node-specific configurations. If the node should behave as a primary you must provide `role: primary`. Any previous configurations can be overridden within a specific node's config. Nodes can also contain weights if you'd like to balance usage based on hardware specifications. Optionally, you can provide a name attribute which will be used in sql logging. ```yml connections: - - role: master - host: mymaster.sql.host + - role: primary + host: myprimary.sql.host blacklist_duration: 0 # implicit role: replica @@ -206,7 +206,7 @@ Connections may specify a `url` parameter in place of host, username, password, ```yml connections: - - role: master + - role: primary blacklist_duration: 0 url: 'mysql2://db_username:db_password@localhost:3306/db_name' ``` @@ -215,9 +215,9 @@ We recommend, if using environmental variables, to interpolate them via ERb. ```yml connections: - - role: master + - role: primary blacklist_duration: 0 - url: <%= ENV['DATABASE_URL_MASTER'] %> + url: <%= ENV['DATABASE_URL_PRIMARY'] %> - role: replica url: <%= ENV['DATABASE_URL_REPLICA'] %> ``` @@ -244,7 +244,7 @@ For more information on url parsing, as used in - ActiveRecord::ConnectionHandling::MergeAndResolveDefaultUrlConfig.new(url_config).resolve - 4.2 [ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(url).to_hash](https://github.com/rails/rails/blob/4-2-stable/activerecord/lib/active_record/connection_handling.rb#L60-L81) -- master +- primary [ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(url).to_hash](https://github.com/rails/rails/blob/97b980b4e61aea3cee429bdee4b2eae2329905cd/activerecord/lib/active_record/connection_handling.rb#L60-L81) @@ -269,15 +269,15 @@ You can provide strings or regexes. In the case of strings, if they start with ## Common Problems / Solutions -On occasion your app may deal with a situation where makara is not present during a write but a read should use master. In the generic proxy details above you are encouraged to use `stick_to_master!` to accomplish this. Here's an example: +On occasion your app may deal with a situation where makara is not present during a write but a read should use primary. In the generic proxy details above you are encouraged to use `stick_to_primary!` to accomplish this. Here's an example: ```ruby # some third party creates a resource in your db, replication may not have completed yet # ... # then your app is told to read the resource. def handle_request_after_third_party_record_creation - CreatedResourceClass.connection.stick_to_master! - CreatedResourceClass.find(params[:id]) # will go to master + CreatedResourceClass.connection.stick_to_primary! + CreatedResourceClass.find(params[:id]) # will go to the primary end ``` diff --git a/lib/active_record/connection_adapters/makara_abstract_adapter.rb b/lib/active_record/connection_adapters/makara_abstract_adapter.rb index 487d780a..90b8c231 100644 --- a/lib/active_record/connection_adapters/makara_abstract_adapter.rb +++ b/lib/active_record/connection_adapters/makara_abstract_adapter.rb @@ -95,16 +95,23 @@ def custom_error_message?(connection, message) control_method :close, :steal!, :expire, :lease, :in_use?, :owner, :schema_cache, :pool=, :pool, :schema_cache=, :lock, :seconds_idle, :== - SQL_MASTER_MATCHERS = [/\A\s*select.+for update\Z/i, /select.+lock in share mode\Z/i, /\A\s*select.+(nextval|currval|lastval|get_lock|release_lock|pg_advisory_lock|pg_advisory_unlock)\(/i].map(&:freeze).freeze + SQL_PRIMARY_MATCHERS = [/\A\s*select.+for update\Z/i, /select.+lock in share mode\Z/i, /\A\s*select.+(nextval|currval|lastval|get_lock|release_lock|pg_advisory_lock|pg_advisory_unlock)\(/i].map(&:freeze).freeze SQL_REPLICA_MATCHERS = [/\A\s*(select|with.+\)\s*select)\s/i].map(&:freeze).freeze SQL_ALL_MATCHERS = [/\A\s*set\s/i].map(&:freeze).freeze SQL_SKIP_STICKINESS_MATCHERS = [/\A\s*show\s([\w]+\s)?(field|table|database|schema|view|index)(es|s)?/i, /\A\s*(set|describe|explain|pragma)\s/i].map(&:freeze).freeze + SQL_MASTER_MATCHERS = SQL_PRIMARY_MATCHERS + deprecate_constant :SQL_MASTER_MATCHERS SQL_SLAVE_MATCHERS = SQL_REPLICA_MATCHERS deprecate_constant :SQL_SLAVE_MATCHERS + def sql_primary_matchers + SQL_PRIMARY_MATCHERS + end + def sql_master_matchers - SQL_MASTER_MATCHERS + warn "#{self.class}#sql_master_matchers is deprecated. Use #sql_primary_matchers" + sql_primary_matchers end def sql_replica_matchers @@ -112,7 +119,7 @@ def sql_replica_matchers end def sql_slave_matchers - warn "sql_slave_matchers is deprecated. Use sql_replica_matchers" + warn "#{self.class}#sql_slave_matchers is deprecated. Use #sql_replica_matchers" sql_replica_matchers end @@ -134,21 +141,17 @@ def initialize(config) def appropriate_connection(method_name, args, &block) if needed_by_all?(method_name, args) - handling_an_all_execution(method_name) do hijacked do # replica pool must run first. @replica_pool.send_to_all(nil, &block) # just yields to each con - @master_pool.send_to_all(nil, &block) # just yields to each con + @primary_pool.send_to_all(nil, &block) # just yields to each con end end - else - super(method_name, args) do |con| yield con end - end end @@ -166,12 +169,17 @@ def needed_by_all?(method_name, args) false end - def needs_master?(method_name, args) - sql = coerce_query_to_sql_string(args.first) - return true if sql_master_matchers.any?{|m| sql =~ m } - return false if sql_replica_matchers.any?{|m| sql =~ m } + def needs_primary?(method_name, args) + if respond_to?(:needs_master?) + warn "#{self.class}#needs_master? is deprecated. Switch to #needs_primary?" + needs_master?(method_name, args) + else + sql = coerce_query_to_sql_string(args.first) + return true if sql_primary_matchers.any?{|m| sql =~ m } + return false if sql_replica_matchers.any?{|m| sql =~ m } - true + true + end end def coerce_query_to_sql_string(sql_or_arel) diff --git a/lib/makara/config_parser.rb b/lib/makara/config_parser.rb index a7256e5e..9958e3c0 100644 --- a/lib/makara/config_parser.rb +++ b/lib/makara/config_parser.rb @@ -10,10 +10,11 @@ # top_level: 'variable' # another: 'top level variable' # makara: -# master_ttl: 3 +# primary_ttl: 3 # blacklist_duration: 20 # connections: -# - role: 'master' +# - role: 'master' # Deprecated in favor of 'primary' +# - role: 'primary' # - role: 'slave' # Deprecated in favor of 'replica' # - role: 'replica' # name: 'replica2' @@ -21,11 +22,21 @@ module Makara class ConfigParser DEFAULTS = { - master_ttl: 5, + primary_ttl: 5, blacklist_duration: 30, sticky: true } + DEPRECATED_KEYS = { + slave_strategy: :replica_strategy, + slave_shard_aware: :replica_shard_aware, + slave_default_shard: :replica_default_shard, + master_strategy: :primary_strategy, + master_shard_aware: :primary_shard_aware, + master_default_shard: :primary_default_shard, + master_ttl: :primary_ttl + }.freeze + # ConnectionUrlResolver is borrowed from Rails 4-2 since its location and implementation # vary slightly among Rails versions, but the behavior is the same. Thus, borrowing the # class should be the most future-safe way to parse a database url. @@ -146,7 +157,7 @@ def initialize(config) @makara_config = DEFAULTS.merge(@config[:makara] || {}) @makara_config = @makara_config.symbolize_keys - deprecate_keys(:slave_strategy, :slave_shard_aware, :slave_default_shard) + replace_deprecated_keys! @id = sanitize_id(@makara_config[:id]) end @@ -158,29 +169,45 @@ def id end end - def master_configs + def primary_configs all_configs - .select { |config| config[:role] == 'master' } + .select { |config| config[:role] == 'primary' } .map { |config| config.except(:role) } end + def master_configs + warn "#{self.class}#master_configs is deprecated. Switch to #primary_configs" + primary_configs + end + def replica_configs all_configs - .reject { |config| config[:role] == 'master' } + .reject { |config| config[:role] == 'primary' } .map { |config| config.except(:role) } end def slave_configs - warn "#slave_configs is deprecated. Switch to #replica_configs" + warn "#{self.class}#slave_configs is deprecated. Switch to #replica_configs" replica_configs end protected def all_configs - @makara_config[:connections].map do |connection| - base_config.merge(makara_config.except(:connections)) - .merge(connection.symbolize_keys) + @all_configs ||= @makara_config[:connections].map do |connection| + config = base_config.merge(makara_config.except(:connections)).merge(connection.symbolize_keys) + + if config[:role] == "master" + warn "Makara role 'master' is deprecated. Use 'primary' instead" + config[:role] = "primary" + end + + if config[:role] == "slave" + warn "Makara role 'slave' is deprecated. Use 'replica' instead" + config[:role] = "primary" + end + + config end end @@ -208,14 +235,13 @@ def sanitize_id(id) end end - def deprecate_keys(*keys) - keys.each do |key| + def replace_deprecated_keys! + DEPRECATED_KEYS.each do |key, replacement| next unless @makara_config[key] - new_key = key.to_s.gsub("slave", "replica").to_sym - warn "Config key #{key} is deprecated, use #{new_key} instead" + warn "Makara config key #{key.inspect} is deprecated, use #{replacement.inspect} instead" - @makara_config[new_key] = @makara_config[key] + @makara_config[replacement] = @makara_config.delete(key) end end end diff --git a/lib/makara/context.rb b/lib/makara/context.rb index ccb45662..636a0a15 100644 --- a/lib/makara/context.rb +++ b/lib/makara/context.rb @@ -99,7 +99,7 @@ def set_current(context_data) set(:makara_current_context, new(context_data)) end - # Called by `Proxy#stick_to_master!` to use master in subsequent requests + # Called by `Proxy#stick_to_primary!` to use primary in subsequent requests def stick(proxy_id, ttl) current.stage(proxy_id, ttl) end diff --git a/lib/makara/logging/subscriber.rb b/lib/makara/logging/subscriber.rb index 687e9b54..7aa950cb 100644 --- a/lib/makara/logging/subscriber.rb +++ b/lib/makara/logging/subscriber.rb @@ -18,7 +18,7 @@ def sql(event) # uses the adapter's connection proxy to modify the name of the event # the name of the used connection will be prepended to the sql log ### - ### [Master|Replica] User Load (1.3ms) SELECT * FROM `users`; + ### [Primary|Replica] User Load (1.3ms) SELECT * FROM `users`; ### def current_wrapper_name(event) connection_object_id = event.payload[:connection_id] diff --git a/lib/makara/middleware.rb b/lib/makara/middleware.rb index b8169349..bfc4bf1d 100644 --- a/lib/makara/middleware.rb +++ b/lib/makara/middleware.rb @@ -1,6 +1,6 @@ require 'rack' -# Persists the Makara::Context across requests ensuring the same master pool is used on subsequent requests. +# Persists the Makara::Context across requests ensuring the same primary pool is used on subsequent requests. module Makara class Middleware def initialize(app, cookie_options = {}) diff --git a/lib/makara/pool.rb b/lib/makara/pool.rb index 42c76d5f..b2ef5878 100644 --- a/lib/makara/pool.rb +++ b/lib/makara/pool.rb @@ -17,7 +17,7 @@ class Pool attr_reader :default_shard def initialize(role, proxy) - @role = role + @role = role == "master" ? "primary" : role @proxy = proxy @connections = [] @blacklist_errors = [] @@ -125,7 +125,7 @@ def provide # when a connection causes a blacklist error within the provided block, we blacklist it then retry rescue Makara::Errors::BlacklistConnection => e @blacklist_errors.insert(0, e) - in_transaction = self.role == "master" && provided_connection._makara_in_transaction? + in_transaction = self.role == "primary" && provided_connection._makara_in_transaction? provided_connection._makara_blacklist! raise Makara::Errors::BlacklistedWhileInTransaction.new(@role) if in_transaction diff --git a/lib/makara/proxy.rb b/lib/makara/proxy.rb index 0410c290..866f242c 100644 --- a/lib/makara/proxy.rb +++ b/lib/makara/proxy.rb @@ -3,7 +3,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/string/inflections' -# The entry point of Makara. It contains a master and replica pool which are chosen based on the invocation +# The entry point of Makara. It contains a primary and replica pool which are chosen based on the invocation # being proxied. Makara::Proxy implementations should declare which methods they are hijacking via the # `hijack_method` class method. # While debugging this class use prepend debug calls with Kernel. (Kernel.byebug for example) @@ -66,7 +66,7 @@ def initialize(config) @config = config.symbolize_keys @config_parser = Makara::ConfigParser.new(@config) @id = @config_parser.id - @ttl = @config_parser.makara_config[:master_ttl] + @ttl = @config_parser.makara_config[:primary_ttl] @sticky = @config_parser.makara_config[:sticky] @hijacked = false @error_handler ||= ::Makara::ErrorHandler.new @@ -86,13 +86,18 @@ def hijacked? @hijacked end - # If persist is true, we stick the proxy to master for subsequent requests - # up to master_ttl duration. Otherwise we just stick it for the current request - def stick_to_master!(persist = true) + # If persist is true, we stick the proxy to primary for subsequent requests + # up to primary_ttl duration. Otherwise we just stick it for the current request + def stick_to_primary!(persist = true) stickiness_duration = persist ? @ttl : 0 Makara::Context.stick(@id, stickiness_duration) end + def stick_to_master!(persist = true) + warn "#{self.class}.stick_to_master! is deprecated. Switch to #stick_to_primary!" + stick_to_primary!(persist) + end + def strategy_for(role) strategy_class_for(strategy_name_for(role)).new(self) end @@ -162,33 +167,33 @@ def disconnect! protected def send_to_all(method_name, *args) - # replica pool must run first to allow for replica --> master failover without running operations on master twice. + # replica pool must run first to allow for replica --> primary failover without running operations on the primary twice. handling_an_all_execution(method_name) do @replica_pool.send_to_all(method_name, *args) - @master_pool.send_to_all(method_name, *args) + @primary_pool.send_to_all(method_name, *args) end end ruby2_keywords :send_to_all if Module.private_method_defined?(:ruby2_keywords) def any_connection - if @master_pool.disabled + if @primary_pool.disabled @replica_pool.provide do |con| yield con end else - @master_pool.provide do |con| + @primary_pool.provide do |con| yield con end end rescue ::Makara::Errors::AllConnectionsBlacklisted, ::Makara::Errors::NoConnectionsAvailable begin - @master_pool.disabled = true + @primary_pool.disabled = true @replica_pool.provide do |con| yield con end ensure - @master_pool.disabled = false + @primary_pool.disabled = false end end @@ -205,42 +210,42 @@ def appropriate_connection(method_name, args) end end - # master or replica + # primary or replica def appropriate_pool(method_name, args) # for testing purposes pool = _appropriate_pool(method_name, args) yield pool rescue ::Makara::Errors::AllConnectionsBlacklisted, ::Makara::Errors::NoConnectionsAvailable => e - if pool == @master_pool - @master_pool.connections.each(&:_makara_whitelist!) + if pool == @primary_pool + @primary_pool.connections.each(&:_makara_whitelist!) @replica_pool.connections.each(&:_makara_whitelist!) Kernel.raise e else - @master_pool.blacklist_errors << e + @primary_pool.blacklist_errors << e retry end end def _appropriate_pool(method_name, args) - # the args provided absolutely need master - if needs_master?(method_name, args) - stick_to_master(method_name, args) - @master_pool + # the args provided absolutely need primary + if needs_primary?(method_name, args) + stick_to_primary(method_name, args) + @primary_pool - elsif stuck_to_master? + elsif stuck_to_primary? - # we're on master because we already stuck this proxy in this + # we're on primary because we already stuck this proxy in this # request or because we got stuck in previous requests and the # stickiness is still valid - @master_pool + @primary_pool # all replicas are down (or empty) elsif @replica_pool.completely_blacklisted? - stick_to_master(method_name, args) - @master_pool + stick_to_primary(method_name, args) + @primary_pool elsif in_transaction? - @master_pool + @primary_pool # yay! use a replica else @@ -248,9 +253,14 @@ def _appropriate_pool(method_name, args) end end - # do these args require a master connection - def needs_master?(method_name, args) - true + # Do these args require a primary connection + def needs_primary?(method_name, args) + if respond_to?(:needs_master?) + warn "#{self.class}#needs_master? is deprecated. Switch to #needs_primary?" + needs_master?(method_name, args) + else + true + end end def in_transaction? @@ -268,16 +278,21 @@ def hijacked @hijacked = false end - def stuck_to_master? + def stuck_to_primary? sticky? && Makara::Context.stuck?(@id) end - def stick_to_master(method_name, args) + def stick_to_primary(method_name, args) # check to see if we're configured, bypassed, or some custom implementation has input return unless should_stick?(method_name, args) # do the sticking - stick_to_master! + stick_to_primary! + end + + def stick_to_master(method_names, args) + warn "#{self.class}#stick_to_master is deprecated. Switch to #stick_to_primary" + stick_to_primary(method_names, args) end # For the generic proxy implementation, we stick if we are sticky, @@ -291,12 +306,12 @@ def sticky? @sticky && !@skip_sticking end - # use the config parser to generate a master and replica pool + # use the config parser to generate a primary and replica pool def instantiate_connections - @master_pool = Makara::Pool.new('master', self) - @config_parser.master_configs.each do |master_config| - @master_pool.add master_config do - graceful_connection_for(master_config) + @primary_pool = Makara::Pool.new('primary', self) + @config_parser.primary_configs.each do |primary_config| + @primary_pool.add primary_config do + graceful_connection_for(primary_config) end end @@ -311,7 +326,7 @@ def instantiate_connections def handling_an_all_execution(method_name) yield rescue ::Makara::Errors::NoConnectionsAvailable => e - if e.role == 'master' + if e.role == 'primary' # this means replica connections are good. return end diff --git a/spec/active_record/connection_adapters/makara_abstract_adapter_error_handling_spec.rb b/spec/active_record/connection_adapters/makara_abstract_adapter_error_handling_spec.rb index b018d722..aa3489ca 100644 --- a/spec/active_record/connection_adapters/makara_abstract_adapter_error_handling_spec.rb +++ b/spec/active_record/connection_adapters/makara_abstract_adapter_error_handling_spec.rb @@ -4,7 +4,7 @@ describe ActiveRecord::ConnectionAdapters::MakaraAbstractAdapter::ErrorHandler do let(:handler){ described_class.new } let(:proxy) { FakeAdapter.new(config(1,1)) } - let(:connection){ proxy.master_pool.connections.first } + let(:connection){ proxy.primary_pool.connections.first } [ %|Mysql::Error: : INSERT INTO `watchers` (`user_id`, `watchable_id`, `watchable_type`) VALUES|, @@ -63,7 +63,7 @@ let(:config) { YAML.load(ERB.new(File.read(config_path)).result)['test'] } let(:handler){ described_class.new } let(:proxy) { FakeAdapter.new(config) } - let(:connection){ proxy.master_pool.connections.first } + let(:connection){ proxy.primary_pool.connections.first } let(:msg1) { "ActiveRecord::StatementInvalid: Mysql2::Error: Unknown command1: SELECT `users`.* FROM `users` WHERE `users`.`id` = 53469 LIMIT 1" } let(:msg2) { "activeRecord::statementInvalid: mysql2::error: unknown command2: SELECT `users`.* FROM `users` WHERE `users`.`id` = 53469 LIMIT 1" } let(:msg3) { "ActiveRecord::StatementInvalid: Mysql2::Error: Unknown command3: SELECT `users`.* FROM `users` WHERE `users`.`id` = 53469 LIMIT 1" } diff --git a/spec/active_record/connection_adapters/makara_abstract_adapter_spec.rb b/spec/active_record/connection_adapters/makara_abstract_adapter_spec.rb index cfbf5f92..899cff0a 100644 --- a/spec/active_record/connection_adapters/makara_abstract_adapter_spec.rb +++ b/spec/active_record/connection_adapters/makara_abstract_adapter_spec.rb @@ -36,10 +36,10 @@ 'select release_lock(\'foo\')' => true, 'select pg_advisory_lock(12345)' => true, 'select pg_advisory_unlock(12345)' => true - }.each do |sql, should_go_to_master| - it "determines that \"#{sql}\" #{should_go_to_master ? 'requires' : 'does not require'} master" do + }.each do |sql, should_go_to_primary| + it "determines that \"#{sql}\" #{should_go_to_primary ? 'requires' : 'does not require'} primary" do proxy = klass.new(config(1,1)) - expect(proxy.master_for?(sql)).to eq(should_go_to_master) + expect(proxy.primary_for?(sql)).to eq(should_go_to_primary) end end @@ -59,7 +59,7 @@ }.each do |sql, should_send_to_all_connections| it "determines that \"#{sql}\" #{should_send_to_all_connections ? 'should' : 'should not'} be sent to all underlying connections" do proxy = klass.new(config(1,1)) - proxy.master_pool.connections.each{|con| expect(con).to receive(:execute).with(sql).once} + proxy.primary_pool.connections.each{|con| expect(con).to receive(:execute).with(sql).once} proxy.replica_pool.connections.each do |con| if should_send_to_all_connections expect(con).to receive(:execute).with(sql).once @@ -102,7 +102,7 @@ max_treats IS NULL } => true }.each do |sql,should_stick| - it "should #{should_stick ? 'stick' : 'not stick'} to master if handling sql like \"#{sql}\"" do + it "should #{should_stick ? 'stick' : 'not stick'} to primary if handling sql like \"#{sql}\"" do proxy = klass.new(config(0,0)) expect(proxy.would_stick?(sql)).to eq(should_stick) end diff --git a/spec/active_record/connection_adapters/makara_mysql2_adapter_spec.rb b/spec/active_record/connection_adapters/makara_mysql2_adapter_spec.rb index 6de33259..aabb1e3a 100644 --- a/spec/active_record/connection_adapters/makara_mysql2_adapter_spec.rb +++ b/spec/active_record/connection_adapters/makara_mysql2_adapter_spec.rb @@ -21,7 +21,7 @@ expect(ActiveRecord::Base.connection).to be_instance_of(ActiveRecord::ConnectionAdapters::MakaraMysql2Adapter) end - it 'should execute a send_to_all against master even if no replicas are connected' do + it 'should execute a send_to_all against primary even if no replicas are connected' do establish_connection(config) connection = ActiveRecord::Base.connection @@ -31,7 +31,7 @@ expect(c).to receive(:execute).with('SET @t1 = 1').never end - connection.master_pool.connections.each do |c| + connection.primary_pool.connections.each do |c| expect(c).to receive(:execute).with('SET @t1 = 1') end @@ -44,7 +44,7 @@ establish_connection(config) connection = ActiveRecord::Base.connection - (connection.replica_pool.connections | connection.master_pool.connections).each do |c| + (connection.replica_pool.connections | connection.primary_pool.connections).each do |c| allow(c).to receive(:_makara_blacklisted?){ true } allow(c).to receive(:_makara_connected?){ false } expect(c).to receive(:execute).with('SET @t1 = 1').never @@ -106,16 +106,16 @@ change_context end - it 'should have one master and two replicas' do - expect(connection.master_pool.connection_count).to eq(1) + it 'should have one primary and two replicas' do + expect(connection.primary_pool.connection_count).to eq(1) expect(connection.replica_pool.connection_count).to eq(2) end it 'should allow real queries to work' do connection.execute("INSERT INTO users (name) VALUES ('John')") - connection.master_pool.connections.each do |master| - expect(master).to receive(:execute).never + connection.primary_pool.connections.each do |primary| + expect(primary).to receive(:execute).never end change_context @@ -130,7 +130,7 @@ end it 'should send SET operations to each connection' do - connection.master_pool.connections.each do |con| + connection.primary_pool.connections.each do |con| expect(con).to receive(:execute).with('SET @t1 = 1').once end @@ -164,8 +164,8 @@ Test::User.exists? end - it 'should send writes to master' do - con = connection.master_pool.connections.first + it 'should send writes to primary' do + con = connection.primary_pool.connections.first expect(con).to receive(:execute).with('UPDATE users SET name = "bob" WHERE id = 1') connection.execute('UPDATE users SET name = "bob" WHERE id = 1') end diff --git a/spec/active_record/connection_adapters/makara_postgis_adapter_spec.rb b/spec/active_record/connection_adapters/makara_postgis_adapter_spec.rb index 85b80646..b41d64d5 100644 --- a/spec/active_record/connection_adapters/makara_postgis_adapter_spec.rb +++ b/spec/active_record/connection_adapters/makara_postgis_adapter_spec.rb @@ -52,16 +52,16 @@ end end - it 'should have one master and two replicas' do - expect(connection.master_pool.connection_count).to eq(1) + it 'should have one primary and two replicas' do + expect(connection.primary_pool.connection_count).to eq(1) expect(connection.replica_pool.connection_count).to eq(2) end it 'should allow real queries to work' do connection.execute('INSERT INTO users (name) VALUES (\'John\')') - connection.master_pool.connections.each do |master| - expect(master).to receive(:execute).never + connection.primary_pool.connections.each do |primary| + expect(primary).to receive(:execute).never end change_context @@ -71,7 +71,7 @@ end it 'should send SET operations to each connection' do - connection.master_pool.connections.each do |con| + connection.primary_pool.connections.each do |con| expect(con).to receive(:execute).with("SET TimeZone = 'UTC'").once end @@ -91,8 +91,8 @@ connection.execute('SELECT * FROM users') end - it 'should send writes to master' do - con = connection.master_pool.connections.first + it 'should send writes to primary' do + con = connection.primary_pool.connections.first expect(con).to receive(:execute).with('UPDATE users SET name = "bob" WHERE id = 1') connection.execute('UPDATE users SET name = "bob" WHERE id = 1') end @@ -115,7 +115,7 @@ end end - context 'with only master connection' do + context 'with only primary connection' do it 'should not raise errors on read and write' do custom_config = config.deep_dup custom_config['makara']['connections'].select{|h| h['role'] == 'replica' }.each{|h| h['port'] = '1'} @@ -135,7 +135,7 @@ ActiveRecord::Base.clear_all_connections! custom_config = config.deep_dup - custom_config['makara']['connections'].select{|h| h['role'] == 'master' }.each{|h| h['port'] = '1'} + custom_config['makara']['connections'].select{|h| h['role'] == 'primary' }.each{|h| h['port'] = '1'} ActiveRecord::Base.establish_connection(custom_config) diff --git a/spec/active_record/connection_adapters/makara_postgresql_adapter_spec.rb b/spec/active_record/connection_adapters/makara_postgresql_adapter_spec.rb index a801e8ee..c87210f9 100644 --- a/spec/active_record/connection_adapters/makara_postgresql_adapter_spec.rb +++ b/spec/active_record/connection_adapters/makara_postgresql_adapter_spec.rb @@ -26,16 +26,16 @@ change_context end - it 'should have one master and two replicas' do - expect(connection.master_pool.connection_count).to eq(1) + it 'should have one primary and two replicas' do + expect(connection.primary_pool.connection_count).to eq(1) expect(connection.replica_pool.connection_count).to eq(2) end it 'should allow real queries to work' do connection.execute('INSERT INTO users (name) VALUES (\'John\')') - connection.master_pool.connections.each do |master| - expect(master).to receive(:execute).never + connection.primary_pool.connections.each do |primary| + expect(primary).to receive(:execute).never end change_context @@ -45,7 +45,7 @@ end it 'should send SET operations to each connection' do - connection.master_pool.connections.each do |con| + connection.primary_pool.connections.each do |con| expect(con).to receive(:execute).with("SET TimeZone = 'UTC'").once end @@ -80,8 +80,8 @@ Test::User.exists? end - it 'should send writes to master' do - con = connection.master_pool.connections.first + it 'should send writes to primary' do + con = connection.primary_pool.connections.first expect(con).to receive(:execute).with('UPDATE users SET name = "bob" WHERE id = 1') connection.execute('UPDATE users SET name = "bob" WHERE id = 1') end @@ -97,7 +97,7 @@ end end - context 'with only master connection' do + context 'with only primary connection' do it 'should not raise errors on read and write' do custom_config = config.deep_dup custom_config['makara']['connections'].select{|h| h['role'] == 'replica' }.each{|h| h['port'] = '1'} @@ -117,7 +117,7 @@ ActiveRecord::Base.clear_all_connections! custom_config = config.deep_dup - custom_config['makara']['connections'].select{|h| h['role'] == 'master' }.each{|h| h['port'] = '1'} + custom_config['makara']['connections'].select{|h| h['role'] == 'primary' }.each{|h| h['port'] = '1'} establish_connection(custom_config) diff --git a/spec/config_parser_spec.rb b/spec/config_parser_spec.rb index 094d477b..109cef87 100644 --- a/spec/config_parser_spec.rb +++ b/spec/config_parser_spec.rb @@ -7,8 +7,8 @@ makara: { connections: [ { - role: 'master', - name: 'themaster' + role: 'primary', + name: 'theprimary' }, { name: 'replica1' @@ -24,7 +24,7 @@ context '::merge_and_resolve_default_url_config' do let(:config_without_url) do { - master_ttl: 5, + primary_ttl: 5, blacklist_duration: 30, sticky: true, adapter: 'mysql2_makara', @@ -39,7 +39,7 @@ let(:config_with_url) do { - master_ttl: 5, + primary_ttl: 5, blacklist_duration: 30, sticky: true, adapter: 'mysql2_makara', @@ -106,16 +106,16 @@ expect(parser.id).to eq('myproxyidwithreservedcharacters') end - context 'master and replica configs' do - it 'should provide master and replica configs' do + context 'primary and replica configs' do + it 'should provide primary and replica configs' do parser = described_class.new(config) - expect(parser.master_configs).to eq([ + expect(parser.primary_configs).to eq([ { - name: 'themaster', + name: 'theprimary', top_level: 'value', sticky: true, blacklist_duration: 30, - master_ttl: 5 + primary_ttl: 5 } ]) expect(parser.replica_configs).to eq([ @@ -124,14 +124,14 @@ top_level: 'value', sticky: true, blacklist_duration: 30, - master_ttl: 5 + primary_ttl: 5 }, { name: 'replica2', top_level: 'value', sticky: true, blacklist_duration: 30, - master_ttl: 5 + primary_ttl: 5 } ]) end @@ -142,13 +142,13 @@ config[:makara][:connections][1][:top_level] = 'replica value' parser = described_class.new(config) - expect(parser.master_configs).to eq([ + expect(parser.primary_configs).to eq([ { - name: 'themaster', + name: 'theprimary', top_level: 'value', sticky: true, blacklist_duration: 456, - master_ttl: 5 + primary_ttl: 5 } ]) expect(parser.replica_configs).to eq([ @@ -157,14 +157,14 @@ top_level: 'replica value', sticky: true, blacklist_duration: 123, - master_ttl: 5 + primary_ttl: 5 }, { name: 'replica2', top_level: 'value', sticky: true, blacklist_duration: 123, - master_ttl: 5 + primary_ttl: 5 } ]) end diff --git a/spec/connection_wrapper_spec.rb b/spec/connection_wrapper_spec.rb index 49ac020e..0d64efcc 100644 --- a/spec/connection_wrapper_spec.rb +++ b/spec/connection_wrapper_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' describe Makara::ConnectionWrapper do - let(:proxy){ FakeProxy.new({makara: {blacklist_duration: 5, connections: [{role: 'master'}, {role: 'replica'}, {role: 'replica'}]}}) } + let(:proxy){ FakeProxy.new({makara: {blacklist_duration: 5, connections: [{role: 'primary'}, {role: 'replica'}, {role: 'replica'}]}}) } let(:connection){ subject._makara_connection } - subject{ proxy.master_pool.connections.first } + subject{ proxy.primary_pool.connections.first } it 'should extend the connection with new functionality' do expect(connection).to respond_to(:_makara_name) diff --git a/spec/context_spec.rb b/spec/context_spec.rb index 819f9eae..5abbd254 100644 --- a/spec/context_spec.rb +++ b/spec/context_spec.rb @@ -49,7 +49,7 @@ Makara::Context.set_current(context_data) end - it 'sticks a proxy to master for the current request' do + it 'sticks a proxy to primary for the current request' do expect(Makara::Context.stuck?('mariadb')).to be_falsey Makara::Context.stick('mariadb', 10) diff --git a/spec/middleware_spec.rb b/spec/middleware_spec.rb index d7554bf0..9ff798bc 100644 --- a/spec/middleware_spec.rb +++ b/spec/middleware_spec.rb @@ -41,15 +41,15 @@ _, headers, body = middleware.call(env) expect(headers).to eq({}) - expect(body).to eq('master/1') + expect(body).to eq('primary/1') end - it 'should set the cookie if master is used' do + it 'should set the cookie if primary is used' do env[:query] = 'update users set name = "phil"' _, headers, body = middleware.call(env) expect(headers['Set-Cookie']).to eq("#{key}=mock_mysql%3A#{(now + 5).to_f}; path=/; max-age=10; expires=#{(Time.now + 10).httpdate}; secure; HttpOnly") - expect(body).to eq('master/1') + expect(body).to eq('primary/1') end end diff --git a/spec/pool_spec.rb b/spec/pool_spec.rb index ac8cf0d3..be490057 100644 --- a/spec/pool_spec.rb +++ b/spec/pool_spec.rb @@ -4,7 +4,7 @@ let(:proxy){ FakeProxy.new({makara: pool_config.merge(connections: [])}) } let(:pool){ Makara::Pool.new('test', proxy) } let(:pool_config){ {blacklist_duration: 5} } - let(:master_pool){ Makara::Pool.new('master', proxy) } + let(:primary_pool){ Makara::Pool.new('primary', proxy) } it 'should wrap connections with a ConnectionWrapper as theyre added to the pool' do expect(pool.connections).to be_empty @@ -147,10 +147,10 @@ end it 'should error out while blacklisted in transaction' do - wrapper_a = master_pool.add(pool_config){ FakeConnection.new(open_transactions: 1) } - master_pool.add(pool_config){ FakeConnection.new } + wrapper_a = primary_pool.add(pool_config){ FakeConnection.new(open_transactions: 1) } + primary_pool.add(pool_config){ FakeConnection.new } expect { - master_pool.provide do |connection| + primary_pool.provide do |connection| if connection == wrapper_a raise Makara::Errors::BlacklistConnection.new(wrapper_a, StandardError.new('failure')) end @@ -158,14 +158,14 @@ }.to raise_error(Makara::Errors::BlacklistedWhileInTransaction) end - it 'skips blacklisted connections in master pool when not in transaction' do - wrapper_a = master_pool.add(pool_config){ FakeConnection.new(open_transactions: 0) } - master_pool.add(pool_config){ FakeConnection.new } - master_pool.provide do |connection| + it 'skips blacklisted connections in primary pool when not in transaction' do + wrapper_a = primary_pool.add(pool_config){ FakeConnection.new(open_transactions: 0) } + primary_pool.add(pool_config){ FakeConnection.new } + primary_pool.provide do |connection| if connection == wrapper_a raise Makara::Errors::BlacklistConnection.new(wrapper_a, StandardError.new('failure')) end end - 10.times{ master_pool.provide{|connection| expect(connection).not_to eq(wrapper_a) } } + 10.times{ primary_pool.provide{|connection| expect(connection).not_to eq(wrapper_a) } } end end diff --git a/spec/proxy_spec.rb b/spec/proxy_spec.rb index 92610380..98a74dac 100644 --- a/spec/proxy_spec.rb +++ b/spec/proxy_spec.rb @@ -3,82 +3,82 @@ describe Makara::Proxy do let(:klass){ FakeProxy } - it 'sets up a master and replica pool no matter the number of connections' do + it 'sets up a primary and replica pool no matter the number of connections' do proxy = klass.new(config(0, 0)) - expect(proxy.master_pool).to be_a(Makara::Pool) + expect(proxy.primary_pool).to be_a(Makara::Pool) expect(proxy.replica_pool).to be_a(Makara::Pool) proxy = klass.new(config(2, 0)) - expect(proxy.master_pool).to be_a(Makara::Pool) + expect(proxy.primary_pool).to be_a(Makara::Pool) expect(proxy.replica_pool).to be_a(Makara::Pool) proxy = klass.new(config(0, 2)) - expect(proxy.master_pool).to be_a(Makara::Pool) + expect(proxy.primary_pool).to be_a(Makara::Pool) expect(proxy.replica_pool).to be_a(Makara::Pool) proxy = klass.new(config(2, 2)) - expect(proxy.master_pool).to be_a(Makara::Pool) + expect(proxy.primary_pool).to be_a(Makara::Pool) expect(proxy.replica_pool).to be_a(Makara::Pool) end it 'instantiates N connections within each pool' do proxy = klass.new(config(1, 2)) - expect(proxy.master_pool.connection_count).to eq(1) + expect(proxy.primary_pool.connection_count).to eq(1) expect(proxy.replica_pool.connection_count).to eq(2) end - it 'should delegate any unknown method to a connection in the master pool' do + it 'should delegate any unknown method to a connection in the primary pool' do proxy = klass.new(config(1, 2)) - con = proxy.master_pool.connections.first + con = proxy.primary_pool.connections.first allow(con).to receive(:irespondtothis){ 'hello!' } expect(proxy).to respond_to(:irespondtothis) expect(proxy.irespondtothis).to eq('hello!') end - describe '#stick_to_master' do + describe '#stick_to_primary' do let(:proxy) { klass.new(config(1, 2)) } - it 'should use master if manually forced' do - expect(proxy.master_for?('select * from users')).to eq(false) + it 'should use primary if manually forced' do + expect(proxy.primary_for?('select * from users')).to eq(false) - proxy.stick_to_master! + proxy.stick_to_primary! - expect(proxy.master_for?('select * from users')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) end it 'should persist stickiness by default' do now = Time.now - proxy.stick_to_master! + proxy.stick_to_primary! next_context = Makara::Context.next expect(next_context[proxy.id]).to be >= (now + 5).to_f proxy = klass.new(config(1, 2)) - expect(proxy.master_for?('select * from users')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) end it 'optionally skips stickiness persistence, so it applies only to the current request' do now = Time.now - proxy.stick_to_master!(false) + proxy.stick_to_primary!(false) - expect(proxy.master_for?('select * from users')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) next_context = Makara::Context.next expect(next_context).to be_nil # Nothing to persist, so context is empty proxy = klass.new(config(1, 2)) - expect(proxy.master_for?('select * from users')).to eq(false) + expect(proxy.primary_for?('select * from users')).to eq(false) end - it 'supports a float master_ttl for stickiness duration' do + it 'supports a float primary_ttl for stickiness duration' do now = Time.now config = config(1, 2).dup - config[:makara][:master_ttl] = 0.5 + config[:makara][:primary_ttl] = 0.5 proxy = klass.new(config) - proxy.stick_to_master! + proxy.stick_to_primary! next_context = Makara::Context.next expect(next_context[proxy.id]).to be >= (now + 0.5).to_f @@ -94,80 +94,80 @@ end it 'should provide the replica pool for a read' do - expect(proxy.master_for?('select * from users')).to eq(false) + expect(proxy.primary_for?('select * from users')).to eq(false) end - it 'should provide the master pool for a write' do - expect(proxy.master_for?('insert into users values (a,b,c)')).to eq(true) + it 'should provide the primary pool for a write' do + expect(proxy.primary_for?('insert into users values (a,b,c)')).to eq(true) end - # master is used, it should continue being used for the duration of the context - it 'should stick to master once used for a sticky operation' do - expect(proxy.master_for?('insert into users values (a,b,c)')).to eq(true) - expect(proxy.master_for?('select * from users')).to eq(true) + # primary is used, it should continue being used for the duration of the context + it 'should stick to primary once used for a sticky operation' do + expect(proxy.primary_for?('insert into users values (a,b,c)')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) end - it 'should not stick to master if stickiness is disabled' do + it 'should not stick to primary if stickiness is disabled' do proxy.sticky = false - expect(proxy.master_for?('insert into users values (a,b,c)')).to eq(true) - expect(proxy.master_for?('select * from users')).to eq(false) + expect(proxy.primary_for?('insert into users values (a,b,c)')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(false) end - it 'should not stick to master if we are in a without_sticking block' do - expect(proxy.master_for?('insert into users values (a,b,c)')).to eq(true) + it 'should not stick to primary if we are in a without_sticking block' do + expect(proxy.primary_for?('insert into users values (a,b,c)')).to eq(true) proxy.without_sticking do - expect(proxy.master_for?('insert into users values (a,b,c)')).to eq(true) - expect(proxy.master_for?('select * from users')).to eq(false) + expect(proxy.primary_for?('insert into users values (a,b,c)')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(false) end - expect(proxy.master_for?('select * from users')).to eq(true) - expect(proxy.master_for?('insert into users values (a,b,c)')).to eq(true) - expect(proxy.master_for?('select * from users')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) + expect(proxy.primary_for?('insert into users values (a,b,c)')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) end - it 'should not stick to master after without_sticking block if there is a write in it' do - expect(proxy.master_for?('select * from users')).to eq(false) + it 'should not stick to primary after without_sticking block if there is a write in it' do + expect(proxy.primary_for?('select * from users')).to eq(false) proxy.without_sticking do - expect(proxy.master_for?('insert into users values (a,b,c)')).to eq(true) - expect(proxy.master_for?('select * from users')).to eq(false) + expect(proxy.primary_for?('insert into users values (a,b,c)')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(false) end - expect(proxy.master_for?('select * from users')).to eq(false) + expect(proxy.primary_for?('select * from users')).to eq(false) end - it "should not release master if it was stuck in the same request (no context changes yet)" do - expect(proxy.master_for?('insert into users values (a,b,c)')).to eq(true) - expect(proxy.master_for?('select * from users')).to eq(true) + it "should not release primary if it was stuck in the same request (no context changes yet)" do + expect(proxy.primary_for?('insert into users values (a,b,c)')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) Timecop.travel Time.now + 10 do - # master_ttl has passed but we are still in the same request, so current context + # primary_ttl has passed but we are still in the same request, so current context # is still relevant - expect(proxy.master_for?('select * from users')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) end end - it 'should release master if all stuck connections are released' do - expect(proxy.master_for?('insert into users values (a,b,c)')).to eq(true) - expect(proxy.master_for?('select * from users')).to eq(true) + it 'should release primary if all stuck connections are released' do + expect(proxy.primary_for?('insert into users values (a,b,c)')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) Makara::Context.release_all - expect(proxy.master_for?('select * from users')).to eq(false) + expect(proxy.primary_for?('select * from users')).to eq(false) end - it 'should use master if all replicas are blacklisted' do + it 'should use primary if all replicas are blacklisted' do allow(proxy.replica_pool).to receive(:completely_blacklisted?){ true } - expect(proxy.master_for?('select * from users')).to eq(true) + expect(proxy.primary_for?('select * from users')).to eq(true) end - it 'should use master if all replicas become blacklisted as part of the invocation' do + it 'should use primary if all replicas become blacklisted as part of the invocation' do allow(proxy.replica_pool).to receive(:next).and_return(nil) test = double expect(test).to receive(:blacklisting).once - expect(test).to receive(:using_master).once + expect(test).to receive(:using_primary).once proxy.send(:appropriate_pool, :execute, ['select * from users']) do |pool| if pool == proxy.replica_pool @@ -176,7 +176,7 @@ pool.connections.each(&:_makara_blacklist!) pool.provide else - test.using_master + test.using_primary end end end @@ -187,21 +187,21 @@ # weird setup to allow for the correct proxy.replica_pool.connections.each(&:_makara_blacklist!) proxy.replica_pool.instance_variable_get('@blacklist_errors') << StandardError.new('some replica connection issue') - proxy.master_pool.connections.each(&:_makara_blacklist!) - proxy.master_pool.instance_variable_get('@blacklist_errors') << StandardError.new('some master connection issue') + proxy.primary_pool.connections.each(&:_makara_blacklist!) + proxy.primary_pool.instance_variable_get('@blacklist_errors') << StandardError.new('some primary connection issue') - allow(proxy).to receive(:_appropriate_pool).and_return(proxy.replica_pool, proxy.master_pool) + allow(proxy).to receive(:_appropriate_pool).and_return(proxy.replica_pool, proxy.primary_pool) begin proxy.send(:appropriate_pool, :execute, ['select * from users']) do |pool| pool.provide{|c| c } end rescue Makara::Errors::AllConnectionsBlacklisted => e - expect(e.message).to eq('[Makara/master] All connections are blacklisted -> some master connection issue -> [Makara/replica] All connections are blacklisted -> some replica connection issue') + expect(e.message).to eq('[Makara/primary] All connections are blacklisted -> some primary connection issue -> [Makara/replica] All connections are blacklisted -> some replica connection issue') end proxy.replica_pool.connections.each{|con| expect(con._makara_blacklisted?).to eq(false) } - proxy.master_pool.connections.each{|con| expect(con._makara_blacklisted?).to eq(false) } + proxy.primary_pool.connections.each{|con| expect(con._makara_blacklisted?).to eq(false) } end end end diff --git a/spec/strategies/priority_failover_spec.rb b/spec/strategies/priority_failover_spec.rb index cb2c268e..c1636206 100644 --- a/spec/strategies/priority_failover_spec.rb +++ b/spec/strategies/priority_failover_spec.rb @@ -2,9 +2,9 @@ describe Makara::Strategies::PriorityFailover do let(:proxy){ FakeProxy.new({makara: pool_config.merge(makara_config).merge(connections: [])}) } - let(:pool){ Makara::Pool.new('master', proxy) } + let(:pool){ Makara::Pool.new('primary', proxy) } let(:pool_config){ {blacklist_duration: 5} } - let(:makara_config) { { master_strategy: 'failover' } } + let(:makara_config) { { primary_strategy: 'failover' } } let(:strategy) { pool.strategy } it 'should use the strategy' do diff --git a/spec/strategies/shard_aware_spec.rb b/spec/strategies/shard_aware_spec.rb index 9d7306a3..ded81ba1 100644 --- a/spec/strategies/shard_aware_spec.rb +++ b/spec/strategies/shard_aware_spec.rb @@ -12,12 +12,12 @@ def with_shard(shard_id) describe "failover strategy with shard awareness," do let(:proxy){ FakeProxy.new({makara: pool_config.merge(makara_config).merge(connections: [])}) } - let(:pool){ Makara::Pool.new('master', proxy) } + let(:pool){ Makara::Pool.new('primary', proxy) } let(:pool_config){ { blacklist_duration: 5} } let(:makara_config) { { - master_strategy: 'failover', - master_shard_aware: true, - master_default_shard: 'shard2' + primary_strategy: 'failover', + primary_shard_aware: true, + primary_default_shard: 'shard2' } } let(:strategy) { pool.strategy } @@ -109,12 +109,12 @@ def with_shard(shard_id) describe "round_robin strategy with shard awareness," do let(:proxy){ FakeProxy.new({makara: pool_config.merge(makara_config).merge(connections: [])}) } - let(:pool){ Makara::Pool.new('master', proxy) } + let(:pool){ Makara::Pool.new('primary', proxy) } let(:pool_config){ { blacklist_duration: 5} } let(:makara_config) { { - master_strategy: 'round_robin', - master_shard_aware: true, - master_default_shard: 'shard2' + primary_strategy: 'round_robin', + primary_shard_aware: true, + primary_default_shard: 'shard2' } } let(:strategy) { pool.strategy } @@ -186,12 +186,12 @@ def with_shard(shard_id) describe "uses the configured failover strategy when shard_aware set to false," do let(:proxy){ FakeProxy.new({makara: pool_config.merge(makara_config).merge(connections: [])}) } - let(:pool){ Makara::Pool.new('master', proxy) } + let(:pool){ Makara::Pool.new('primary', proxy) } let(:pool_config){ { blacklist_duration: 5} } let(:makara_config) { { - master_strategy: 'failover', - master_shard_aware: false, - master_default_shard: 'shard2' + primary_strategy: 'failover', + primary_shard_aware: false, + primary_default_shard: 'shard2' } } let(:strategy) { pool.strategy } @@ -202,12 +202,12 @@ def with_shard(shard_id) describe "uses the configured roundrobin strategy when shard_aware set to false," do let(:proxy){ FakeProxy.new({makara: pool_config.merge(makara_config).merge(connections: [])}) } - let(:pool){ Makara::Pool.new('master', proxy) } + let(:pool){ Makara::Pool.new('primary', proxy) } let(:pool_config){ { blacklist_duration: 5} } let(:makara_config) { { - master_strategy: 'round_robin', - master_shard_aware: false, - master_default_shard: 'shard2' + primary_strategy: 'round_robin', + primary_shard_aware: false, + primary_default_shard: 'shard2' } } let(:strategy) { pool.strategy } diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 2cc857d0..3909065d 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -4,19 +4,19 @@ def establish_connection(config) # make sure these are all reset to not be blacklisted ActiveRecord::Base.connection.replica_pool.connections.each(&:_makara_whitelist!) - ActiveRecord::Base.connection.master_pool.connections.each(&:_makara_whitelist!) + ActiveRecord::Base.connection.primary_pool.connections.each(&:_makara_whitelist!) ActiveRecord::Base.connection end - def config(masters = 1, replicas = 2) + def config(primaries = 1, replicas = 2) connections = [] - masters.times{ connections << {role: 'master'} } + primaries.times{ connections << {role: 'primary'} } replicas.times{ connections << {role: 'replica'} } { makara: { # Defaults: - # :master_ttl => 5, + # :primary_ttl => 5, # :blacklist_duration => 30, # :sticky => true id: 'mock_mysql', diff --git a/spec/support/mock_objects.rb b/spec/support/mock_objects.rb index 3fdf35a8..f088f5d6 100644 --- a/spec/support/mock_objects.rb +++ b/spec/support/mock_objects.rb @@ -56,7 +56,7 @@ def connection_for(config) FakeConnection.new(config) end - def needs_master?(method_name, args) + def needs_primary?(method_name, args) return false if args.first =~ /^select/ true diff --git a/spec/support/mysql2_database.yml b/spec/support/mysql2_database.yml index 1e0e42b2..cd78abd4 100644 --- a/spec/support/mysql2_database.yml +++ b/spec/support/mysql2_database.yml @@ -11,8 +11,8 @@ test: makara: blacklist_duration: 2 - master_ttl: 5 + primary_ttl: 5 connections: - - role: master + - role: primary - role: replica - role: replica diff --git a/spec/support/mysql2_database_with_custom_errors.yml b/spec/support/mysql2_database_with_custom_errors.yml index 852242d3..7c9a8ec8 100644 --- a/spec/support/mysql2_database_with_custom_errors.yml +++ b/spec/support/mysql2_database_with_custom_errors.yml @@ -11,9 +11,9 @@ test: makara: blacklist_duration: 2 - master_ttl: 5 + primary_ttl: 5 connections: - - role: master + - role: primary - role: replica - role: replica connection_error_matchers: diff --git a/spec/support/postgis_database.yml b/spec/support/postgis_database.yml index dd5ec495..2a16d4e1 100644 --- a/spec/support/postgis_database.yml +++ b/spec/support/postgis_database.yml @@ -8,8 +8,8 @@ test: makara: blacklist_duration: 2 - master_ttl: 5 + primary_ttl: 5 connections: - - role: master + - role: primary - role: replica - role: replica diff --git a/spec/support/postgresql_database.yml b/spec/support/postgresql_database.yml index cf41dd7d..0a41a7de 100644 --- a/spec/support/postgresql_database.yml +++ b/spec/support/postgresql_database.yml @@ -6,8 +6,8 @@ test: makara: blacklist_duration: 2 - master_ttl: 5 + primary_ttl: 5 connections: - - role: master + - role: primary - role: replica - role: replica diff --git a/spec/support/proxy_extensions.rb b/spec/support/proxy_extensions.rb index f9ec04fc..ae73903b 100644 --- a/spec/support/proxy_extensions.rb +++ b/spec/support/proxy_extensions.rb @@ -1,8 +1,8 @@ module ProxyExtensions - attr_reader :master_pool, :replica_pool, :id + attr_reader :primary_pool, :replica_pool, :id - def master_for?(sql) - pool_for(sql) == master_pool + def primary_for?(sql) + pool_for(sql) == primary_pool end def would_stick?(sql)