Commit 771f94fc authored by Yorick Peterse's avatar Yorick Peterse

Add support for load balancing multiple databases

This adds support for using the database load balancer with multiple
databases. Load balancing is applied to two classes:

- ActiveRecord::Base
- Ci::CiDatabaseRecord

Each class has its own load balancer, configuration, service discovery,
etc. Load balancing for the CI class is only enabled when a CI
configuration exists, as it can reuse the main load balancer when
there's no dedicated CI database.

Sticking technically supports multiple databases, but in practise we
apply the same sticking rules to all databases. This is due to how
LoadBalancing::Session is used: there is only one instance per
request/Sidekiq job, and it's not aware of what database connections did
what. This means that a write to database A will result in GitLab
sticking to the primaries of _all_ databases. The choice for this is
simple: it requires fewer code changes, and allows us to introduce
multiple database support in smaller increments.

One change we made to sticking is to turn the Sticking module into a
class, and attach an instance to every base module that has its own load
balancer. This makes it easier to apply sticking on a per-database level
in the future, without having to type
`Gitlab::Database::LoadBalancing::Sticking...` every time.

Sticking also supports reading and writing of data using the old Redis
key names. This ensures sticking continues to work during a deployment,
as during this window we'll run two different versions in production.
Once the code has been deployed to GitLab.com and has been confirmed to
work, we'll remove support for reading/writing the old keys.

Sidekiq also supports load balancing multiple databases. If a load
balancer/database doesn't have any WAL data in the Sidekiq job, we treat
the database as being in sync. This way we can support Sidekiq jobs
using both the old and new load balancing data.

See https://gitlab.com/gitlab-org/gitlab/-/issues/331776 for more
details.

Changelog: added
parent e6ec3cdc
...@@ -1109,7 +1109,7 @@ module Ci ...@@ -1109,7 +1109,7 @@ module Ci
return unless saved_change_to_status? return unless saved_change_to_status?
return unless running? return unless running?
::Gitlab::Database::LoadBalancing::Sticking.stick(:build, id) self.class.sticking.stick(:build, id)
end end
def status_commit_hooks def status_commit_hooks
......
...@@ -12,13 +12,6 @@ module Ci ...@@ -12,13 +12,6 @@ module Ci
if Gitlab::Database.has_config?(:ci) if Gitlab::Database.has_config?(:ci)
connects_to database: { writing: :ci, reading: :ci } connects_to database: { writing: :ci, reading: :ci }
# TODO: Load Balancing messes with `CiDatabaseRecord`
# returning wrong connection. To be removed once merged:
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67773
def self.connection
retrieve_connection
end
end end
end end
end end
...@@ -348,7 +348,7 @@ module Ci ...@@ -348,7 +348,7 @@ module Ci
# intention here is not to execute `Ci::RegisterJobService#execute` on # intention here is not to execute `Ci::RegisterJobService#execute` on
# the primary database. # the primary database.
# #
::Gitlab::Database::LoadBalancing::Sticking.stick(:runner, id) ::Ci::Runner.sticking.stick(:runner, id)
SecureRandom.hex.tap do |new_update| SecureRandom.hex.tap do |new_update|
::Gitlab::Workhorse.set_key_and_notify(runner_queue_key, new_update, ::Gitlab::Workhorse.set_key_and_notify(runner_queue_key, new_update,
......
...@@ -2400,7 +2400,7 @@ class Project < ApplicationRecord ...@@ -2400,7 +2400,7 @@ class Project < ApplicationRecord
end end
def mark_primary_write_location def mark_primary_write_location
::Gitlab::Database::LoadBalancing::Sticking.mark_primary_write_location(:project, self.id) self.class.sticking.mark_primary_write_location(:project, self.id)
end end
def toggle_ci_cd_settings!(settings_attribute) def toggle_ci_cd_settings!(settings_attribute)
......
...@@ -22,7 +22,8 @@ module Ci ...@@ -22,7 +22,8 @@ module Ci
end end
def execute(params = {}) def execute(params = {})
db_all_caught_up = ::Gitlab::Database::LoadBalancing::Sticking.all_caught_up?(:runner, runner.id) db_all_caught_up =
::Ci::Runner.sticking.all_caught_up?(:runner, runner.id)
@metrics.increment_queue_operation(:queue_attempt) @metrics.increment_queue_operation(:queue_attempt)
......
...@@ -30,7 +30,7 @@ class UserProjectAccessChangedService ...@@ -30,7 +30,7 @@ class UserProjectAccessChangedService
end end
end end
::Gitlab::Database::LoadBalancing::Sticking.bulk_stick(:user, @user_ids) ::User.sticking.bulk_stick(:user, @user_ids)
result result
end end
......
# frozen_string_literal: true # frozen_string_literal: true
ActiveRecord::Base.singleton_class.attr_accessor :load_balancing_proxy
Gitlab::Database.main.disable_prepared_statements
Gitlab::Application.configure do |config| Gitlab::Application.configure do |config|
config.middleware.use(Gitlab::Database::LoadBalancing::RackMiddleware) config.middleware.use(Gitlab::Database::LoadBalancing::RackMiddleware)
end end
# This hijacks the "connection" method to ensure both Gitlab::Database::LoadBalancing.base_models.each do |model|
# `ActiveRecord::Base.connection` and all models use the same load # The load balancer needs to be configured immediately, and re-configured
# balancing proxy. # after forking. This ensures queries that run before forking use the load
ActiveRecord::Base.singleton_class.prepend(Gitlab::Database::LoadBalancing::ActiveRecordProxy) # balancer, and queries running after a fork don't run into any errors when
# using dead database connections.
# The load balancer needs to be configured immediately, and re-configured after #
# forking. This ensures queries that run before forking use the load balancer, # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63485 for more
# and queries running after a fork don't run into any errors when using dead # information.
# database connections. Gitlab::Database::LoadBalancing::Setup.new(model).setup
#
# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63485 for more # Database queries may be run before we fork, so we must set up the load
# information. # balancer as early as possible. When we do fork, we need to make sure all the
setup = proc do # hosts are disconnected.
lb = Gitlab::Database::LoadBalancing::LoadBalancer.new( Gitlab::Cluster::LifecycleEvents.on_before_fork do
Gitlab::Database::LoadBalancing.configuration, # When forking, we don't want to wait until the connections aren't in use
primary_only: !Gitlab::Database::LoadBalancing.enable_replicas? # any more, as this could delay the boot cycle.
) model.connection.load_balancer.disconnect!(timeout: 0)
end
ActiveRecord::Base.load_balancing_proxy =
Gitlab::Database::LoadBalancing::ConnectionProxy.new(lb) # Service discovery only needs to run in the worker processes, as the main one
# won't be running many (if any) database queries.
# Populate service discovery immediately if it is configured Gitlab::Cluster::LifecycleEvents.on_worker_start do
Gitlab::Database::LoadBalancing.perform_service_discovery Gitlab::Database::LoadBalancing::Setup
end .new(model, start_service_discovery: true)
.setup
setup.call end
# Database queries may be run before we fork, so we must set up the load
# balancer as early as possible. When we do fork, we need to make sure all the
# hosts are disconnected.
Gitlab::Cluster::LifecycleEvents.on_before_fork do
# When forking, we don't want to wait until the connections aren't in use any
# more, as this could delay the boot cycle.
Gitlab::Database::LoadBalancing.proxy.load_balancer.disconnect!(timeout: 0)
end
# Service discovery only needs to run in the worker processes, as the main one
# won't be running many (if any) database queries.
Gitlab::Cluster::LifecycleEvents.on_worker_start do
setup.call
Gitlab::Database::LoadBalancing.start_service_discovery
end end
...@@ -42,8 +42,7 @@ module API ...@@ -42,8 +42,7 @@ module API
token = params[:token] token = params[:token]
if token if token
::Gitlab::Database::LoadBalancing::RackMiddleware ::Ci::Runner.sticking.stick_or_unstick_request(env, :runner, token)
.stick_or_unstick(env, :runner, token)
end end
strong_memoize(:current_runner) do strong_memoize(:current_runner) do
...@@ -80,8 +79,9 @@ module API ...@@ -80,8 +79,9 @@ module API
id = params[:id] id = params[:id]
if id if id
::Gitlab::Database::LoadBalancing::RackMiddleware ::Ci::Build
.stick_or_unstick(env, :build, id) .sticking
.stick_or_unstick_request(env, :build, id)
end end
strong_memoize(:current_job) do strong_memoize(:current_job) do
......
...@@ -75,8 +75,9 @@ module API ...@@ -75,8 +75,9 @@ module API
save_current_user_in_env(@current_user) if @current_user save_current_user_in_env(@current_user) if @current_user
if @current_user if @current_user
::Gitlab::Database::LoadBalancing::RackMiddleware ::ApplicationRecord
.stick_or_unstick(env, :user, @current_user.id) .sticking
.stick_or_unstick_request(env, :user, @current_user.id)
end end
@current_user @current_user
......
...@@ -27,7 +27,7 @@ module Gitlab ...@@ -27,7 +27,7 @@ module Gitlab
# report no matching merge requests. To avoid this, we check # report no matching merge requests. To avoid this, we check
# the write location to ensure the replica can make this query. # the write location to ensure the replica can make this query.
track_session_metrics do track_session_metrics do
::Gitlab::Database::LoadBalancing::Sticking.select_valid_host(:project, @project.id) ::ApplicationRecord.sticking.select_valid_host(:project, @project.id)
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
......
...@@ -286,7 +286,8 @@ module Gitlab ...@@ -286,7 +286,8 @@ module Gitlab
def destroy_stream(build) def destroy_stream(build)
if consistent_archived_trace?(build) if consistent_archived_trace?(build)
::Gitlab::Database::LoadBalancing::Sticking ::Ci::Build
.sticking
.stick(LOAD_BALANCING_STICKING_NAMESPACE, build.id) .stick(LOAD_BALANCING_STICKING_NAMESPACE, build.id)
end end
...@@ -295,7 +296,8 @@ module Gitlab ...@@ -295,7 +296,8 @@ module Gitlab
def read_trace_artifact(build) def read_trace_artifact(build)
if consistent_archived_trace?(build) if consistent_archived_trace?(build)
::Gitlab::Database::LoadBalancing::Sticking ::Ci::Build
.sticking
.unstick_or_continue_sticking(LOAD_BALANCING_STICKING_NAMESPACE, build.id) .unstick_or_continue_sticking(LOAD_BALANCING_STICKING_NAMESPACE, build.id)
end end
......
...@@ -53,7 +53,12 @@ module Gitlab ...@@ -53,7 +53,12 @@ module Gitlab
def self.database_base_models def self.database_base_models
@database_base_models ||= { @database_base_models ||= {
main: ::ApplicationRecord, # Note that we use ActiveRecord::Base here and not ApplicationRecord.
# This is deliberate, as we also use these classes to apply load
# balancing to, and the load balancer must be enabled for _all_ models
# that inher from ActiveRecord::Base; not just our own models that
# inherit from ApplicationRecord.
main: ::ActiveRecord::Base,
ci: ::Ci::CiDatabaseRecord.connection_class? ? ::Ci::CiDatabaseRecord : nil ci: ::Ci::CiDatabaseRecord.connection_class? ? ::Ci::CiDatabaseRecord : nil
}.compact.freeze }.compact.freeze
end end
......
...@@ -18,44 +18,20 @@ module Gitlab ...@@ -18,44 +18,20 @@ module Gitlab
ActiveRecord::ConnectionNotEstablished ActiveRecord::ConnectionNotEstablished
].freeze ].freeze
def self.proxy def self.base_models
ActiveRecord::Base.load_balancing_proxy @base_models ||= ::Gitlab::Database.database_base_models.values.freeze
end end
# Returns a Hash containing the load balancing configuration. def self.each_load_balancer
def self.configuration return to_enum(__method__) unless block_given?
@configuration ||= Configuration.for_model(ActiveRecord::Base)
end
# Returns `true` if the use of load balancing replicas should be enabled.
#
# This is disabled for Rake tasks to ensure e.g. database migrations
# always produce consistent results.
def self.enable_replicas?
return false if Gitlab::Runtime.rake?
configured? base_models.each do |model|
end yield model.connection.load_balancer
end
def self.configured?
configuration.load_balancing_enabled? ||
configuration.service_discovery_enabled?
end
def self.start_service_discovery
return unless configuration.service_discovery_enabled?
ServiceDiscovery
.new(proxy.load_balancer, **configuration.service_discovery)
.start
end end
def self.perform_service_discovery def self.release_hosts
return unless configuration.service_discovery_enabled? each_load_balancer(&:release_host)
ServiceDiscovery
.new(proxy.load_balancer, **configuration.service_discovery)
.perform_service_discovery
end end
DB_ROLES = [ DB_ROLES = [
......
...@@ -16,7 +16,7 @@ module Gitlab ...@@ -16,7 +16,7 @@ module Gitlab
inner.call inner.call
ensure ensure
::Gitlab::Database::LoadBalancing.proxy.load_balancer.release_host ::Gitlab::Database::LoadBalancing.release_hosts
::Gitlab::Database::LoadBalancing::Session.clear_session ::Gitlab::Database::LoadBalancing::Session.clear_session
end end
end end
......
# frozen_string_literal: true
module Gitlab
module Database
module LoadBalancing
# Module injected into ActiveRecord::Base to allow hijacking of the
# "connection" method.
module ActiveRecordProxy
def connection
::Gitlab::Database::LoadBalancing.proxy || super
end
end
end
end
end
...@@ -72,7 +72,14 @@ module Gitlab ...@@ -72,7 +72,14 @@ module Gitlab
Database.default_pool_size Database.default_pool_size
end end
# Returns `true` if the use of load balancing replicas should be
# enabled.
#
# This is disabled for Rake tasks to ensure e.g. database migrations
# always produce consistent results.
def load_balancing_enabled? def load_balancing_enabled?
return false if Gitlab::Runtime.rake?
hosts.any? || service_discovery_enabled? hosts.any? || service_discovery_enabled?
end end
......
...@@ -12,22 +12,22 @@ module Gitlab ...@@ -12,22 +12,22 @@ module Gitlab
REPLICA_SUFFIX = '_replica' REPLICA_SUFFIX = '_replica'
attr_reader :host_list, :configuration attr_reader :name, :host_list, :configuration
# configuration - An instance of `LoadBalancing::Configuration` that # configuration - An instance of `LoadBalancing::Configuration` that
# contains the configuration details (such as the hosts) # contains the configuration details (such as the hosts)
# for this load balancer. # for this load balancer.
# primary_only - If set, the replicas are ignored and the primary is def initialize(configuration)
# always used.
def initialize(configuration, primary_only: false)
@configuration = configuration @configuration = configuration
@primary_only = primary_only @primary_only = !configuration.load_balancing_enabled?
@host_list = @host_list =
if primary_only if @primary_only
HostList.new([PrimaryHost.new(self)]) HostList.new([PrimaryHost.new(self)])
else else
HostList.new(configuration.hosts.map { |addr| Host.new(addr, self) }) HostList.new(configuration.hosts.map { |addr| Host.new(addr, self) })
end end
@name = @configuration.model.connection_db_config.name.to_sym
end end
def primary_only? def primary_only?
......
...@@ -9,21 +9,6 @@ module Gitlab ...@@ -9,21 +9,6 @@ module Gitlab
class RackMiddleware class RackMiddleware
STICK_OBJECT = 'load_balancing.stick_object' STICK_OBJECT = 'load_balancing.stick_object'
# Unsticks or continues sticking the current request.
#
# This method also updates the Rack environment so #call can later
# determine if we still need to stick or not.
#
# env - The Rack environment.
# namespace - The namespace to use for sticking.
# id - The identifier to use for sticking.
def self.stick_or_unstick(env, namespace, id)
::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking(namespace, id)
env[STICK_OBJECT] ||= Set.new
env[STICK_OBJECT] << [namespace, id]
end
def initialize(app) def initialize(app)
@app = app @app = app
end end
...@@ -51,41 +36,46 @@ module Gitlab ...@@ -51,41 +36,46 @@ module Gitlab
# Typically this code will only be reachable for Rails requests as # Typically this code will only be reachable for Rails requests as
# Grape data is not yet available at this point. # Grape data is not yet available at this point.
def unstick_or_continue_sticking(env) def unstick_or_continue_sticking(env)
namespaces_and_ids = sticking_namespaces_and_ids(env) namespaces_and_ids = sticking_namespaces(env)
namespaces_and_ids.each do |namespace, id| namespaces_and_ids.each do |(model, namespace, id)|
::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking(namespace, id) model.sticking.unstick_or_continue_sticking(namespace, id)
end end
end end
# Determine if we need to stick after handling a request. # Determine if we need to stick after handling a request.
def stick_if_necessary(env) def stick_if_necessary(env)
namespaces_and_ids = sticking_namespaces_and_ids(env) namespaces_and_ids = sticking_namespaces(env)
namespaces_and_ids.each do |namespace, id| namespaces_and_ids.each do |model, namespace, id|
::Gitlab::Database::LoadBalancing::Sticking.stick_if_necessary(namespace, id) model.sticking.stick_if_necessary(namespace, id)
end end
end end
def clear def clear
load_balancer.release_host ::Gitlab::Database::LoadBalancing.release_hosts
::Gitlab::Database::LoadBalancing::Session.clear_session ::Gitlab::Database::LoadBalancing::Session.clear_session
end end
def load_balancer
::Gitlab::Database::LoadBalancing.proxy.load_balancer
end
# Determines the sticking namespace and identifier based on the Rack # Determines the sticking namespace and identifier based on the Rack
# environment. # environment.
# #
# For Rails requests this uses warden, but Grape and others have to # For Rails requests this uses warden, but Grape and others have to
# manually set the right environment variable. # manually set the right environment variable.
def sticking_namespaces_and_ids(env) def sticking_namespaces(env)
warden = env['warden'] warden = env['warden']
if warden && warden.user if warden && warden.user
[[:user, warden.user.id]] # When sticking per user, _only_ sticking the main connection could
# result in the application trying to read data from a different
# connection, while that data isn't available yet.
#
# To prevent this from happening, we scope sticking to all the
# models that support load balancing. In the future (if we
# determined this to be OK) we may be able to relax this.
LoadBalancing.base_models.map do |model|
[model, :user, warden.user.id]
end
elsif env[STICK_OBJECT].present? elsif env[STICK_OBJECT].present?
env[STICK_OBJECT].to_a env[STICK_OBJECT].to_a
else else
......
# frozen_string_literal: true
module Gitlab
module Database
module LoadBalancing
# Class for setting up load balancing of a specific model.
class Setup
attr_reader :configuration
def initialize(model, start_service_discovery: false)
@model = model
@configuration = Configuration.for_model(model)
@start_service_discovery = start_service_discovery
end
def setup
disable_prepared_statements
setup_load_balancer
setup_service_discovery
end
def disable_prepared_statements
db_config_object = @model.connection_db_config
config =
db_config_object.configuration_hash.merge(prepared_statements: false)
hash_config = ActiveRecord::DatabaseConfigurations::HashConfig.new(
db_config_object.env_name,
db_config_object.name,
config
)
@model.establish_connection(hash_config)
end
def setup_load_balancer
lb = LoadBalancer.new(configuration)
# We just use a simple `class_attribute` here so we don't need to
# inject any modules and/or expose unnecessary methods.
@model.class_attribute(:connection)
@model.class_attribute(:sticking)
@model.connection = ConnectionProxy.new(lb)
@model.sticking = Sticking.new(lb)
end
def setup_service_discovery
return unless configuration.service_discovery_enabled?
lb = @model.connection.load_balancer
sv = ServiceDiscovery.new(lb, **configuration.service_discovery)
sv.perform_service_discovery
sv.start if @start_service_discovery
end
end
end
end
end
...@@ -30,26 +30,23 @@ module Gitlab ...@@ -30,26 +30,23 @@ module Gitlab
end end
def set_data_consistency_locations!(job) def set_data_consistency_locations!(job)
# Once we add support for multiple databases to our load balancer, we would use something like this: locations = {}
# job['wal_locations'] = Gitlab::Database.databases.transform_values do |connection|
# connection.load_balancer.primary_write_location
# end
#
job['wal_locations'] = { ::Gitlab::Database::MAIN_DATABASE_NAME.to_sym => wal_location } if wal_location
end
def wal_location ::Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
strong_memoize(:wal_location) do if (location = wal_location_for(lb))
if ::Gitlab::Database::LoadBalancing::Session.current.use_primary? locations[lb.name] = location
load_balancer.primary_write_location
else
load_balancer.host.database_replica_location
end end
end end
job['wal_locations'] = locations
end end
def load_balancer def wal_location_for(load_balancer)
::Gitlab::Database::LoadBalancing.proxy.load_balancer if ::Gitlab::Database::LoadBalancing::Session.current.use_primary?
load_balancer.primary_write_location
else
load_balancer.host.database_replica_location
end
end end
end end
end end
......
...@@ -29,7 +29,7 @@ module Gitlab ...@@ -29,7 +29,7 @@ module Gitlab
private private
def clear def clear
release_hosts LoadBalancing.release_hosts
Session.clear_session Session.clear_session
end end
...@@ -44,7 +44,7 @@ module Gitlab ...@@ -44,7 +44,7 @@ module Gitlab
return :primary_no_wal unless wal_locations return :primary_no_wal unless wal_locations
if all_databases_has_replica_caught_up?(wal_locations) if databases_in_sync?(wal_locations)
# Happy case: we can read from a replica. # Happy case: we can read from a replica.
retried_before?(worker_class, job) ? :replica_retried : :replica retried_before?(worker_class, job) ? :replica_retried : :replica
elsif can_retry?(worker_class, job) elsif can_retry?(worker_class, job)
...@@ -89,27 +89,18 @@ module Gitlab ...@@ -89,27 +89,18 @@ module Gitlab
job['retry_count'].nil? job['retry_count'].nil?
end end
def all_databases_has_replica_caught_up?(wal_locations) def databases_in_sync?(wal_locations)
wal_locations.all? do |_config_name, location| LoadBalancing.each_load_balancer.all? do |lb|
# Once we add support for multiple databases to our load balancer, we would use something like this: if (location = wal_locations[lb.name])
# Gitlab::Database.databases[config_name].load_balancer.select_up_to_date_host(location) lb.select_up_to_date_host(location)
load_balancer.select_up_to_date_host(location) else
# If there's no entry for a load balancer it means the Sidekiq
# job doesn't care for it. In this case we'll treat the load
# balancer as being in sync.
true
end
end end
end end
def release_hosts
# Once we add support for multiple databases to our load balancer, we would use something like this:
# connection.load_balancer.primary_write_location
#
# Gitlab::Database.databases.values.each do |connection|
# connection.load_balancer.release_host
# end
load_balancer.release_host
end
def load_balancer
LoadBalancing.proxy.load_balancer
end
end end
end end
end end
......
...@@ -5,34 +5,47 @@ module Gitlab ...@@ -5,34 +5,47 @@ module Gitlab
module LoadBalancing module LoadBalancing
# Module used for handling sticking connections to a primary, if # Module used for handling sticking connections to a primary, if
# necessary. # necessary.
# class Sticking
# ## Examples
#
# Sticking a user to the primary:
#
# Sticking.stick_if_necessary(:user, current_user.id)
#
# To unstick if possible, or continue using the primary otherwise:
#
# Sticking.unstick_or_continue_sticking(:user, current_user.id)
module Sticking
# The number of seconds after which a session should stop reading from # The number of seconds after which a session should stop reading from
# the primary. # the primary.
EXPIRATION = 30 EXPIRATION = 30
def initialize(load_balancer)
@load_balancer = load_balancer
@model = load_balancer.configuration.model
end
# Unsticks or continues sticking the current request.
#
# This method also updates the Rack environment so #call can later
# determine if we still need to stick or not.
#
# env - The Rack environment.
# namespace - The namespace to use for sticking.
# id - The identifier to use for sticking.
# model - The ActiveRecord model to scope sticking to.
def stick_or_unstick_request(env, namespace, id)
unstick_or_continue_sticking(namespace, id)
env[RackMiddleware::STICK_OBJECT] ||= Set.new
env[RackMiddleware::STICK_OBJECT] << [@model, namespace, id]
end
# Sticks to the primary if a write was performed. # Sticks to the primary if a write was performed.
def self.stick_if_necessary(namespace, id) def stick_if_necessary(namespace, id)
stick(namespace, id) if Session.current.performed_write? stick(namespace, id) if Session.current.performed_write?
end end
# Checks if we are caught-up with all the work def all_caught_up?(namespace, id)
def self.all_caught_up?(namespace, id)
location = last_write_location_for(namespace, id) location = last_write_location_for(namespace, id)
return true unless location return true unless location
load_balancer.select_up_to_date_host(location).tap do |found| @load_balancer.select_up_to_date_host(location).tap do |found|
ActiveSupport::Notifications.instrument('caught_up_replica_pick.load_balancing', { result: found } ) ActiveSupport::Notifications.instrument(
'caught_up_replica_pick.load_balancing',
{ result: found }
)
unstick(namespace, id) if found unstick(namespace, id) if found
end end
...@@ -43,7 +56,7 @@ module Gitlab ...@@ -43,7 +56,7 @@ module Gitlab
# in another thread. # in another thread.
# #
# Returns true if one host was selected. # Returns true if one host was selected.
def self.select_caught_up_replicas(namespace, id) def select_caught_up_replicas(namespace, id)
location = last_write_location_for(namespace, id) location = last_write_location_for(namespace, id)
# Unlike all_caught_up?, we return false if no write location exists. # Unlike all_caught_up?, we return false if no write location exists.
...@@ -51,33 +64,36 @@ module Gitlab ...@@ -51,33 +64,36 @@ module Gitlab
# write location. If no such location exists, err on the side of caution. # write location. If no such location exists, err on the side of caution.
return false unless location return false unless location
load_balancer.select_up_to_date_host(location).tap do |selected| @load_balancer.select_up_to_date_host(location).tap do |selected|
unstick(namespace, id) if selected unstick(namespace, id) if selected
end end
end end
# Sticks to the primary if necessary, otherwise unsticks an object (if # Sticks to the primary if necessary, otherwise unsticks an object (if
# it was previously stuck to the primary). # it was previously stuck to the primary).
def self.unstick_or_continue_sticking(namespace, id) def unstick_or_continue_sticking(namespace, id)
Session.current.use_primary! unless all_caught_up?(namespace, id) return if all_caught_up?(namespace, id)
Session.current.use_primary!
end end
# Select a replica that has caught up with the primary. If one has not been # Select a replica that has caught up with the primary. If one has not been
# found, stick to the primary. # found, stick to the primary.
def self.select_valid_host(namespace, id) def select_valid_host(namespace, id)
replica_selected = select_caught_up_replicas(namespace, id) replica_selected =
select_caught_up_replicas(namespace, id)
Session.current.use_primary! unless replica_selected Session.current.use_primary! unless replica_selected
end end
# Starts sticking to the primary for the given namespace and id, using # Starts sticking to the primary for the given namespace and id, using
# the latest WAL pointer from the primary. # the latest WAL pointer from the primary.
def self.stick(namespace, id) def stick(namespace, id)
mark_primary_write_location(namespace, id) mark_primary_write_location(namespace, id)
Session.current.use_primary! Session.current.use_primary!
end end
def self.bulk_stick(namespace, ids) def bulk_stick(namespace, ids)
with_primary_write_location do |location| with_primary_write_location do |location|
ids.each do |id| ids.each do |id|
set_write_location_for(namespace, id, location) set_write_location_for(namespace, id, location)
...@@ -87,45 +103,49 @@ module Gitlab ...@@ -87,45 +103,49 @@ module Gitlab
Session.current.use_primary! Session.current.use_primary!
end end
def self.with_primary_write_location def with_primary_write_location
location = load_balancer.primary_write_location location = @load_balancer.primary_write_location
return if location.blank? return if location.blank?
yield(location) yield(location)
end end
def self.mark_primary_write_location(namespace, id) def mark_primary_write_location(namespace, id)
with_primary_write_location do |location| with_primary_write_location do |location|
set_write_location_for(namespace, id, location) set_write_location_for(namespace, id, location)
end end
end end
# Stops sticking to the primary. def unstick(namespace, id)
def self.unstick(namespace, id)
Gitlab::Redis::SharedState.with do |redis| Gitlab::Redis::SharedState.with do |redis|
redis.del(redis_key_for(namespace, id)) redis.del(redis_key_for(namespace, id))
redis.del(old_redis_key_for(namespace, id))
end end
end end
def self.set_write_location_for(namespace, id, location) def set_write_location_for(namespace, id, location)
Gitlab::Redis::SharedState.with do |redis| Gitlab::Redis::SharedState.with do |redis|
redis.set(redis_key_for(namespace, id), location, ex: EXPIRATION) redis.set(redis_key_for(namespace, id), location, ex: EXPIRATION)
redis.set(old_redis_key_for(namespace, id), location, ex: EXPIRATION)
end end
end end
def self.last_write_location_for(namespace, id) def last_write_location_for(namespace, id)
Gitlab::Redis::SharedState.with do |redis| Gitlab::Redis::SharedState.with do |redis|
redis.get(redis_key_for(namespace, id)) redis.get(redis_key_for(namespace, id)) ||
redis.get(old_redis_key_for(namespace, id))
end end
end end
def self.redis_key_for(namespace, id) def redis_key_for(namespace, id)
"database-load-balancing/write-location/#{namespace}/#{id}" name = @load_balancer.name
"database-load-balancing/write-location/#{name}/#{namespace}/#{id}"
end end
def self.load_balancer def old_redis_key_for(namespace, id)
LoadBalancing.proxy.load_balancer "database-load-balancing/write-location/#{namespace}/#{id}"
end end
end end
end end
......
...@@ -15,8 +15,8 @@ RSpec.describe API::Ci::Helpers::Runner do ...@@ -15,8 +15,8 @@ RSpec.describe API::Ci::Helpers::Runner do
it 'handles sticking of a build when a build ID is specified' do it 'handles sticking of a build when a build ID is specified' do
allow(helper).to receive(:params).and_return(id: build.id) allow(helper).to receive(:params).and_return(id: build.id)
expect(Gitlab::Database::LoadBalancing::RackMiddleware) expect(ApplicationRecord.sticking)
.to receive(:stick_or_unstick) .to receive(:stick_or_unstick_request)
.with({}, :build, build.id) .with({}, :build, build.id)
helper.current_job helper.current_job
...@@ -25,8 +25,8 @@ RSpec.describe API::Ci::Helpers::Runner do ...@@ -25,8 +25,8 @@ RSpec.describe API::Ci::Helpers::Runner do
it 'does not handle sticking if no build ID was specified' do it 'does not handle sticking if no build ID was specified' do
allow(helper).to receive(:params).and_return({}) allow(helper).to receive(:params).and_return({})
expect(Gitlab::Database::LoadBalancing::RackMiddleware) expect(ApplicationRecord.sticking)
.not_to receive(:stick_or_unstick) .not_to receive(:stick_or_unstick_request)
helper.current_job helper.current_job
end end
...@@ -44,8 +44,8 @@ RSpec.describe API::Ci::Helpers::Runner do ...@@ -44,8 +44,8 @@ RSpec.describe API::Ci::Helpers::Runner do
it 'handles sticking of a runner if a token is specified' do it 'handles sticking of a runner if a token is specified' do
allow(helper).to receive(:params).and_return(token: runner.token) allow(helper).to receive(:params).and_return(token: runner.token)
expect(Gitlab::Database::LoadBalancing::RackMiddleware) expect(ApplicationRecord.sticking)
.to receive(:stick_or_unstick) .to receive(:stick_or_unstick_request)
.with({}, :runner, runner.token) .with({}, :runner, runner.token)
helper.current_runner helper.current_runner
...@@ -54,8 +54,8 @@ RSpec.describe API::Ci::Helpers::Runner do ...@@ -54,8 +54,8 @@ RSpec.describe API::Ci::Helpers::Runner do
it 'does not handle sticking if no token was specified' do it 'does not handle sticking if no token was specified' do
allow(helper).to receive(:params).and_return({}) allow(helper).to receive(:params).and_return({})
expect(Gitlab::Database::LoadBalancing::RackMiddleware) expect(ApplicationRecord.sticking)
.not_to receive(:stick_or_unstick) .not_to receive(:stick_or_unstick_request)
helper.current_runner helper.current_runner
end end
......
...@@ -35,8 +35,8 @@ RSpec.describe API::Helpers do ...@@ -35,8 +35,8 @@ RSpec.describe API::Helpers do
it 'handles sticking when a user could be found' do it 'handles sticking when a user could be found' do
allow_any_instance_of(API::Helpers).to receive(:initial_current_user).and_return(user) allow_any_instance_of(API::Helpers).to receive(:initial_current_user).and_return(user)
expect(Gitlab::Database::LoadBalancing::RackMiddleware) expect(ApplicationRecord.sticking)
.to receive(:stick_or_unstick).with(any_args, :user, 42) .to receive(:stick_or_unstick_request).with(any_args, :user, 42)
get 'user' get 'user'
...@@ -46,8 +46,8 @@ RSpec.describe API::Helpers do ...@@ -46,8 +46,8 @@ RSpec.describe API::Helpers do
it 'does not handle sticking if no user could be found' do it 'does not handle sticking if no user could be found' do
allow_any_instance_of(API::Helpers).to receive(:initial_current_user).and_return(nil) allow_any_instance_of(API::Helpers).to receive(:initial_current_user).and_return(nil)
expect(Gitlab::Database::LoadBalancing::RackMiddleware) expect(ApplicationRecord.sticking)
.not_to receive(:stick_or_unstick) .not_to receive(:stick_or_unstick_request)
get 'user' get 'user'
......
...@@ -37,10 +37,20 @@ RSpec.describe Gitlab::Checks::MatchingMergeRequest do ...@@ -37,10 +37,20 @@ RSpec.describe Gitlab::Checks::MatchingMergeRequest do
before do before do
Gitlab::Database::LoadBalancing::Session.clear_session Gitlab::Database::LoadBalancing::Session.clear_session
allow(::Gitlab::Database::LoadBalancing::Sticking).to receive(:all_caught_up?).and_return(all_caught_up)
expect(::Gitlab::Database::LoadBalancing::Sticking).to receive(:select_valid_host).with(:project, project.id).and_call_original allow(::ApplicationRecord.sticking)
allow(::Gitlab::Database::LoadBalancing::Sticking).to receive(:select_caught_up_replicas).with(:project, project.id).and_return(all_caught_up) .to receive(:all_caught_up?)
.and_return(all_caught_up)
expect(::ApplicationRecord.sticking)
.to receive(:select_valid_host)
.with(:project, project.id)
.and_call_original
allow(::ApplicationRecord.sticking)
.to receive(:select_caught_up_replicas)
.with(:project, project.id)
.and_return(all_caught_up)
end end
after do after do
......
...@@ -7,7 +7,15 @@ RSpec.describe Gitlab::Database::Consistency do ...@@ -7,7 +7,15 @@ RSpec.describe Gitlab::Database::Consistency do
Gitlab::Database::LoadBalancing::Session.current Gitlab::Database::LoadBalancing::Session.current
end end
describe '.with_read_consistency', :db_load_balancing do before do
Gitlab::Database::LoadBalancing::Session.clear_session
end
after do
Gitlab::Database::LoadBalancing::Session.clear_session
end
describe '.with_read_consistency' do
it 'sticks to primary database' do it 'sticks to primary database' do
expect(session).not_to be_using_primary expect(session).not_to be_using_primary
......
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe Gitlab::Database::LoadBalancing::ActionCableCallbacks, :request_store do RSpec.describe Gitlab::Database::LoadBalancing::ActionCableCallbacks, :request_store do
describe '.wrapper' do describe '.wrapper' do
it 'uses primary and then releases the connection and clears the session' do it 'uses primary and then releases the connection and clears the session' do
expect(Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer, :release_host) expect(Gitlab::Database::LoadBalancing).to receive(:release_hosts)
expect(Gitlab::Database::LoadBalancing::Session).to receive(:clear_session) expect(Gitlab::Database::LoadBalancing::Session).to receive(:clear_session)
described_class.wrapper.call( described_class.wrapper.call(
...@@ -18,7 +18,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ActionCableCallbacks, :request_s ...@@ -18,7 +18,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ActionCableCallbacks, :request_s
context 'with an exception' do context 'with an exception' do
it 'releases the connection and clears the session' do it 'releases the connection and clears the session' do
expect(Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer, :release_host) expect(Gitlab::Database::LoadBalancing).to receive(:release_hosts)
expect(Gitlab::Database::LoadBalancing::Session).to receive(:clear_session) expect(Gitlab::Database::LoadBalancing::Session).to receive(:clear_session)
expect do expect do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Database::LoadBalancing::ActiveRecordProxy do
describe '#connection' do
it 'returns a connection proxy' do
dummy = Class.new do
include Gitlab::Database::LoadBalancing::ActiveRecordProxy
end
proxy = double(:proxy)
expect(Gitlab::Database::LoadBalancing).to receive(:proxy)
.and_return(proxy)
expect(dummy.new.connection).to eq(proxy)
end
it 'returns a connection when no proxy is present' do
allow(Gitlab::Database::LoadBalancing).to receive(:proxy).and_return(nil)
expect(ActiveRecord::Base.connection)
.to eq(ActiveRecord::Base.retrieve_connection)
end
end
end
...@@ -108,6 +108,14 @@ RSpec.describe Gitlab::Database::LoadBalancing::Configuration do ...@@ -108,6 +108,14 @@ RSpec.describe Gitlab::Database::LoadBalancing::Configuration do
end end
describe '#load_balancing_enabled?' do describe '#load_balancing_enabled?' do
it 'returns false when running inside a Rake task' do
config = described_class.new(ActiveRecord::Base, %w[foo bar])
allow(Gitlab::Runtime).to receive(:rake?).and_return(true)
expect(config.load_balancing_enabled?).to eq(false)
end
it 'returns true when hosts are configured' do it 'returns true when hosts are configured' do
config = described_class.new(ActiveRecord::Base, %w[foo bar]) config = described_class.new(ActiveRecord::Base, %w[foo bar])
......
...@@ -47,16 +47,27 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do ...@@ -47,16 +47,27 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
end end
describe '#initialize' do describe '#initialize' do
it 'ignores the hosts when the primary_only option is enabled' do it 'ignores the hosts when load balancing is disabled' do
config = Gitlab::Database::LoadBalancing::Configuration config = Gitlab::Database::LoadBalancing::Configuration
.new(ActiveRecord::Base, [db_host]) .new(ActiveRecord::Base, [db_host])
lb = described_class.new(config, primary_only: true)
allow(config).to receive(:load_balancing_enabled?).and_return(false)
lb = described_class.new(config)
hosts = lb.host_list.hosts hosts = lb.host_list.hosts
expect(hosts.length).to eq(1) expect(hosts.length).to eq(1)
expect(hosts.first) expect(hosts.first)
.to be_instance_of(Gitlab::Database::LoadBalancing::PrimaryHost) .to be_instance_of(Gitlab::Database::LoadBalancing::PrimaryHost)
end end
it 'sets the name of the connection that is used' do
config =
Gitlab::Database::LoadBalancing::Configuration.new(ActiveRecord::Base)
lb = described_class.new(config)
expect(lb.name).to eq(:main)
end
end end
describe '#read' do describe '#read' do
...@@ -140,10 +151,13 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do ...@@ -140,10 +151,13 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
.to yield_with_args(ActiveRecord::Base.retrieve_connection) .to yield_with_args(ActiveRecord::Base.retrieve_connection)
end end
it 'uses the primary when the primary_only option is enabled' do it 'uses the primary when load balancing is disabled' do
config = Gitlab::Database::LoadBalancing::Configuration config = Gitlab::Database::LoadBalancing::Configuration
.new(ActiveRecord::Base) .new(ActiveRecord::Base)
lb = described_class.new(config, primary_only: true)
allow(config).to receive(:load_balancing_enabled?).and_return(false)
lb = described_class.new(config)
# When no hosts are configured, we don't want to produce any warnings, as # When no hosts are configured, we don't want to produce any warnings, as
# they aren't useful/too noisy. # they aren't useful/too noisy.
......
...@@ -6,12 +6,12 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -6,12 +6,12 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
let(:app) { double(:app) } let(:app) { double(:app) }
let(:middleware) { described_class.new(app) } let(:middleware) { described_class.new(app) }
let(:warden_user) { double(:warden, user: double(:user, id: 42)) } let(:warden_user) { double(:warden, user: double(:user, id: 42)) }
let(:single_sticking_object) { Set.new([[:user, 42]]) } let(:single_sticking_object) { Set.new([[ActiveRecord::Base, :user, 42]]) }
let(:multiple_sticking_objects) do let(:multiple_sticking_objects) do
Set.new([ Set.new([
[:user, 42], [ActiveRecord::Base, :user, 42],
[:runner, '123456789'], [ActiveRecord::Base, :runner, '123456789'],
[:runner, '1234'] [ActiveRecord::Base, :runner, '1234']
]) ])
end end
...@@ -19,42 +19,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -19,42 +19,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
Gitlab::Database::LoadBalancing::Session.clear_session Gitlab::Database::LoadBalancing::Session.clear_session
end end
describe '.stick_or_unstick' do
it 'sticks or unsticks a single object and updates the Rack environment' do
expect(Gitlab::Database::LoadBalancing::Sticking)
.to receive(:unstick_or_continue_sticking)
.with(:user, 42)
env = {}
described_class.stick_or_unstick(env, :user, 42)
expect(env[described_class::STICK_OBJECT].to_a).to eq([[:user, 42]])
end
it 'sticks or unsticks multiple objects and updates the Rack environment' do
expect(Gitlab::Database::LoadBalancing::Sticking)
.to receive(:unstick_or_continue_sticking)
.with(:user, 42)
.ordered
expect(Gitlab::Database::LoadBalancing::Sticking)
.to receive(:unstick_or_continue_sticking)
.with(:runner, '123456789')
.ordered
env = {}
described_class.stick_or_unstick(env, :user, 42)
described_class.stick_or_unstick(env, :runner, '123456789')
expect(env[described_class::STICK_OBJECT].to_a).to eq([
[:user, 42],
[:runner, '123456789']
])
end
end
describe '#call' do describe '#call' do
it 'handles a request' do it 'handles a request' do
env = {} env = {}
...@@ -77,7 +41,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -77,7 +41,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
describe '#unstick_or_continue_sticking' do describe '#unstick_or_continue_sticking' do
it 'does not stick if no namespace and identifier could be found' do it 'does not stick if no namespace and identifier could be found' do
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.not_to receive(:unstick_or_continue_sticking) .not_to receive(:unstick_or_continue_sticking)
middleware.unstick_or_continue_sticking({}) middleware.unstick_or_continue_sticking({})
...@@ -86,9 +50,11 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -86,9 +50,11 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
it 'sticks to the primary if a warden user is found' do it 'sticks to the primary if a warden user is found' do
env = { 'warden' => warden_user } env = { 'warden' => warden_user }
expect(Gitlab::Database::LoadBalancing::Sticking) Gitlab::Database::LoadBalancing.base_models.each do |model|
.to receive(:unstick_or_continue_sticking) expect(model.sticking)
.with(:user, 42) .to receive(:unstick_or_continue_sticking)
.with(:user, 42)
end
middleware.unstick_or_continue_sticking(env) middleware.unstick_or_continue_sticking(env)
end end
...@@ -96,7 +62,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -96,7 +62,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
it 'sticks to the primary if a sticking namespace and identifier is found' do it 'sticks to the primary if a sticking namespace and identifier is found' do
env = { described_class::STICK_OBJECT => single_sticking_object } env = { described_class::STICK_OBJECT => single_sticking_object }
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.to receive(:unstick_or_continue_sticking) .to receive(:unstick_or_continue_sticking)
.with(:user, 42) .with(:user, 42)
...@@ -106,17 +72,17 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -106,17 +72,17 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
it 'sticks to the primary if multiple sticking namespaces and identifiers were found' do it 'sticks to the primary if multiple sticking namespaces and identifiers were found' do
env = { described_class::STICK_OBJECT => multiple_sticking_objects } env = { described_class::STICK_OBJECT => multiple_sticking_objects }
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.to receive(:unstick_or_continue_sticking) .to receive(:unstick_or_continue_sticking)
.with(:user, 42) .with(:user, 42)
.ordered .ordered
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.to receive(:unstick_or_continue_sticking) .to receive(:unstick_or_continue_sticking)
.with(:runner, '123456789') .with(:runner, '123456789')
.ordered .ordered
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.to receive(:unstick_or_continue_sticking) .to receive(:unstick_or_continue_sticking)
.with(:runner, '1234') .with(:runner, '1234')
.ordered .ordered
...@@ -127,7 +93,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -127,7 +93,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
describe '#stick_if_necessary' do describe '#stick_if_necessary' do
it 'does not stick to the primary if not necessary' do it 'does not stick to the primary if not necessary' do
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.not_to receive(:stick_if_necessary) .not_to receive(:stick_if_necessary)
middleware.stick_if_necessary({}) middleware.stick_if_necessary({})
...@@ -136,9 +102,11 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -136,9 +102,11 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
it 'sticks to the primary if a warden user is found' do it 'sticks to the primary if a warden user is found' do
env = { 'warden' => warden_user } env = { 'warden' => warden_user }
expect(Gitlab::Database::LoadBalancing::Sticking) Gitlab::Database::LoadBalancing.base_models.each do |model|
.to receive(:stick_if_necessary) expect(model.sticking)
.with(:user, 42) .to receive(:stick_if_necessary)
.with(:user, 42)
end
middleware.stick_if_necessary(env) middleware.stick_if_necessary(env)
end end
...@@ -146,7 +114,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -146,7 +114,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
it 'sticks to the primary if a a single sticking object is found' do it 'sticks to the primary if a a single sticking object is found' do
env = { described_class::STICK_OBJECT => single_sticking_object } env = { described_class::STICK_OBJECT => single_sticking_object }
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.to receive(:stick_if_necessary) .to receive(:stick_if_necessary)
.with(:user, 42) .with(:user, 42)
...@@ -156,17 +124,17 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -156,17 +124,17 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
it 'sticks to the primary if multiple sticking namespaces and identifiers were found' do it 'sticks to the primary if multiple sticking namespaces and identifiers were found' do
env = { described_class::STICK_OBJECT => multiple_sticking_objects } env = { described_class::STICK_OBJECT => multiple_sticking_objects }
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.to receive(:stick_if_necessary) .to receive(:stick_if_necessary)
.with(:user, 42) .with(:user, 42)
.ordered .ordered
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.to receive(:stick_if_necessary) .to receive(:stick_if_necessary)
.with(:runner, '123456789') .with(:runner, '123456789')
.ordered .ordered
expect(Gitlab::Database::LoadBalancing::Sticking) expect(ApplicationRecord.sticking)
.to receive(:stick_if_necessary) .to receive(:stick_if_necessary)
.with(:runner, '1234') .with(:runner, '1234')
.ordered .ordered
...@@ -177,47 +145,34 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -177,47 +145,34 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
describe '#clear' do describe '#clear' do
it 'clears the currently used host and session' do it 'clears the currently used host and session' do
lb = double(:lb)
session = spy(:session) session = spy(:session)
allow(middleware).to receive(:load_balancer).and_return(lb)
expect(lb).to receive(:release_host)
stub_const('Gitlab::Database::LoadBalancing::Session', session) stub_const('Gitlab::Database::LoadBalancing::Session', session)
expect(Gitlab::Database::LoadBalancing).to receive(:release_hosts)
middleware.clear middleware.clear
expect(session).to have_received(:clear_session) expect(session).to have_received(:clear_session)
end end
end end
describe '.load_balancer' do describe '#sticking_namespaces' do
it 'returns a the load balancer' do
proxy = double(:proxy)
expect(Gitlab::Database::LoadBalancing).to receive(:proxy)
.and_return(proxy)
expect(proxy).to receive(:load_balancer)
middleware.load_balancer
end
end
describe '#sticking_namespaces_and_ids' do
context 'using a Warden request' do context 'using a Warden request' do
it 'returns the warden user if present' do it 'returns the warden user if present' do
env = { 'warden' => warden_user } env = { 'warden' => warden_user }
ids = Gitlab::Database::LoadBalancing.base_models.map do |model|
[model, :user, 42]
end
expect(middleware.sticking_namespaces_and_ids(env)).to eq([[:user, 42]]) expect(middleware.sticking_namespaces(env)).to eq(ids)
end end
it 'returns an empty Array if no user was present' do it 'returns an empty Array if no user was present' do
warden = double(:warden, user: nil) warden = double(:warden, user: nil)
env = { 'warden' => warden } env = { 'warden' => warden }
expect(middleware.sticking_namespaces_and_ids(env)).to eq([]) expect(middleware.sticking_namespaces(env)).to eq([])
end end
end end
...@@ -225,17 +180,17 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -225,17 +180,17 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
it 'returns the sticking object' do it 'returns the sticking object' do
env = { described_class::STICK_OBJECT => multiple_sticking_objects } env = { described_class::STICK_OBJECT => multiple_sticking_objects }
expect(middleware.sticking_namespaces_and_ids(env)).to eq([ expect(middleware.sticking_namespaces(env)).to eq([
[:user, 42], [ActiveRecord::Base, :user, 42],
[:runner, '123456789'], [ActiveRecord::Base, :runner, '123456789'],
[:runner, '1234'] [ActiveRecord::Base, :runner, '1234']
]) ])
end end
end end
context 'using a regular request' do context 'using a regular request' do
it 'returns an empty Array' do it 'returns an empty Array' do
expect(middleware.sticking_namespaces_and_ids({})).to eq([]) expect(middleware.sticking_namespaces({})).to eq([])
end end
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Database::LoadBalancing::Setup do
describe '#setup' do
it 'sets up the load balancer' do
setup = described_class.new(ActiveRecord::Base)
expect(setup).to receive(:disable_prepared_statements)
expect(setup).to receive(:setup_load_balancer)
expect(setup).to receive(:setup_service_discovery)
setup.setup
end
end
describe '#disable_prepared_statements' do
it 'disables prepared statements and reconnects to the database' do
config = double(
:config,
configuration_hash: { host: 'localhost' },
env_name: 'test',
name: 'main'
)
model = double(:model, connection_db_config: config)
expect(ActiveRecord::DatabaseConfigurations::HashConfig)
.to receive(:new)
.with('test', 'main', { host: 'localhost', prepared_statements: false })
.and_call_original
# HashConfig doesn't implement its own #==, so we can't directly compare
# the expected value with a pre-defined one.
expect(model)
.to receive(:establish_connection)
.with(an_instance_of(ActiveRecord::DatabaseConfigurations::HashConfig))
described_class.new(model).disable_prepared_statements
end
end
describe '#setup_load_balancer' do
it 'sets up the load balancer' do
model = Class.new(ActiveRecord::Base)
setup = described_class.new(model)
config = Gitlab::Database::LoadBalancing::Configuration.new(model)
lb = instance_spy(Gitlab::Database::LoadBalancing::LoadBalancer)
allow(lb).to receive(:configuration).and_return(config)
expect(Gitlab::Database::LoadBalancing::LoadBalancer)
.to receive(:new)
.with(setup.configuration)
.and_return(lb)
setup.setup_load_balancer
expect(model.connection.load_balancer).to eq(lb)
expect(model.sticking)
.to be_an_instance_of(Gitlab::Database::LoadBalancing::Sticking)
end
end
describe '#setup_service_discovery' do
context 'when service discovery is disabled' do
it 'does nothing' do
expect(Gitlab::Database::LoadBalancing::ServiceDiscovery)
.not_to receive(:new)
described_class.new(ActiveRecord::Base).setup_service_discovery
end
end
context 'when service discovery is enabled' do
it 'immediately performs service discovery' do
model = ActiveRecord::Base
setup = described_class.new(model)
sv = instance_spy(Gitlab::Database::LoadBalancing::ServiceDiscovery)
lb = model.connection.load_balancer
allow(setup.configuration)
.to receive(:service_discovery_enabled?)
.and_return(true)
allow(Gitlab::Database::LoadBalancing::ServiceDiscovery)
.to receive(:new)
.with(lb, setup.configuration.service_discovery)
.and_return(sv)
expect(sv).to receive(:perform_service_discovery)
expect(sv).not_to receive(:start)
setup.setup_service_discovery
end
it 'starts service discovery if needed' do
model = ActiveRecord::Base
setup = described_class.new(model, start_service_discovery: true)
sv = instance_spy(Gitlab::Database::LoadBalancing::ServiceDiscovery)
lb = model.connection.load_balancer
allow(setup.configuration)
.to receive(:service_discovery_enabled?)
.and_return(true)
allow(Gitlab::Database::LoadBalancing::ServiceDiscovery)
.to receive(:new)
.with(lb, setup.configuration.service_discovery)
.and_return(sv)
expect(sv).to receive(:perform_service_discovery)
expect(sv).to receive(:start)
setup.setup_service_discovery
end
end
end
end
...@@ -5,7 +5,6 @@ require 'spec_helper' ...@@ -5,7 +5,6 @@ require 'spec_helper'
RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do
let(:middleware) { described_class.new } let(:middleware) { described_class.new }
let(:load_balancer) { Gitlab::Database::LoadBalancing.proxy.load_balancer }
let(:worker_class) { 'TestDataConsistencyWorker' } let(:worker_class) { 'TestDataConsistencyWorker' }
let(:job) { { "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e" } } let(:job) { { "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e" } }
...@@ -84,9 +83,15 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do ...@@ -84,9 +83,15 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do
end end
it 'passes database_replica_location' do it 'passes database_replica_location' do
expected_location = { Gitlab::Database::MAIN_DATABASE_NAME.to_sym => location } expected_location = {}
expect(load_balancer).to receive_message_chain(:host, "database_replica_location").and_return(location) Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
expect(lb.host)
.to receive(:database_replica_location)
.and_return(location)
expected_location[lb.name] = location
end
run_middleware run_middleware
...@@ -102,9 +107,15 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do ...@@ -102,9 +107,15 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do
end end
it 'passes primary write location', :aggregate_failures do it 'passes primary write location', :aggregate_failures do
expected_location = { Gitlab::Database::MAIN_DATABASE_NAME.to_sym => location } expected_location = {}
expect(load_balancer).to receive(:primary_write_location).and_return(location) Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
expect(lb)
.to receive(:primary_write_location)
.and_return(location)
expected_location[lb.name] = location
end
run_middleware run_middleware
...@@ -136,8 +147,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do ...@@ -136,8 +147,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqClientMiddleware do
let(:job) { { "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'wal_locations' => wal_locations } } let(:job) { { "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'wal_locations' => wal_locations } }
before do before do
allow(load_balancer).to receive(:primary_write_location).and_return(new_location) Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
allow(load_balancer).to receive(:database_replica_location).and_return(new_location) allow(lb).to receive(:primary_write_location).and_return(new_location)
allow(lb).to receive(:database_replica_location).and_return(new_location)
end
end end
shared_examples_for 'does not set database location again' do |use_primary| shared_examples_for 'does not set database location again' do |use_primary|
......
...@@ -4,9 +4,6 @@ require 'spec_helper' ...@@ -4,9 +4,6 @@ require 'spec_helper'
RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_gitlab_redis_queues do RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_gitlab_redis_queues do
let(:middleware) { described_class.new } let(:middleware) { described_class.new }
let(:load_balancer) { Gitlab::Database::LoadBalancing.proxy.load_balancer }
let(:worker) { worker_class.new } let(:worker) { worker_class.new }
let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'database_replica_location' => '0/D525E3A8' } } let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'database_replica_location' => '0/D525E3A8' } }
...@@ -15,6 +12,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ ...@@ -15,6 +12,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
skip_default_enabled_yaml_check skip_default_enabled_yaml_check
replication_lag!(false) replication_lag!(false)
Gitlab::Database::LoadBalancing::Session.clear_session
end end
after do after do
...@@ -66,7 +64,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ ...@@ -66,7 +64,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
let(:wal_locations) { { Gitlab::Database::MAIN_DATABASE_NAME.to_sym => location } } let(:wal_locations) { { Gitlab::Database::MAIN_DATABASE_NAME.to_sym => location } }
it 'does not stick to the primary', :aggregate_failures do it 'does not stick to the primary', :aggregate_failures do
expect(load_balancer).to receive(:select_up_to_date_host).with(location).and_return(true) expect(ActiveRecord::Base.connection.load_balancer)
.to receive(:select_up_to_date_host)
.with(location)
.and_return(true)
run_middleware do run_middleware do
expect(Gitlab::Database::LoadBalancing::Session.current.use_primary?).not_to be_truthy expect(Gitlab::Database::LoadBalancing::Session.current.use_primary?).not_to be_truthy
...@@ -91,7 +92,12 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ ...@@ -91,7 +92,12 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'wal_locations' => wal_locations } } let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'wal_locations' => wal_locations } }
before do before do
allow(load_balancer).to receive(:select_up_to_date_host).with(location).and_return(true) Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
allow(lb)
.to receive(:select_up_to_date_host)
.with(location)
.and_return(true)
end
end end
it_behaves_like 'replica is up to date', 'replica' it_behaves_like 'replica is up to date', 'replica'
...@@ -101,7 +107,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ ...@@ -101,7 +107,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'dedup_wal_locations' => wal_locations } } let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'dedup_wal_locations' => wal_locations } }
before do before do
allow(load_balancer).to receive(:select_up_to_date_host).with(wal_locations[:main]).and_return(true) allow(ActiveRecord::Base.connection.load_balancer)
.to receive(:select_up_to_date_host)
.with(wal_locations[:main])
.and_return(true)
end end
it_behaves_like 'replica is up to date', 'replica' it_behaves_like 'replica is up to date', 'replica'
...@@ -111,7 +120,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ ...@@ -111,7 +120,10 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'database_write_location' => '0/D525E3A8' } } let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'database_write_location' => '0/D525E3A8' } }
before do before do
allow(load_balancer).to receive(:select_up_to_date_host).with('0/D525E3A8').and_return(true) allow(ActiveRecord::Base.connection.load_balancer)
.to receive(:select_up_to_date_host)
.with('0/D525E3A8')
.and_return(true)
end end
it_behaves_like 'replica is up to date', 'replica' it_behaves_like 'replica is up to date', 'replica'
...@@ -187,7 +199,9 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ ...@@ -187,7 +199,9 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
context 'when replica is not up to date' do context 'when replica is not up to date' do
before do before do
allow(load_balancer).to receive(:select_up_to_date_host).and_return(false) Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
allow(lb).to receive(:select_up_to_date_host).and_return(false)
end
end end
include_examples 'stick to the primary', 'primary' include_examples 'stick to the primary', 'primary'
...@@ -195,6 +209,45 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ ...@@ -195,6 +209,45 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
end end
end end
describe '#databases_in_sync?' do
it 'treats load balancers without WAL entries as in sync' do
expect(middleware.send(:databases_in_sync?, {}))
.to eq(true)
end
it 'returns true when all load balancers are in sync' do
locations = {}
Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
locations[lb.name] = 'foo'
expect(lb)
.to receive(:select_up_to_date_host)
.with('foo')
.and_return(true)
end
expect(middleware.send(:databases_in_sync?, locations))
.to eq(true)
end
it 'returns false when the load balancers are not in sync' do
locations = {}
Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
locations[lb.name] = 'foo'
allow(lb)
.to receive(:select_up_to_date_host)
.with('foo')
.and_return(false)
end
expect(middleware.send(:databases_in_sync?, locations))
.to eq(false)
end
end
def process_job(job) def process_job(job)
Sidekiq::JobRetry.new.local(worker_class, job.to_json, 'default') do Sidekiq::JobRetry.new.local(worker_class, job.to_json, 'default') do
worker_class.process_job(job) worker_class.process_job(job)
...@@ -208,6 +261,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_ ...@@ -208,6 +261,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
end end
def replication_lag!(exists) def replication_lag!(exists)
allow(load_balancer).to receive(:select_up_to_date_host).and_return(!exists) Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
allow(lb).to receive(:select_up_to_date_host).and_return(!exists)
end
end end
end end
...@@ -3,173 +3,48 @@ ...@@ -3,173 +3,48 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Database::LoadBalancing do RSpec.describe Gitlab::Database::LoadBalancing do
describe '.proxy' do describe '.base_models' do
it 'returns the connection proxy' do it 'returns the models to apply load balancing to' do
proxy = double(:connection_proxy) models = described_class.base_models
allow(ActiveRecord::Base) expect(models).to include(ActiveRecord::Base)
.to receive(:load_balancing_proxy)
.and_return(proxy)
expect(described_class.proxy).to eq(proxy) if Gitlab::Database.has_config?(:ci)
end expect(models).to include(Ci::CiDatabaseRecord)
end
describe '.configuration' do
it 'returns the configuration for the load balancer' do
raw = ActiveRecord::Base.connection_db_config.configuration_hash
cfg = described_class.configuration
# There isn't much to test here as the load balancing settings might not
# (and likely aren't) set when running tests.
expect(cfg.pool_size).to eq(raw[:pool])
end
end
describe '.enable_replicas?' do
context 'when hosts are specified' do
before do
allow(described_class.configuration)
.to receive(:hosts)
.and_return(%w(foo))
end
it 'returns true' do
expect(described_class.enable_replicas?).to eq(true)
end
it 'returns true when Sidekiq is being used' do
allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true)
expect(described_class.enable_replicas?).to eq(true)
end
it 'returns false when running inside a Rake task' do
allow(Gitlab::Runtime).to receive(:rake?).and_return(true)
expect(described_class.enable_replicas?).to eq(false)
end end
end end
context 'when no hosts are specified but service discovery is enabled' do it 'returns the models as a frozen array' do
it 'returns true' do expect(described_class.base_models).to be_frozen
allow(described_class.configuration).to receive(:hosts).and_return([])
allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(false)
allow(described_class.configuration)
.to receive(:service_discovery_enabled?)
.and_return(true)
expect(described_class.enable_replicas?).to eq(true)
end
end
context 'when no hosts are specified and service discovery is disabled' do
it 'returns false' do
allow(described_class.configuration).to receive(:hosts).and_return([])
allow(described_class.configuration)
.to receive(:service_discovery_enabled?)
.and_return(false)
expect(described_class.enable_replicas?).to eq(false)
end
end end
end end
describe '.configured?' do describe '.each_load_balancer' do
it 'returns true when hosts are configured' do it 'yields every load balancer to the supplied block' do
allow(described_class.configuration) lbs = []
.to receive(:hosts)
.and_return(%w[foo])
expect(described_class.configured?).to eq(true) described_class.each_load_balancer do |lb|
end lbs << lb
end
it 'returns true when service discovery is enabled' do
allow(described_class.configuration).to receive(:hosts).and_return([])
allow(described_class.configuration)
.to receive(:service_discovery_enabled?)
.and_return(true)
expect(described_class.configured?).to eq(true)
end
it 'returns false when neither service discovery nor hosts are configured' do
allow(described_class.configuration).to receive(:hosts).and_return([])
allow(described_class.configuration)
.to receive(:service_discovery_enabled?)
.and_return(false)
expect(described_class.configured?).to eq(false)
end
end
describe '.start_service_discovery' do
it 'does not start if service discovery is disabled' do
expect(Gitlab::Database::LoadBalancing::ServiceDiscovery)
.not_to receive(:new)
described_class.start_service_discovery expect(lbs.length).to eq(described_class.base_models.length)
end end
it 'starts service discovery if enabled' do it 'returns an Enumerator when no block is given' do
allow(described_class.configuration) res = described_class.each_load_balancer
.to receive(:service_discovery_enabled?)
.and_return(true)
instance = double(:instance)
config = Gitlab::Database::LoadBalancing::Configuration
.new(ActiveRecord::Base)
lb = Gitlab::Database::LoadBalancing::LoadBalancer.new(config)
proxy = Gitlab::Database::LoadBalancing::ConnectionProxy.new(lb)
allow(described_class) expect(res.next)
.to receive(:proxy) .to be_an_instance_of(Gitlab::Database::LoadBalancing::LoadBalancer)
.and_return(proxy)
expect(Gitlab::Database::LoadBalancing::ServiceDiscovery)
.to receive(:new)
.with(lb, an_instance_of(Hash))
.and_return(instance)
expect(instance)
.to receive(:start)
described_class.start_service_discovery
end end
end end
describe '.perform_service_discovery' do describe '.release_hosts' do
it 'does nothing if service discovery is disabled' do it 'releases the host of every load balancer' do
expect(Gitlab::Database::LoadBalancing::ServiceDiscovery) described_class.each_load_balancer do |lb|
.not_to receive(:new) expect(lb).to receive(:release_host)
end
described_class.perform_service_discovery
end
it 'performs service discovery when enabled' do
allow(described_class.configuration)
.to receive(:service_discovery_enabled?)
.and_return(true)
sv = instance_spy(Gitlab::Database::LoadBalancing::ServiceDiscovery)
cfg = Gitlab::Database::LoadBalancing::Configuration
.new(ActiveRecord::Base)
lb = Gitlab::Database::LoadBalancing::LoadBalancer.new(cfg)
proxy = Gitlab::Database::LoadBalancing::ConnectionProxy.new(lb)
allow(described_class)
.to receive(:proxy)
.and_return(proxy)
expect(Gitlab::Database::LoadBalancing::ServiceDiscovery)
.to receive(:new)
.with(lb, cfg.service_discovery)
.and_return(sv)
expect(sv).to receive(:perform_service_discovery)
described_class.perform_service_discovery described_class.release_hosts
end end
end end
...@@ -227,7 +102,7 @@ RSpec.describe Gitlab::Database::LoadBalancing do ...@@ -227,7 +102,7 @@ RSpec.describe Gitlab::Database::LoadBalancing do
# - In each test, we listen to the SQL queries (via sql.active_record # - In each test, we listen to the SQL queries (via sql.active_record
# instrumentation) while triggering real queries from the defined model. # instrumentation) while triggering real queries from the defined model.
# - We assert the desinations (replica/primary) of the queries in order. # - We assert the desinations (replica/primary) of the queries in order.
describe 'LoadBalancing integration tests', :db_load_balancing, :delete do describe 'LoadBalancing integration tests', :database_replica, :delete do
before(:all) do before(:all) do
ActiveRecord::Schema.define do ActiveRecord::Schema.define do
create_table :load_balancing_test, force: true do |t| create_table :load_balancing_test, force: true do |t|
......
...@@ -203,7 +203,7 @@ RSpec.describe Gitlab::Database do ...@@ -203,7 +203,7 @@ RSpec.describe Gitlab::Database do
.to eq('main') .to eq('main')
end end
context 'when replicas are configured', :db_load_balancing do context 'when replicas are configured', :database_replica do
it 'returns the name for a replica' do it 'returns the name for a replica' do
replica = ActiveRecord::Base.connection.load_balancer.host replica = ActiveRecord::Base.connection.load_balancer.host
......
...@@ -158,7 +158,7 @@ RSpec.describe Gitlab::ImportExport::Json::StreamingSerializer do ...@@ -158,7 +158,7 @@ RSpec.describe Gitlab::ImportExport::Json::StreamingSerializer do
end end
describe 'load balancing' do describe 'load balancing' do
context 'when feature flag load_balancing_for_export_workers is enabled', :db_load_balancing do context 'when feature flag load_balancing_for_export_workers is enabled' do
before do before do
stub_feature_flags(load_balancing_for_export_workers: true) stub_feature_flags(load_balancing_for_export_workers: true)
end end
......
...@@ -195,7 +195,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do ...@@ -195,7 +195,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
with_them do with_them do
let(:payload) { { name: name, sql: sql(sql_query, comments: comments), connection: connection } } let(:payload) { { name: name, sql: sql(sql_query, comments: comments), connection: connection } }
context 'query using a connection to a replica', :db_load_balancing do context 'query using a connection to a replica' do
before do before do
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).and_return(:replica) allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).and_return(:replica)
end end
......
...@@ -317,7 +317,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do ...@@ -317,7 +317,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do
end end
end end
context 'when load balancing is enabled', :db_load_balancing do context 'when load balancing is enabled' do
let(:db_config_name) do let(:db_config_name) do
::Gitlab::Database.db_config_name(ApplicationRecord.retrieve_connection) ::Gitlab::Database.db_config_name(ApplicationRecord.retrieve_connection)
end end
......
...@@ -347,10 +347,10 @@ RSpec.describe Ci::Build do ...@@ -347,10 +347,10 @@ RSpec.describe Ci::Build do
end end
describe '#stick_build_if_status_changed' do describe '#stick_build_if_status_changed' do
it 'sticks the build if the status changed', :db_load_balancing do it 'sticks the build if the status changed' do
job = create(:ci_build, :pending) job = create(:ci_build, :pending)
expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) expect(ApplicationRecord.sticking).to receive(:stick)
.with(:build, job.id) .with(:build, job.id)
job.update!(status: :running) job.update!(status: :running)
......
...@@ -2790,7 +2790,16 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -2790,7 +2790,16 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
extra_update_queries = 4 # transition ... => :canceled, queue pop extra_update_queries = 4 # transition ... => :canceled, queue pop
extra_generic_commit_status_validation_queries = 2 # name_uniqueness_across_types extra_generic_commit_status_validation_queries = 2 # name_uniqueness_across_types
extra_load_balancer_queries = 3
# The number of extra load balancing queries depends on whether or not
# we use a load balancer for CI. That in turn depends on the contents of
# database.yml, so here we support both cases.
extra_load_balancer_queries =
if Gitlab::Database.has_config?(:ci)
6
else
3
end
expect(control2.count).to eq(control1.count + extra_update_queries + extra_generic_commit_status_validation_queries + extra_load_balancer_queries) expect(control2.count).to eq(control1.count + extra_update_queries + extra_generic_commit_status_validation_queries + extra_load_balancer_queries)
end end
......
...@@ -397,7 +397,7 @@ RSpec.describe Ci::Runner do ...@@ -397,7 +397,7 @@ RSpec.describe Ci::Runner do
it 'sticks the runner to the primary and calls the original method' do it 'sticks the runner to the primary and calls the original method' do
runner = create(:ci_runner) runner = create(:ci_runner)
expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) expect(ApplicationRecord.sticking).to receive(:stick)
.with(:runner, runner.id) .with(:runner, runner.id)
expect(Gitlab::Workhorse).to receive(:set_key_and_notify) expect(Gitlab::Workhorse).to receive(:set_key_and_notify)
......
...@@ -133,10 +133,8 @@ RSpec.describe ProjectFeatureUsage, type: :model do ...@@ -133,10 +133,8 @@ RSpec.describe ProjectFeatureUsage, type: :model do
subject { project.feature_usage } subject { project.feature_usage }
context 'database load balancing is configured', :db_load_balancing do context 'database load balancing is configured' do
before do before do
allow(ActiveRecord::Base).to receive(:connection).and_return(::Gitlab::Database::LoadBalancing.proxy)
::Gitlab::Database::LoadBalancing::Session.clear_session ::Gitlab::Database::LoadBalancing::Session.clear_session
end end
......
...@@ -3050,7 +3050,7 @@ RSpec.describe Project, factory_default: :keep do ...@@ -3050,7 +3050,7 @@ RSpec.describe Project, factory_default: :keep do
let(:project) { create(:project) } let(:project) { create(:project) }
it 'marks the location with project ID' do it 'marks the location with project ID' do
expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:mark_primary_write_location).with(:project, project.id) expect(ApplicationRecord.sticking).to receive(:mark_primary_write_location).with(:project, project.id)
project.mark_primary_write_location project.mark_primary_write_location
end end
......
...@@ -115,7 +115,7 @@ RSpec.describe 'Setting assignees of a merge request', :assume_throttled do ...@@ -115,7 +115,7 @@ RSpec.describe 'Setting assignees of a merge request', :assume_throttled do
context 'when passing append as true' do context 'when passing append as true' do
let(:mode) { Types::MutationOperationModeEnum.enum[:append] } let(:mode) { Types::MutationOperationModeEnum.enum[:append] }
let(:input) { { assignee_usernames: [assignee2.username], operation_mode: mode } } let(:input) { { assignee_usernames: [assignee2.username], operation_mode: mode } }
let(:db_query_limit) { 21 } let(:db_query_limit) { 22 }
before do before do
# In CE, APPEND is a NOOP as you can't have multiple assignees # In CE, APPEND is a NOOP as you can't have multiple assignees
......
...@@ -50,13 +50,14 @@ RSpec.describe Ci::DropPipelineService do ...@@ -50,13 +50,14 @@ RSpec.describe Ci::DropPipelineService do
end.count end.count
writes_per_build = 2 writes_per_build = 2
load_balancer_queries = 3
expected_reads_count = control_count - writes_per_build expected_reads_count = control_count - writes_per_build
create_list(:ci_build, 5, :running, pipeline: cancelable_pipeline) create_list(:ci_build, 5, :running, pipeline: cancelable_pipeline)
expect do expect do
drop_pipeline!(cancelable_pipeline) drop_pipeline!(cancelable_pipeline)
end.not_to exceed_query_limit(expected_reads_count + (5 * writes_per_build)) end.not_to exceed_query_limit(expected_reads_count + (5 * writes_per_build) + load_balancer_queries)
end end
end end
end end
...@@ -14,7 +14,7 @@ module Ci ...@@ -14,7 +14,7 @@ module Ci
let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) } let!(:pending_job) { create(:ci_build, :pending, :queued, pipeline: pipeline) }
describe '#execute' do describe '#execute' do
context 'checks database loadbalancing stickiness', :db_load_balancing do context 'checks database loadbalancing stickiness' do
subject { described_class.new(shared_runner).execute } subject { described_class.new(shared_runner).execute }
before do before do
...@@ -22,14 +22,14 @@ module Ci ...@@ -22,14 +22,14 @@ module Ci
end end
it 'result is valid if replica did caught-up' do it 'result is valid if replica did caught-up' do
expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:all_caught_up?) expect(ApplicationRecord.sticking).to receive(:all_caught_up?)
.with(:runner, shared_runner.id) { true } .with(:runner, shared_runner.id) { true }
expect(subject).to be_valid expect(subject).to be_valid
end end
it 'result is invalid if replica did not caught-up' do it 'result is invalid if replica did not caught-up' do
expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:all_caught_up?) expect(ApplicationRecord.sticking).to receive(:all_caught_up?)
.with(:runner, shared_runner.id) { false } .with(:runner, shared_runner.id) { false }
expect(subject).not_to be_valid expect(subject).not_to be_valid
......
...@@ -53,7 +53,7 @@ RSpec.describe UserProjectAccessChangedService do ...@@ -53,7 +53,7 @@ RSpec.describe UserProjectAccessChangedService do
end end
it 'sticks all the updated users and returns the original result', :aggregate_failures do it 'sticks all the updated users and returns the original result', :aggregate_failures do
expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:bulk_stick).with(:user, [1, 2]) expect(ApplicationRecord.sticking).to receive(:bulk_stick).with(:user, [1, 2])
expect(service.execute).to eq(10) expect(service.execute).to eq(10)
end end
......
...@@ -91,9 +91,9 @@ RSpec.describe Users::ActivityService do ...@@ -91,9 +91,9 @@ RSpec.describe Users::ActivityService do
context 'when last activity is in the past' do context 'when last activity is in the past' do
let(:user) { create(:user, last_activity_on: Date.today - 1.week) } let(:user) { create(:user, last_activity_on: Date.today - 1.week) }
context 'database load balancing is configured', :db_load_balancing do context 'database load balancing is configured' do
before do before do
allow(ActiveRecord::Base).to receive(:connection).and_return(::Gitlab::Database::LoadBalancing.proxy) ::Gitlab::Database::LoadBalancing::Session.clear_session
end end
let(:service) do let(:service) do
......
# frozen_string_literal: true # frozen_string_literal: true
RSpec.configure do |config| RSpec.configure do |config|
config.before(:each, :db_load_balancing) do config.around(:each, :database_replica) do |example|
config = Gitlab::Database::LoadBalancing::Configuration old_proxies = []
.new(ActiveRecord::Base, [Gitlab::Database.main.config['host']])
lb = ::Gitlab::Database::LoadBalancing::LoadBalancer.new(config)
proxy = ::Gitlab::Database::LoadBalancing::ConnectionProxy.new(lb)
allow(ActiveRecord::Base).to receive(:load_balancing_proxy).and_return(proxy) Gitlab::Database::LoadBalancing.base_models.each do |model|
config = Gitlab::Database::LoadBalancing::Configuration
.new(model, [model.connection_db_config.configuration_hash[:host]])
lb = Gitlab::Database::LoadBalancing::LoadBalancer.new(config)
::Gitlab::Database::LoadBalancing::Session.clear_session old_proxies << [model, model.connection]
model.connection =
Gitlab::Database::LoadBalancing::ConnectionProxy.new(lb)
end
Gitlab::Database::LoadBalancing::Session.clear_session
redis_shared_state_cleanup! redis_shared_state_cleanup!
end
config.after(:each, :db_load_balancing) do example.run
::Gitlab::Database::LoadBalancing::Session.clear_session
Gitlab::Database::LoadBalancing::Session.clear_session
redis_shared_state_cleanup! redis_shared_state_cleanup!
old_proxies.each do |(model, proxy)|
model.connection = proxy
end
end end
end end
...@@ -35,8 +35,8 @@ RSpec.shared_examples 'common trace features' do ...@@ -35,8 +35,8 @@ RSpec.shared_examples 'common trace features' do
stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: trace.job.project) stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: trace.job.project)
end end
it 'calls ::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking' do it 'calls ::ApplicationRecord.sticking.unstick_or_continue_sticking' do
expect(::Gitlab::Database::LoadBalancing::Sticking).to receive(:unstick_or_continue_sticking) expect(::ApplicationRecord.sticking).to receive(:unstick_or_continue_sticking)
.with(described_class::LOAD_BALANCING_STICKING_NAMESPACE, trace.job.id) .with(described_class::LOAD_BALANCING_STICKING_NAMESPACE, trace.job.id)
.and_call_original .and_call_original
...@@ -49,8 +49,8 @@ RSpec.shared_examples 'common trace features' do ...@@ -49,8 +49,8 @@ RSpec.shared_examples 'common trace features' do
stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: false) stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: false)
end end
it 'does not call ::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking' do it 'does not call ::ApplicationRecord.sticking.unstick_or_continue_sticking' do
expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:unstick_or_continue_sticking) expect(::ApplicationRecord.sticking).not_to receive(:unstick_or_continue_sticking)
trace.read { |stream| stream } trace.read { |stream| stream }
end end
...@@ -305,8 +305,8 @@ RSpec.shared_examples 'common trace features' do ...@@ -305,8 +305,8 @@ RSpec.shared_examples 'common trace features' do
stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: trace.job.project) stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: trace.job.project)
end end
it 'calls ::Gitlab::Database::LoadBalancing::Sticking.stick' do it 'calls ::ApplicationRecord.sticking.stick' do
expect(::Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) expect(::ApplicationRecord.sticking).to receive(:stick)
.with(described_class::LOAD_BALANCING_STICKING_NAMESPACE, trace.job.id) .with(described_class::LOAD_BALANCING_STICKING_NAMESPACE, trace.job.id)
.and_call_original .and_call_original
...@@ -319,8 +319,8 @@ RSpec.shared_examples 'common trace features' do ...@@ -319,8 +319,8 @@ RSpec.shared_examples 'common trace features' do
stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: false) stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: false)
end end
it 'does not call ::Gitlab::Database::LoadBalancing::Sticking.stick' do it 'does not call ::ApplicationRecord.sticking.stick' do
expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:stick) expect(::ApplicationRecord.sticking).not_to receive(:stick)
subject subject
end end
......
...@@ -44,7 +44,7 @@ RSpec.describe AuthorizedProjectUpdate::UserRefreshFromReplicaWorker do ...@@ -44,7 +44,7 @@ RSpec.describe AuthorizedProjectUpdate::UserRefreshFromReplicaWorker do
end end
end end
context 'with load balancing enabled', :db_load_balancing do context 'with load balancing enabled' do
it 'reads from the replica database' do it 'reads from the replica database' do
expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original
......
...@@ -156,7 +156,7 @@ RSpec.describe ContainerExpirationPolicyWorker do ...@@ -156,7 +156,7 @@ RSpec.describe ContainerExpirationPolicyWorker do
subject subject
end end
context 'with load balancing enabled', :db_load_balancing do context 'with load balancing enabled' do
it 'reads the counts from the replica' do it 'reads the counts from the replica' do
expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original expect(Gitlab::Database::LoadBalancing::Session.current).to receive(:use_replicas_for_read_queries).and_call_original
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment