Commit 4e7a4d32 authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'always-enable-load-balancer' into 'master'

Always enable the database load balancer

See merge request gitlab-org/gitlab!68042
parents c0f49022 cf3a0e35
...@@ -30,8 +30,6 @@ module AuthorizedProjectUpdate ...@@ -30,8 +30,6 @@ module AuthorizedProjectUpdate
# does not allow us to deduplicate these jobs. # does not allow us to deduplicate these jobs.
# https://gitlab.com/gitlab-org/gitlab/-/issues/325291 # https://gitlab.com/gitlab-org/gitlab/-/issues/325291
def use_replica_if_available(&block) def use_replica_if_available(&block)
return yield unless ::Gitlab::Database::LoadBalancing.enable?
::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&block) ::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&block)
end end
......
...@@ -45,8 +45,6 @@ class ContainerExpirationPolicyWorker # rubocop:disable Scalability/IdempotentWo ...@@ -45,8 +45,6 @@ class ContainerExpirationPolicyWorker # rubocop:disable Scalability/IdempotentWo
# not perfomed with a delay # not perfomed with a delay
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63635#note_603771207 # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63635#note_603771207
def use_replica_if_available(&blk) def use_replica_if_available(&blk)
return yield unless ::Gitlab::Database::LoadBalancing.enable?
::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&blk) ::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&blk)
end end
......
...@@ -19,4 +19,4 @@ ActionCable::SubscriptionAdapter::Redis.redis_connector = lambda do |config| ...@@ -19,4 +19,4 @@ ActionCable::SubscriptionAdapter::Redis.redis_connector = lambda do |config|
end end
Gitlab::ActionCable::RequestStoreCallbacks.install Gitlab::ActionCable::RequestStoreCallbacks.install
Gitlab::Database::LoadBalancing::ActionCableCallbacks.install if Gitlab::Database::LoadBalancing.enable? Gitlab::Database::LoadBalancing::ActionCableCallbacks.install
...@@ -2,27 +2,51 @@ ...@@ -2,27 +2,51 @@
ActiveRecord::Base.singleton_class.attr_accessor :load_balancing_proxy ActiveRecord::Base.singleton_class.attr_accessor :load_balancing_proxy
if Gitlab::Database::LoadBalancing.enable? Gitlab::Database.main.disable_prepared_statements
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
# `ActiveRecord::Base.connection` and all models use the same load
# balancing proxy.
ActiveRecord::Base.singleton_class.prepend(Gitlab::Database::LoadBalancing::ActiveRecordProxy)
# The load balancer needs to be configured immediately, and re-configured after
# forking. This ensures queries that run before forking use the load balancer,
# and queries running after a fork don't run into any errors when using dead
# database connections.
#
# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63485 for more
# information.
setup = proc do
lb = Gitlab::Database::LoadBalancing::LoadBalancer.new(
Gitlab::Database::LoadBalancing.configuration,
primary_only: !Gitlab::Database::LoadBalancing.enable_replicas?
)
# This hijacks the "connection" method to ensure both ActiveRecord::Base.load_balancing_proxy =
# `ActiveRecord::Base.connection` and all models use the same load Gitlab::Database::LoadBalancing::ConnectionProxy.new(lb)
# balancing proxy.
ActiveRecord::Base.singleton_class.prepend(Gitlab::Database::LoadBalancing::ActiveRecordProxy)
Gitlab::Database::LoadBalancing.configure_proxy # Populate service discovery immediately if it is configured
Gitlab::Database::LoadBalancing.perform_service_discovery
end
setup.call
# This needs to be executed after fork of clustered processes # Database queries may be run before we fork, so we must set up the load
Gitlab::Cluster::LifecycleEvents.on_worker_start do # balancer as early as possible. When we do fork, we need to make sure all the
# For Host-based LB, we need to re-connect as Rails discards connections on fork # hosts are disconnected.
Gitlab::Database::LoadBalancing.configure_proxy 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 must be started after configuring the proxy, as service # Service discovery only needs to run in the worker processes, as the main one
# discovery depends on this. # won't be running many (if any) database queries.
Gitlab::Database::LoadBalancing.start_service_discovery Gitlab::Cluster::LifecycleEvents.on_worker_start do
end setup.call
Gitlab::Database::LoadBalancing.start_service_discovery
end end
...@@ -29,12 +29,8 @@ if Gitlab::Metrics.enabled? && !Rails.env.test? && !(Rails.env.development? && d ...@@ -29,12 +29,8 @@ if Gitlab::Metrics.enabled? && !Rails.env.test? && !(Rails.env.development? && d
Gitlab::Application.configure do |config| Gitlab::Application.configure do |config|
# We want to track certain metrics during the Load Balancing host resolving process. # We want to track certain metrics during the Load Balancing host resolving process.
# Because of that, we need to have metrics code available earlier for Load Balancing. # Because of that, we need to have metrics code available earlier for Load Balancing.
if Gitlab::Database::LoadBalancing.enable? config.middleware.insert_before Gitlab::Database::LoadBalancing::RackMiddleware,
config.middleware.insert_before Gitlab::Database::LoadBalancing::RackMiddleware, Gitlab::Metrics::RackMiddleware
Gitlab::Metrics::RackMiddleware
else
config.middleware.use(Gitlab::Metrics::RackMiddleware)
end
config.middleware.use(Gitlab::Middleware::RailsQueueDuration) config.middleware.use(Gitlab::Middleware::RailsQueueDuration)
config.middleware.use(Gitlab::Metrics::ElasticsearchRackMiddleware) config.middleware.use(Gitlab::Metrics::ElasticsearchRackMiddleware)
......
...@@ -166,7 +166,7 @@ RSpec.describe Gitlab::Geo, :geo, :request_store do ...@@ -166,7 +166,7 @@ RSpec.describe Gitlab::Geo, :geo, :request_store do
describe '.connected?' do describe '.connected?' do
context 'when there is a database issue' do context 'when there is a database issue' do
it 'returns false when it cannot open an active database connection' do it 'returns false when it cannot open an active database connection' do
allow(GeoNode.connection).to receive(:active?).and_return(false) allow(GeoNode.retrieve_connection).to receive(:active?).and_return(false)
expect(described_class.connected?).to be_falsey expect(described_class.connected?).to be_falsey
end end
......
...@@ -13,23 +13,21 @@ module Gitlab ...@@ -13,23 +13,21 @@ module Gitlab
end end
def match? def match?
if ::Gitlab::Database::LoadBalancing.enable? # When a user merges a merge request, the following sequence happens:
# When a user merges a merge request, the following sequence happens: #
# # 1. Sidekiq: MergeService runs and updates the merge request in a locked state.
# 1. Sidekiq: MergeService runs and updates the merge request in a locked state. # 2. Gitaly: The UserMergeBranch RPC runs.
# 2. Gitaly: The UserMergeBranch RPC runs. # 3. Gitaly (gitaly-ruby): This RPC calls the pre-receive hook.
# 3. Gitaly (gitaly-ruby): This RPC calls the pre-receive hook. # 4. Rails: This hook makes an API request to /api/v4/internal/allowed.
# 4. Rails: This hook makes an API request to /api/v4/internal/allowed. # 5. Rails: This API check does a SQL query for locked merge
# 5. Rails: This API check does a SQL query for locked merge # requests with a matching SHA.
# requests with a matching SHA. #
# # Since steps 1 and 5 will happen on different database
# Since steps 1 and 5 will happen on different database # sessions, replication lag could erroneously cause step 5 to
# sessions, replication lag could erroneously cause step 5 to # 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)
::Gitlab::Database::LoadBalancing::Sticking.select_valid_host(:project, @project.id)
end
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
......
...@@ -4,34 +4,22 @@ module Gitlab ...@@ -4,34 +4,22 @@ module Gitlab
module Database module Database
module LoadBalancing module LoadBalancing
# The exceptions raised for connection errors. # The exceptions raised for connection errors.
CONNECTION_ERRORS = if defined?(PG) CONNECTION_ERRORS = [
[ PG::ConnectionBad,
PG::ConnectionBad, PG::ConnectionDoesNotExist,
PG::ConnectionDoesNotExist, PG::ConnectionException,
PG::ConnectionException, PG::ConnectionFailure,
PG::ConnectionFailure, PG::UnableToSend,
PG::UnableToSend, # During a failover this error may be raised when
# During a failover this error may be raised when # writing to a primary.
# writing to a primary. PG::ReadOnlySqlTransaction,
PG::ReadOnlySqlTransaction # This error is raised when we can't connect to the database in the
].freeze # first place (e.g. it's offline or the hostname is incorrect).
else ActiveRecord::ConnectionNotEstablished
[].freeze ].freeze
end
ProxyNotConfiguredError = Class.new(StandardError)
# The connection proxy to use for load balancing (if enabled).
def self.proxy def self.proxy
unless load_balancing_proxy = ActiveRecord::Base.load_balancing_proxy ActiveRecord::Base.load_balancing_proxy
Gitlab::ErrorTracking.track_exception(
ProxyNotConfiguredError.new(
"Attempting to access the database load balancing proxy, but it wasn't configured.\n" \
"Did you forget to call '#{self.name}.configure_proxy'?"
))
end
load_balancing_proxy
end end
# Returns a Hash containing the load balancing configuration. # Returns a Hash containing the load balancing configuration.
...@@ -39,8 +27,11 @@ module Gitlab ...@@ -39,8 +27,11 @@ module Gitlab
@configuration ||= Configuration.for_model(ActiveRecord::Base) @configuration ||= Configuration.for_model(ActiveRecord::Base)
end end
# Returns true if load balancing is to be enabled. # Returns `true` if the use of load balancing replicas should be enabled.
def self.enable? #
# 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? return false if Gitlab::Runtime.rake?
configured? configured?
...@@ -59,17 +50,12 @@ module Gitlab ...@@ -59,17 +50,12 @@ module Gitlab
.start .start
end end
# Configures proxying of requests. def self.perform_service_discovery
def self.configure_proxy return unless configuration.service_discovery_enabled?
lb = LoadBalancer.new(configuration, primary_only: !enable?)
ActiveRecord::Base.load_balancing_proxy = ConnectionProxy.new(lb)
# Populate service discovery immediately if it is configured ServiceDiscovery
if configuration.service_discovery_enabled? .new(proxy.load_balancer, **configuration.service_discovery)
ServiceDiscovery .perform_service_discovery
.new(lb, **configuration.service_discovery)
.perform_service_discovery
end
end end
DB_ROLES = [ DB_ROLES = [
......
...@@ -9,19 +9,12 @@ module Gitlab ...@@ -9,19 +9,12 @@ module Gitlab
delegate :connection, :release_connection, :enable_query_cache!, :disable_query_cache!, :query_cache_enabled, to: :pool delegate :connection, :release_connection, :enable_query_cache!, :disable_query_cache!, :query_cache_enabled, to: :pool
CONNECTION_ERRORS = CONNECTION_ERRORS = [
if defined?(PG) ActionView::Template::Error,
[ ActiveRecord::StatementInvalid,
ActionView::Template::Error, ActiveRecord::ConnectionNotEstablished,
ActiveRecord::StatementInvalid, PG::Error
PG::Error ].freeze
].freeze
else
[
ActionView::Template::Error,
ActiveRecord::StatementInvalid
].freeze
end
# host - The address of the database. # host - The address of the database.
# load_balancer - The LoadBalancer that manages this Host. # load_balancer - The LoadBalancer that manages this Host.
......
...@@ -18,8 +18,6 @@ module Gitlab ...@@ -18,8 +18,6 @@ module Gitlab
# namespace - The namespace to use for sticking. # namespace - The namespace to use for sticking.
# id - The identifier to use for sticking. # id - The identifier to use for sticking.
def self.stick_or_unstick(env, namespace, id) def self.stick_or_unstick(env, namespace, id)
return unless ::Gitlab::Database::LoadBalancing.enable?
::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking(namespace, id) ::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking(namespace, id)
env[STICK_OBJECT] ||= Set.new env[STICK_OBJECT] ||= Set.new
......
...@@ -22,8 +22,6 @@ module Gitlab ...@@ -22,8 +22,6 @@ module Gitlab
# 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 self.stick_if_necessary(namespace, id)
return unless LoadBalancing.enable?
stick(namespace, id) if Session.current.performed_write? stick(namespace, id) if Session.current.performed_write?
end end
...@@ -75,15 +73,11 @@ module Gitlab ...@@ -75,15 +73,11 @@ module Gitlab
# 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 self.stick(namespace, id)
return unless LoadBalancing.enable?
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 self.bulk_stick(namespace, ids)
return unless LoadBalancing.enable?
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)
...@@ -94,17 +88,7 @@ module Gitlab ...@@ -94,17 +88,7 @@ module Gitlab
end end
def self.with_primary_write_location def self.with_primary_write_location
return unless LoadBalancing.configured? location = load_balancer.primary_write_location
# Load balancing could be enabled for the Web application server,
# but it's not activated for Sidekiq. We should update Redis with
# the write location just in case load balancing is being used.
location =
if LoadBalancing.enable?
load_balancer.primary_write_location
else
Gitlab::Database.main.get_write_location(ActiveRecord::Base.connection)
end
return if location.blank? return if location.blank?
......
...@@ -171,7 +171,6 @@ module Gitlab ...@@ -171,7 +171,6 @@ module Gitlab
def read_from_replica_if_available(&block) def read_from_replica_if_available(&block)
return yield unless ::Feature.enabled?(:load_balancing_for_export_workers, type: :development, default_enabled: :yaml) return yield unless ::Feature.enabled?(:load_balancing_for_export_workers, type: :development, default_enabled: :yaml)
return yield unless ::Gitlab::Database::LoadBalancing.enable?
::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&block) ::Gitlab::Database::LoadBalancing::Session.current.use_replicas_for_read_queries(&block)
end end
......
...@@ -47,13 +47,11 @@ module Gitlab ...@@ -47,13 +47,11 @@ module Gitlab
buckets SQL_DURATION_BUCKET buckets SQL_DURATION_BUCKET
end end
if ::Gitlab::Database::LoadBalancing.enable? db_role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(payload[:connection])
db_role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(payload[:connection]) return if db_role.blank?
return if db_role.blank?
increment_db_role_counters(db_role, payload) increment_db_role_counters(db_role, payload)
observe_db_role_duration(db_role, event) observe_db_role_duration(db_role, event)
end
end end
def self.db_counter_payload def self.db_counter_payload
...@@ -64,7 +62,7 @@ module Gitlab ...@@ -64,7 +62,7 @@ module Gitlab
payload[key] = Gitlab::SafeRequestStore[key].to_i payload[key] = Gitlab::SafeRequestStore[key].to_i
end end
if ::Gitlab::SafeRequestStore.active? && ::Gitlab::Database::LoadBalancing.enable? if ::Gitlab::SafeRequestStore.active?
load_balancing_metric_counter_keys.each do |counter| load_balancing_metric_counter_keys.each do |counter|
payload[counter] = ::Gitlab::SafeRequestStore[counter].to_i payload[counter] = ::Gitlab::SafeRequestStore[counter].to_i
end end
......
...@@ -10,7 +10,7 @@ module Gitlab ...@@ -10,7 +10,7 @@ module Gitlab
LOG_COUNTERS = { true => :caught_up_replica_pick_ok, false => :caught_up_replica_pick_fail }.freeze LOG_COUNTERS = { true => :caught_up_replica_pick_ok, false => :caught_up_replica_pick_fail }.freeze
def caught_up_replica_pick(event) def caught_up_replica_pick(event)
return unless Gitlab::SafeRequestStore.active? && ::Gitlab::Database::LoadBalancing.enable? return unless Gitlab::SafeRequestStore.active?
result = event.payload[:result] result = event.payload[:result]
counter_name = counter(result) counter_name = counter(result)
...@@ -20,13 +20,13 @@ module Gitlab ...@@ -20,13 +20,13 @@ module Gitlab
# we want to update Prometheus counter after the controller/action are set # we want to update Prometheus counter after the controller/action are set
def web_transaction_completed(_event) def web_transaction_completed(_event)
return unless Gitlab::SafeRequestStore.active? && ::Gitlab::Database::LoadBalancing.enable? return unless Gitlab::SafeRequestStore.active?
LOG_COUNTERS.keys.each { |result| increment_prometheus_for_result_label(result) } LOG_COUNTERS.keys.each { |result| increment_prometheus_for_result_label(result) }
end end
def self.load_balancing_payload def self.load_balancing_payload
return {} unless Gitlab::SafeRequestStore.active? && ::Gitlab::Database::LoadBalancing.enable? return {} unless Gitlab::SafeRequestStore.active?
{}.tap do |payload| {}.tap do |payload|
LOG_COUNTERS.values.each do |counter| LOG_COUNTERS.values.each do |counter|
......
...@@ -39,7 +39,7 @@ module Gitlab ...@@ -39,7 +39,7 @@ module Gitlab
# DuplicateJobs::Server should be placed at the bottom, but before the SidekiqServerMiddleware, # DuplicateJobs::Server should be placed at the bottom, but before the SidekiqServerMiddleware,
# so we can compare the latest WAL location against replica # so we can compare the latest WAL location against replica
chain.add ::Gitlab::SidekiqMiddleware::DuplicateJobs::Server chain.add ::Gitlab::SidekiqMiddleware::DuplicateJobs::Server
chain.add ::Gitlab::Database::LoadBalancing::SidekiqServerMiddleware if load_balancing_enabled? chain.add ::Gitlab::Database::LoadBalancing::SidekiqServerMiddleware
end end
end end
...@@ -52,7 +52,7 @@ module Gitlab ...@@ -52,7 +52,7 @@ module Gitlab
chain.add ::Labkit::Middleware::Sidekiq::Client chain.add ::Labkit::Middleware::Sidekiq::Client
# Sidekiq Client Middleware should be placed before DuplicateJobs::Client middleware, # Sidekiq Client Middleware should be placed before DuplicateJobs::Client middleware,
# so we can store WAL location before we deduplicate the job. # so we can store WAL location before we deduplicate the job.
chain.add ::Gitlab::Database::LoadBalancing::SidekiqClientMiddleware if load_balancing_enabled? chain.add ::Gitlab::Database::LoadBalancing::SidekiqClientMiddleware
chain.add ::Gitlab::SidekiqMiddleware::DuplicateJobs::Client chain.add ::Gitlab::SidekiqMiddleware::DuplicateJobs::Client
chain.add ::Gitlab::SidekiqStatus::ClientMiddleware chain.add ::Gitlab::SidekiqStatus::ClientMiddleware
chain.add ::Gitlab::SidekiqMiddleware::AdminMode::Client chain.add ::Gitlab::SidekiqMiddleware::AdminMode::Client
...@@ -61,10 +61,5 @@ module Gitlab ...@@ -61,10 +61,5 @@ module Gitlab
chain.add ::Gitlab::SidekiqMiddleware::ClientMetrics chain.add ::Gitlab::SidekiqMiddleware::ClientMetrics
end end
end end
def self.load_balancing_enabled?
::Gitlab::Database::LoadBalancing.enable?
end
private_class_method :load_balancing_enabled?
end end
end end
...@@ -53,10 +53,7 @@ module Gitlab ...@@ -53,10 +53,7 @@ module Gitlab
def initialize def initialize
@metrics = self.class.metrics @metrics = self.class.metrics
@metrics[:sidekiq_load_balancing_count] = ::Gitlab::Metrics.counter(:sidekiq_load_balancing_count, 'Sidekiq jobs with load balancing')
if ::Gitlab::Database::LoadBalancing.enable?
@metrics[:sidekiq_load_balancing_count] = ::Gitlab::Metrics.counter(:sidekiq_load_balancing_count, 'Sidekiq jobs with load balancing')
end
end end
def call(worker, job, queue) def call(worker, job, queue)
...@@ -128,8 +125,6 @@ module Gitlab ...@@ -128,8 +125,6 @@ module Gitlab
private private
def with_load_balancing_settings(job) def with_load_balancing_settings(job)
return unless ::Gitlab::Database::LoadBalancing.enable?
keys = %w[load_balancing_strategy worker_data_consistency] keys = %w[load_balancing_strategy worker_data_consistency]
return unless keys.all? { |k| job.key?(k) } return unless keys.all? { |k| job.key?(k) }
......
...@@ -44,10 +44,8 @@ module Peek ...@@ -44,10 +44,8 @@ module Peek
count[item[:transaction]] += 1 count[item[:transaction]] += 1
end end
if ::Gitlab::Database::LoadBalancing.enable? count[item[:db_role]] ||= 0
count[item[:db_role]] ||= 0 count[item[:db_role]] += 1
count[item[:db_role]] += 1
end
end end
def setup_subscribers def setup_subscribers
...@@ -72,8 +70,6 @@ module Peek ...@@ -72,8 +70,6 @@ module Peek
end end
def db_role(data) def db_role(data)
return unless ::Gitlab::Database::LoadBalancing.enable?
role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection]) || role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection]) ||
::Gitlab::Database::LoadBalancing::ROLE_UNKNOWN ::Gitlab::Database::LoadBalancing::ROLE_UNKNOWN
......
...@@ -230,39 +230,21 @@ RSpec.describe 'lograge', type: :request do ...@@ -230,39 +230,21 @@ RSpec.describe 'lograge', type: :request do
end end
end end
context 'when load balancing is enabled' do context 'with db payload' do
before do context 'when RequestStore is enabled', :request_store do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) it 'includes db counters for load balancing' do
end subscriber.process_action(event)
context 'with db payload' do
context 'when RequestStore is enabled', :request_store do
it 'includes db counters for load balancing' do
subscriber.process_action(event)
expect(log_data).to include(*db_load_balancing_logging_keys)
end
end
context 'when RequestStore is disabled' do
it 'does not include db counters for load balancing' do
subscriber.process_action(event)
expect(log_data).not_to include(*db_load_balancing_logging_keys) expect(log_data).to include(*db_load_balancing_logging_keys)
end
end end
end end
end
context 'when load balancing is disabled' do context 'when RequestStore is disabled' do
before do it 'does not include db counters for load balancing' do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) subscriber.process_action(event)
end
it 'does not include db counters for load balancing' do expect(log_data).not_to include(*db_load_balancing_logging_keys)
subscriber.process_action(event) end
expect(log_data).not_to include(*db_load_balancing_logging_keys)
end end
end end
end end
......
...@@ -32,10 +32,6 @@ RSpec.describe API::Helpers do ...@@ -32,10 +32,6 @@ RSpec.describe API::Helpers do
helper helper
end end
before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
end
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)
......
...@@ -31,35 +31,22 @@ RSpec.describe Gitlab::Checks::MatchingMergeRequest do ...@@ -31,35 +31,22 @@ RSpec.describe Gitlab::Checks::MatchingMergeRequest do
expect(matcher.match?).to be false expect(matcher.match?).to be false
end end
context 'with load balancing disabled', :request_store, :redis do context 'with load balancing enabled' do
before do
expect(::Gitlab::Database::LoadBalancing).to receive(:enable?).at_least(:once).and_return(false)
expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:unstick_or_continue_sticking)
expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:select_valid_replicas)
end
it 'does not attempt to stick to primary' do
expect(subject.match?).to be true
end
it 'increments no counters' do
expect { subject.match? }
.to change { total_counter.get }.by(0)
.and change { stale_counter.get }.by(0)
end
end
context 'with load balancing enabled', :db_load_balancing do
let(:session) { ::Gitlab::Database::LoadBalancing::Session.current } let(:session) { ::Gitlab::Database::LoadBalancing::Session.current }
let(:all_caught_up) { true } let(:all_caught_up) { true }
before do before do
Gitlab::Database::LoadBalancing::Session.clear_session
allow(::Gitlab::Database::LoadBalancing::Sticking).to receive(:all_caught_up?).and_return(all_caught_up) 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 expect(::Gitlab::Database::LoadBalancing::Sticking).to receive(:select_valid_host).with(:project, project.id).and_call_original
allow(::Gitlab::Database::LoadBalancing::Sticking).to receive(:select_caught_up_replicas).with(:project, project.id).and_return(all_caught_up) allow(::Gitlab::Database::LoadBalancing::Sticking).to receive(:select_caught_up_replicas).with(:project, project.id).and_return(all_caught_up)
end end
after do
Gitlab::Database::LoadBalancing::Session.clear_session
end
shared_examples 'secondary that has caught up to a primary' do shared_examples 'secondary that has caught up to a primary' do
it 'continues to use the secondary' do it 'continues to use the secondary' do
expect(session.use_primary?).to be false expect(session.use_primary?).to be false
......
...@@ -122,7 +122,7 @@ RSpec.describe Gitlab::Database::BulkUpdate do ...@@ -122,7 +122,7 @@ RSpec.describe Gitlab::Database::BulkUpdate do
stub_const('ActiveRecordBasePreparedStatementsInverted', klass) stub_const('ActiveRecordBasePreparedStatementsInverted', klass)
c = ActiveRecord::Base.connection.instance_variable_get(:@config) c = ActiveRecord::Base.retrieve_connection.instance_variable_get(:@config)
inverted = c.merge(prepared_statements: !ActiveRecord::Base.connection.prepared_statements) inverted = c.merge(prepared_statements: !ActiveRecord::Base.connection.prepared_statements)
ActiveRecordBasePreparedStatementsInverted.establish_connection(inverted) ActiveRecordBasePreparedStatementsInverted.establish_connection(inverted)
......
...@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Database::Consistency do ...@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Database::Consistency do
Gitlab::Database::LoadBalancing::Session.current Gitlab::Database::LoadBalancing::Session.current
end end
describe '.with_read_consistency' do describe '.with_read_consistency', :db_load_balancing 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
......
...@@ -172,6 +172,14 @@ RSpec.describe Gitlab::Database::LoadBalancing::Host do ...@@ -172,6 +172,14 @@ RSpec.describe Gitlab::Database::LoadBalancing::Host do
expect(host).not_to be_online expect(host).not_to be_online
end end
it 'returns false when ActiveRecord::ConnectionNotEstablished is raised' do
allow(host)
.to receive(:check_replica_status?)
.and_raise(ActiveRecord::ConnectionNotEstablished)
expect(host).not_to be_online
end
end end
end end
......
...@@ -277,31 +277,26 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do ...@@ -277,31 +277,26 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
end end
describe '#connection_error?' do describe '#connection_error?' do
before do
stub_const('Gitlab::Database::LoadBalancing::LoadBalancer::CONNECTION_ERRORS',
[NotImplementedError])
end
it 'returns true for a connection error' do it 'returns true for a connection error' do
error = NotImplementedError.new error = ActiveRecord::ConnectionNotEstablished.new
expect(lb.connection_error?(error)).to eq(true) expect(lb.connection_error?(error)).to eq(true)
end end
it 'returns true for a wrapped connection error' do it 'returns true for a wrapped connection error' do
wrapped = wrapped_exception(ActiveRecord::StatementInvalid, NotImplementedError) wrapped = wrapped_exception(ActiveRecord::StatementInvalid, ActiveRecord::ConnectionNotEstablished)
expect(lb.connection_error?(wrapped)).to eq(true) expect(lb.connection_error?(wrapped)).to eq(true)
end end
it 'returns true for a wrapped connection error from a view' do it 'returns true for a wrapped connection error from a view' do
wrapped = wrapped_exception(ActionView::Template::Error, NotImplementedError) wrapped = wrapped_exception(ActionView::Template::Error, ActiveRecord::ConnectionNotEstablished)
expect(lb.connection_error?(wrapped)).to eq(true) expect(lb.connection_error?(wrapped)).to eq(true)
end end
it 'returns true for deeply wrapped/nested errors' do it 'returns true for deeply wrapped/nested errors' do
top = twice_wrapped_exception(ActionView::Template::Error, ActiveRecord::StatementInvalid, NotImplementedError) top = twice_wrapped_exception(ActionView::Template::Error, ActiveRecord::StatementInvalid, ActiveRecord::ConnectionNotEstablished)
expect(lb.connection_error?(top)).to eq(true) expect(lb.connection_error?(top)).to eq(true)
end end
......
...@@ -20,11 +20,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do ...@@ -20,11 +20,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do
end end
describe '.stick_or_unstick' do describe '.stick_or_unstick' do
before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?)
.and_return(true)
end
it 'sticks or unsticks a single object and updates the Rack environment' do it 'sticks or unsticks a single object and updates the Rack environment' do
expect(Gitlab::Database::LoadBalancing::Sticking) expect(Gitlab::Database::LoadBalancing::Sticking)
.to receive(:unstick_or_continue_sticking) .to receive(:unstick_or_continue_sticking)
......
...@@ -5,14 +5,13 @@ require 'spec_helper' ...@@ -5,14 +5,13 @@ 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) { double.as_null_object } 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" } }
before do before do
skip_feature_flags_yaml_validation skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check skip_default_enabled_yaml_check
allow(::Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer).and_return(load_balancer)
end end
after do after do
......
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do
let(:middleware) { described_class.new } let(:middleware) { described_class.new }
let(:load_balancer) { double.as_null_object } 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' } }
...@@ -13,7 +13,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do ...@@ -13,7 +13,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do
before do before do
skip_feature_flags_yaml_validation skip_feature_flags_yaml_validation
skip_default_enabled_yaml_check skip_default_enabled_yaml_check
allow(::Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer).and_return(load_balancer)
replication_lag!(false) replication_lag!(false)
end end
......
...@@ -8,39 +8,24 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do ...@@ -8,39 +8,24 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
end end
describe '.stick_if_necessary' do describe '.stick_if_necessary' do
context 'when sticking is disabled' do it 'does not stick if no write was performed' do
it 'does not perform any sticking' do allow(Gitlab::Database::LoadBalancing::Session.current)
expect(described_class).not_to receive(:stick) .to receive(:performed_write?)
.and_return(false)
described_class.stick_if_necessary(:user, 42) expect(described_class).not_to receive(:stick)
end
end
context 'when sticking is enabled' do
before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?)
.and_return(true)
end
it 'does not stick if no write was performed' do
allow(Gitlab::Database::LoadBalancing::Session.current)
.to receive(:performed_write?)
.and_return(false)
expect(described_class).not_to receive(:stick) described_class.stick_if_necessary(:user, 42)
end
described_class.stick_if_necessary(:user, 42)
end
it 'sticks to the primary if a write was performed' do it 'sticks to the primary if a write was performed' do
allow(Gitlab::Database::LoadBalancing::Session.current) allow(Gitlab::Database::LoadBalancing::Session.current)
.to receive(:performed_write?) .to receive(:performed_write?)
.and_return(true) .and_return(true)
expect(described_class).to receive(:stick).with(:user, 42) expect(described_class).to receive(:stick).with(:user, 42)
described_class.stick_if_necessary(:user, 42) described_class.stick_if_necessary(:user, 42)
end
end end
end end
...@@ -155,35 +140,22 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do ...@@ -155,35 +140,22 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
end end
RSpec.shared_examples 'sticking' do RSpec.shared_examples 'sticking' do
context 'when sticking is disabled' do before do
it 'does not perform any sticking', :aggregate_failures do lb = double(:lb, primary_write_location: 'foo')
expect(described_class).not_to receive(:set_write_location_for)
expect(Gitlab::Database::LoadBalancing::Session.current).not_to receive(:use_primary!)
described_class.bulk_stick(:user, ids) allow(described_class).to receive(:load_balancer).and_return(lb)
end
end end
context 'when sticking is enabled' do it 'sticks an entity to the primary', :aggregate_failures do
before do ids.each do |id|
allow(Gitlab::Database::LoadBalancing).to receive(:configured?).and_return(true) expect(described_class).to receive(:set_write_location_for)
.with(:user, id, 'foo')
lb = double(:lb, primary_write_location: 'foo')
allow(described_class).to receive(:load_balancer).and_return(lb)
end end
it 'sticks an entity to the primary', :aggregate_failures do expect(Gitlab::Database::LoadBalancing::Session.current)
ids.each do |id| .to receive(:use_primary!)
expect(described_class).to receive(:set_write_location_for)
.with(:user, id, 'foo')
end
expect(Gitlab::Database::LoadBalancing::Session.current)
.to receive(:use_primary!)
subject subject
end
end end
end end
...@@ -202,63 +174,15 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do ...@@ -202,63 +174,15 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
end end
describe '.mark_primary_write_location' do describe '.mark_primary_write_location' do
context 'when enabled' do it 'updates the write location with the load balancer' do
before do lb = double(:lb, primary_write_location: 'foo')
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
allow(Gitlab::Database::LoadBalancing).to receive(:configured?).and_return(true)
end
it 'updates the write location with the load balancer' do allow(described_class).to receive(:load_balancer).and_return(lb)
lb = double(:lb, primary_write_location: 'foo')
allow(described_class).to receive(:load_balancer).and_return(lb)
expect(described_class).to receive(:set_write_location_for)
.with(:user, 42, 'foo')
described_class.mark_primary_write_location(:user, 42)
end
end
context 'when load balancing is configured but not enabled' do
before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false)
allow(Gitlab::Database::LoadBalancing).to receive(:configured?).and_return(true)
end
it 'updates the write location with the main ActiveRecord connection' do
allow(described_class).to receive(:load_balancer).and_return(nil)
expect(ActiveRecord::Base).to receive(:connection).and_call_original
expect(described_class).to receive(:set_write_location_for)
.with(:user, 42, anything)
described_class.mark_primary_write_location(:user, 42)
end
context 'when write location is nil' do
before do
allow(Gitlab::Database.main).to receive(:get_write_location).and_return(nil)
end
it 'does not update the write location' do
expect(described_class).not_to receive(:set_write_location_for)
described_class.mark_primary_write_location(:user, 42)
end
end
end
context 'when load balancing is disabled' do
before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false)
allow(Gitlab::Database::LoadBalancing).to receive(:configured?).and_return(false)
end
it 'updates the write location with the main ActiveRecord connection' do expect(described_class).to receive(:set_write_location_for)
expect(described_class).not_to receive(:set_write_location_for) .with(:user, 42, 'foo')
described_class.mark_primary_write_location(:user, 42) described_class.mark_primary_write_location(:user, 42)
end
end end
end end
......
...@@ -4,38 +4,14 @@ require 'spec_helper' ...@@ -4,38 +4,14 @@ require 'spec_helper'
RSpec.describe Gitlab::Database::LoadBalancing do RSpec.describe Gitlab::Database::LoadBalancing do
describe '.proxy' do describe '.proxy' do
before do it 'returns the connection proxy' do
@previous_proxy = ActiveRecord::Base.load_balancing_proxy proxy = double(:connection_proxy)
ActiveRecord::Base.load_balancing_proxy = connection_proxy allow(ActiveRecord::Base)
end .to receive(:load_balancing_proxy)
.and_return(proxy)
after do
ActiveRecord::Base.load_balancing_proxy = @previous_proxy
end
context 'when configured' do
let(:connection_proxy) { double(:connection_proxy) }
it 'returns the connection proxy' do
expect(subject.proxy).to eq(connection_proxy)
end
end
context 'when not configured' do
let(:connection_proxy) { nil }
it 'returns nil' do
expect(subject.proxy).to be_nil
end
it 'tracks an error to sentry' do
expect(Gitlab::ErrorTracking).to receive(:track_exception).with(
an_instance_of(subject::ProxyNotConfiguredError)
)
subject.proxy expect(described_class.proxy).to eq(proxy)
end
end end
end end
...@@ -50,46 +26,53 @@ RSpec.describe Gitlab::Database::LoadBalancing do ...@@ -50,46 +26,53 @@ RSpec.describe Gitlab::Database::LoadBalancing do
end end
end end
describe '.enable?' do describe '.enable_replicas?' do
before do context 'when hosts are specified' do
allow(described_class.configuration) before do
.to receive(:hosts) allow(described_class.configuration)
.and_return(%w(foo)) .to receive(:hosts)
end .and_return(%w(foo))
end
it 'returns false when no hosts are specified' do
allow(described_class.configuration).to receive(:hosts).and_return([])
expect(described_class.enable?).to eq(false) it 'returns true' do
end expect(described_class.enable_replicas?).to eq(true)
end
it 'returns true when Sidekiq is being used' do it 'returns true when Sidekiq is being used' do
allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true) allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true)
expect(described_class.enable?).to eq(true) expect(described_class.enable_replicas?).to eq(true)
end end
it 'returns false when running inside a Rake task' do it 'returns false when running inside a Rake task' do
allow(Gitlab::Runtime).to receive(:rake?).and_return(true) allow(Gitlab::Runtime).to receive(:rake?).and_return(true)
expect(described_class.enable?).to eq(false) expect(described_class.enable_replicas?).to eq(false)
end
end end
it 'returns true when load balancing should be enabled' do context 'when no hosts are specified but service discovery is enabled' do
allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(false) it 'returns true' do
allow(described_class.configuration).to receive(:hosts).and_return([])
allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(false)
expect(described_class.enable?).to eq(true) allow(described_class.configuration)
end .to receive(:service_discovery_enabled?)
.and_return(true)
it 'returns true when service discovery is enabled' do expect(described_class.enable_replicas?).to eq(true)
allow(described_class.configuration).to receive(:hosts).and_return([]) end
allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(false) end
allow(described_class.configuration) context 'when no hosts are specified and service discovery is disabled' do
.to receive(:service_discovery_enabled?) it 'returns false' do
.and_return(true) 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?).to eq(true) expect(described_class.enable_replicas?).to eq(false)
end
end end
end end
...@@ -121,41 +104,6 @@ RSpec.describe Gitlab::Database::LoadBalancing do ...@@ -121,41 +104,6 @@ RSpec.describe Gitlab::Database::LoadBalancing do
end end
end end
describe '.configure_proxy' do
before do
allow(ActiveRecord::Base).to receive(:load_balancing_proxy=)
end
it 'configures the connection proxy' do
described_class.configure_proxy
expect(ActiveRecord::Base).to have_received(:load_balancing_proxy=)
.with(Gitlab::Database::LoadBalancing::ConnectionProxy)
end
context 'when service discovery is enabled' do
it 'runs initial service discovery when configuring the connection proxy' do
discover = instance_spy(Gitlab::Database::LoadBalancing::ServiceDiscovery)
allow(described_class.configuration)
.to receive(:service_discovery)
.and_return({ record: 'foo' })
expect(Gitlab::Database::LoadBalancing::ServiceDiscovery)
.to receive(:new)
.with(
an_instance_of(Gitlab::Database::LoadBalancing::LoadBalancer),
an_instance_of(Hash)
)
.and_return(discover)
expect(discover).to receive(:perform_service_discovery)
described_class.configure_proxy
end
end
end
describe '.start_service_discovery' do describe '.start_service_discovery' do
it 'does not start if service discovery is disabled' do it 'does not start if service discovery is disabled' do
expect(Gitlab::Database::LoadBalancing::ServiceDiscovery) expect(Gitlab::Database::LoadBalancing::ServiceDiscovery)
...@@ -191,15 +139,41 @@ RSpec.describe Gitlab::Database::LoadBalancing do ...@@ -191,15 +139,41 @@ RSpec.describe Gitlab::Database::LoadBalancing do
end end
end end
describe '.db_role_for_connection' do describe '.perform_service_discovery' do
context 'when the load balancing is not configured' do it 'does nothing if service discovery is disabled' do
let(:connection) { ActiveRecord::Base.connection } expect(Gitlab::Database::LoadBalancing::ServiceDiscovery)
.not_to receive(:new)
it 'returns primary' do described_class.perform_service_discovery
expect(described_class.db_role_for_connection(connection)).to eq(:primary)
end
end 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
end
end
describe '.db_role_for_connection' do
context 'when the NullPool is used for connection' do context 'when the NullPool is used for connection' do
let(:pool) { ActiveRecord::ConnectionAdapters::NullPool.new } let(:pool) { ActiveRecord::ConnectionAdapters::NullPool.new }
let(:connection) { double(:connection, pool: pool) } let(:connection) { double(:connection, pool: pool) }
...@@ -274,10 +248,6 @@ RSpec.describe Gitlab::Database::LoadBalancing do ...@@ -274,10 +248,6 @@ RSpec.describe Gitlab::Database::LoadBalancing do
end end
end end
before do
model.singleton_class.prepend ::Gitlab::Database::LoadBalancing::ActiveRecordProxy
end
where(:queries, :include_transaction, :expected_results) do where(:queries, :include_transaction, :expected_results) do
[ [
# Read methods # Read methods
......
...@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Database::WithLockRetries do ...@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Database::WithLockRetries do
let(:logger) { Gitlab::Database::WithLockRetries::NULL_LOGGER } let(:logger) { Gitlab::Database::WithLockRetries::NULL_LOGGER }
let(:subject) { described_class.new(env: env, logger: logger, allow_savepoints: allow_savepoints, timing_configuration: timing_configuration) } let(:subject) { described_class.new(env: env, logger: logger, allow_savepoints: allow_savepoints, timing_configuration: timing_configuration) }
let(:allow_savepoints) { true } let(:allow_savepoints) { true }
let(:connection) { ActiveRecord::Base.connection } let(:connection) { ActiveRecord::Base.retrieve_connection }
let(:timing_configuration) do let(:timing_configuration) do
[ [
......
...@@ -185,10 +185,23 @@ RSpec.describe Gitlab::Database do ...@@ -185,10 +185,23 @@ RSpec.describe Gitlab::Database do
describe '.db_config_name' do describe '.db_config_name' do
it 'returns the db_config name for the connection' do it 'returns the db_config name for the connection' do
connection = ActiveRecord::Base.connection model = ActiveRecord::Base
expect(described_class.db_config_name(connection)).to be_a(String) # This is a ConnectionProxy
expect(described_class.db_config_name(connection)).to eq(connection.pool.db_config.name) expect(described_class.db_config_name(model.connection))
.to eq('unknown')
# This is an actual connection
expect(described_class.db_config_name(model.retrieve_connection))
.to eq('main')
end
context 'when replicas are configured', :db_load_balancing do
it 'returns the name for a replica' do
replica = ActiveRecord::Base.connection.load_balancer.host
expect(described_class.db_config_name(replica)).to eq('main_replica')
end
end end
end end
...@@ -279,7 +292,7 @@ RSpec.describe Gitlab::Database do ...@@ -279,7 +292,7 @@ RSpec.describe Gitlab::Database do
expect(event).not_to be_nil expect(event).not_to be_nil
expect(event.duration).to be > 0.0 expect(event.duration).to be > 0.0
expect(event.payload).to a_hash_including( expect(event.payload).to a_hash_including(
connection: be_a(ActiveRecord::ConnectionAdapters::AbstractAdapter) connection: be_a(Gitlab::Database::LoadBalancing::ConnectionProxy)
) )
end end
end end
...@@ -296,7 +309,7 @@ RSpec.describe Gitlab::Database do ...@@ -296,7 +309,7 @@ RSpec.describe Gitlab::Database do
expect(event).not_to be_nil expect(event).not_to be_nil
expect(event.duration).to be > 0.0 expect(event.duration).to be > 0.0
expect(event.payload).to a_hash_including( expect(event.payload).to a_hash_including(
connection: be_a(ActiveRecord::ConnectionAdapters::AbstractAdapter) connection: be_a(Gitlab::Database::LoadBalancing::ConnectionProxy)
) )
end end
end end
...@@ -319,7 +332,7 @@ RSpec.describe Gitlab::Database do ...@@ -319,7 +332,7 @@ RSpec.describe Gitlab::Database do
expect(event).not_to be_nil expect(event).not_to be_nil
expect(event.duration).to be > 0.0 expect(event.duration).to be > 0.0
expect(event.payload).to a_hash_including( expect(event.payload).to a_hash_including(
connection: be_a(ActiveRecord::ConnectionAdapters::AbstractAdapter) connection: be_a(Gitlab::Database::LoadBalancing::ConnectionProxy)
) )
end end
end end
...@@ -340,7 +353,7 @@ RSpec.describe Gitlab::Database do ...@@ -340,7 +353,7 @@ RSpec.describe Gitlab::Database do
expect(event).not_to be_nil expect(event).not_to be_nil
expect(event.duration).to be > 0.0 expect(event.duration).to be > 0.0
expect(event.payload).to a_hash_including( expect(event.payload).to a_hash_including(
connection: be_a(ActiveRecord::ConnectionAdapters::AbstractAdapter) connection: be_a(Gitlab::Database::LoadBalancing::ConnectionProxy)
) )
end end
end end
......
...@@ -158,26 +158,15 @@ RSpec.describe Gitlab::ImportExport::Json::StreamingSerializer do ...@@ -158,26 +158,15 @@ 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' do context 'when feature flag load_balancing_for_export_workers is enabled', :db_load_balancing 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
context 'when enabled', :db_load_balancing do it 'reads from replica' do
it 'reads from 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
subject.execute subject.execute
end
end
context 'when disabled' do
it 'reads from primary' do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false)
expect(Gitlab::Database::LoadBalancing::Session.current).not_to receive(:use_replicas_for_read_queries)
subject.execute
end
end end
end end
......
...@@ -107,69 +107,44 @@ RSpec.describe Gitlab::InstrumentationHelper do ...@@ -107,69 +107,44 @@ RSpec.describe Gitlab::InstrumentationHelper do
end end
end end
context 'when load balancing is enabled' do it 'includes DB counts' do
before do subject
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
end
it 'includes DB counts' do
subject
expect(payload).to include(db_replica_count: 0,
db_replica_cached_count: 0,
db_primary_count: 0,
db_primary_cached_count: 0,
db_primary_wal_count: 0,
db_replica_wal_count: 0,
db_primary_wal_cached_count: 0,
db_replica_wal_cached_count: 0)
end
context 'when replica caught up search was made' do
before do
Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2
Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1
end
it 'includes related metrics' do expect(payload).to include(db_replica_count: 0,
subject db_replica_cached_count: 0,
db_primary_count: 0,
db_primary_cached_count: 0,
db_primary_wal_count: 0,
db_replica_wal_count: 0,
db_primary_wal_cached_count: 0,
db_replica_wal_cached_count: 0)
end
expect(payload).to include(caught_up_replica_pick_ok: 2) context 'when replica caught up search was made' do
expect(payload).to include(caught_up_replica_pick_fail: 1) before do
end Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2
Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1
end end
context 'when only a single counter was updated' do it 'includes related metrics' do
before do subject
Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 1
Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = nil
end
it 'includes only that counter into logging' do
subject
expect(payload).to include(caught_up_replica_pick_ok: 1) expect(payload).to include(caught_up_replica_pick_ok: 2)
expect(payload).not_to include(:caught_up_replica_pick_fail) expect(payload).to include(caught_up_replica_pick_fail: 1)
end
end end
end end
context 'when load balancing is disabled' do context 'when only a single counter was updated' do
before do before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 1
Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = nil
end end
it 'does not include DB counts' do it 'includes only that counter into logging' do
subject subject
expect(payload).not_to include(db_replica_count: 0, expect(payload).to include(caught_up_replica_pick_ok: 1)
db_replica_cached_count: 0, expect(payload).not_to include(:caught_up_replica_pick_fail)
db_primary_count: 0,
db_primary_cached_count: 0,
db_primary_wal_count: 0,
db_replica_wal_count: 0,
db_primary_wal_cached_count: 0,
db_replica_wal_cached_count: 0)
end end
end end
end end
......
...@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do ...@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
let(:env) { {} } let(:env) { {} }
let(:subscriber) { described_class.new } let(:subscriber) { described_class.new }
let(:connection) { ActiveRecord::Base.connection } let(:connection) { ActiveRecord::Base.retrieve_connection }
let(:db_config_name) { ::Gitlab::Database.db_config_name(connection) } let(:db_config_name) { ::Gitlab::Database.db_config_name(connection) }
describe '#transaction' do describe '#transaction' do
...@@ -135,7 +135,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do ...@@ -135,7 +135,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
end end
it_behaves_like 'record ActiveRecord metrics' it_behaves_like 'record ActiveRecord metrics'
it_behaves_like 'store ActiveRecord info in RequestStore' it_behaves_like 'store ActiveRecord info in RequestStore', :primary
end end
end end
...@@ -195,11 +195,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do ...@@ -195,11 +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 } }
before do context 'query using a connection to a replica', :db_load_balancing do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
end
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
......
...@@ -5,10 +5,6 @@ require 'spec_helper' ...@@ -5,10 +5,6 @@ require 'spec_helper'
RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store do RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store do
let(:subscriber) { described_class.new } let(:subscriber) { described_class.new }
before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
end
describe '#caught_up_replica_pick' do describe '#caught_up_replica_pick' do
shared_examples 'having payload result value' do |result, counter_name| shared_examples 'having payload result value' do |result, counter_name|
subject { subscriber.caught_up_replica_pick(event) } subject { subscriber.caught_up_replica_pick(event) }
......
...@@ -243,8 +243,22 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do ...@@ -243,8 +243,22 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do
expected_end_payload.merge( expected_end_payload.merge(
'db_duration_s' => a_value >= 0.1, 'db_duration_s' => a_value >= 0.1,
'db_count' => a_value >= 1, 'db_count' => a_value >= 1,
'db_cached_count' => 0, "db_replica_#{db_config_name}_count" => 0,
'db_write_count' => 0 'db_replica_duration_s' => a_value >= 0,
'db_primary_count' => a_value >= 1,
"db_primary_#{db_config_name}_count" => a_value >= 1,
'db_primary_duration_s' => a_value > 0,
"db_primary_#{db_config_name}_duration_s" => a_value > 0
)
end
let(:end_payload) do
start_payload.merge(db_payload_defaults).merge(
'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec',
'job_status' => 'done',
'duration_s' => 0.0,
'completed_at' => timestamp.to_f,
'cpu_s' => 1.111112
) )
end end
...@@ -274,59 +288,9 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do ...@@ -274,59 +288,9 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do
end end
end end
context 'when load balancing is disabled' do
before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false)
end
let(:expected_end_payload_with_db) do
expected_end_payload.merge(
'db_duration_s' => a_value >= 0.1,
'db_count' => a_value >= 1,
'db_cached_count' => 0,
'db_write_count' => 0
)
end
include_examples 'performs database queries'
end
context 'when load balancing is enabled', :db_load_balancing do context 'when load balancing is enabled', :db_load_balancing do
let(:db_config_name) { ::Gitlab::Database.db_config_name(ApplicationRecord.connection) } let(:db_config_name) do
::Gitlab::Database.db_config_name(ApplicationRecord.retrieve_connection)
let(:expected_db_payload_defaults) do
metrics =
::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_counter_keys +
::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_duration_keys +
::Gitlab::Metrics::Subscribers::ActiveRecord.db_counter_keys +
[:db_duration_s]
metrics.each_with_object({}) do |key, result|
result[key.to_s] = 0
end
end
let(:expected_end_payload_with_db) do
expected_end_payload.merge(expected_db_payload_defaults).merge(
'db_duration_s' => a_value >= 0.1,
'db_count' => a_value >= 1,
"db_replica_#{db_config_name}_count" => 0,
'db_replica_duration_s' => a_value >= 0,
'db_primary_count' => a_value >= 1,
"db_primary_#{db_config_name}_count" => a_value >= 1,
'db_primary_duration_s' => a_value > 0,
"db_primary_#{db_config_name}_duration_s" => a_value > 0
)
end
let(:end_payload) do
start_payload.merge(expected_db_payload_defaults).merge(
'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec',
'job_status' => 'done',
'duration_s' => 0.0,
'completed_at' => timestamp.to_f,
'cpu_s' => 1.111112
)
end end
include_examples 'performs database queries' include_examples 'performs database queries'
......
...@@ -250,60 +250,30 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do ...@@ -250,60 +250,30 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do
end end
end end
context 'when load_balancing is enabled' do describe '#call' do
before do context 'when worker declares data consistency' do
allow(::Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) include_context 'worker declaring data consistency'
end
describe '#call' do
context 'when worker declares data consistency' do
include_context 'worker declaring data consistency'
it 'increments load balancing counter with defined data consistency' do
process_job
expect(load_balancing_metric).to have_received(:increment).with(
a_hash_including(
data_consistency: :delayed,
load_balancing_strategy: 'replica'
), 1)
end
end
context 'when worker does not declare data consistency' do
it 'increments load balancing counter with default data consistency' do
process_job
expect(load_balancing_metric).to have_received(:increment).with(
a_hash_including(
data_consistency: :always,
load_balancing_strategy: 'primary'
), 1)
end
end
end
end
context 'when load_balancing is disabled' do
include_context 'worker declaring data consistency'
before do it 'increments load balancing counter with defined data consistency' do
allow(::Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) process_job
end
describe '#initialize' do
it 'does not set load_balancing metrics' do
expect(Gitlab::Metrics).not_to receive(:counter).with(:sidekiq_load_balancing_count, anything)
subject expect(load_balancing_metric).to have_received(:increment).with(
a_hash_including(
data_consistency: :delayed,
load_balancing_strategy: 'replica'
), 1)
end end
end end
describe '#call' do context 'when worker does not declare data consistency' do
it 'does not increment load balancing counter' do it 'increments load balancing counter with default data consistency' do
process_job process_job
expect(load_balancing_metric).not_to have_received(:increment) expect(load_balancing_metric).to have_received(:increment).with(
a_hash_including(
data_consistency: :always,
load_balancing_strategy: 'primary'
), 1)
end end
end end
end end
......
...@@ -28,9 +28,8 @@ RSpec.describe Gitlab::SidekiqMiddleware do ...@@ -28,9 +28,8 @@ RSpec.describe Gitlab::SidekiqMiddleware do
stub_const('TestWorker', worker_class) stub_const('TestWorker', worker_class)
end end
shared_examples "a middleware chain" do |load_balancing_enabled| shared_examples "a middleware chain" do
before do before do
allow(::Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(load_balancing_enabled)
configurator.call(chain) configurator.call(chain)
end end
...@@ -45,10 +44,10 @@ RSpec.describe Gitlab::SidekiqMiddleware do ...@@ -45,10 +44,10 @@ RSpec.describe Gitlab::SidekiqMiddleware do
end end
end end
shared_examples "a middleware chain for mailer" do |load_balancing_enabled| shared_examples "a middleware chain for mailer" do
let(:worker_class) { ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper } let(:worker_class) { ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper }
it_behaves_like "a middleware chain", load_balancing_enabled it_behaves_like "a middleware chain"
end end
describe '.server_configurator' do describe '.server_configurator' do
...@@ -105,25 +104,8 @@ RSpec.describe Gitlab::SidekiqMiddleware do ...@@ -105,25 +104,8 @@ RSpec.describe Gitlab::SidekiqMiddleware do
end end
context "all optional middlewares on" do context "all optional middlewares on" do
context "when load balancing is enabled" do it_behaves_like "a middleware chain"
before do it_behaves_like "a middleware chain for mailer"
allow(::Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer, :release_host)
end
it_behaves_like "a middleware chain", true
it_behaves_like "a middleware chain for mailer", true
end
context "when load balancing is disabled" do
let(:disabled_sidekiq_middlewares) do
[
Gitlab::Database::LoadBalancing::SidekiqServerMiddleware
]
end
it_behaves_like "a middleware chain", false
it_behaves_like "a middleware chain for mailer", false
end
end end
context "all optional middlewares off" do context "all optional middlewares off" do
...@@ -135,36 +117,16 @@ RSpec.describe Gitlab::SidekiqMiddleware do ...@@ -135,36 +117,16 @@ RSpec.describe Gitlab::SidekiqMiddleware do
) )
end end
context "when load balancing is enabled" do let(:disabled_sidekiq_middlewares) do
let(:disabled_sidekiq_middlewares) do [
[ Gitlab::SidekiqMiddleware::ServerMetrics,
Gitlab::SidekiqMiddleware::ServerMetrics, Gitlab::SidekiqMiddleware::ArgumentsLogger,
Gitlab::SidekiqMiddleware::ArgumentsLogger, Gitlab::SidekiqMiddleware::MemoryKiller
Gitlab::SidekiqMiddleware::MemoryKiller ]
]
end
before do
allow(::Gitlab::Database::LoadBalancing).to receive_message_chain(:proxy, :load_balancer, :release_host)
end
it_behaves_like "a middleware chain", true
it_behaves_like "a middleware chain for mailer", true
end end
context "when load balancing is disabled" do it_behaves_like "a middleware chain"
let(:disabled_sidekiq_middlewares) do it_behaves_like "a middleware chain for mailer"
[
Gitlab::SidekiqMiddleware::ServerMetrics,
Gitlab::SidekiqMiddleware::ArgumentsLogger,
Gitlab::SidekiqMiddleware::MemoryKiller,
Gitlab::Database::LoadBalancing::SidekiqServerMiddleware
]
end
it_behaves_like "a middleware chain", false
it_behaves_like "a middleware chain for mailer", false
end
end end
end end
...@@ -186,30 +148,7 @@ RSpec.describe Gitlab::SidekiqMiddleware do ...@@ -186,30 +148,7 @@ RSpec.describe Gitlab::SidekiqMiddleware do
] ]
end end
context "when load balancing is disabled" do it_behaves_like "a middleware chain"
let(:disabled_sidekiq_middlewares) do it_behaves_like "a middleware chain for mailer"
[
Gitlab::Database::LoadBalancing::SidekiqClientMiddleware
]
end
it_behaves_like "a middleware chain", false
it_behaves_like "a middleware chain for mailer", false
# Sidekiq documentation states that the worker class could be a string
# or a class reference. We should test for both
context "worker_class as string value" do
let(:worker_args) { [worker_class.to_s, { 'args' => job_args }, queue, redis_pool] }
let(:middleware_expected_args) { [worker_class.to_s, hash_including({ 'args' => job_args }), queue, redis_pool] }
it_behaves_like "a middleware chain", false
it_behaves_like "a middleware chain for mailer", false
end
end
context "when load balancing is enabled" do
it_behaves_like "a middleware chain", true
it_behaves_like "a middleware chain for mailer", true
end
end end
end end
...@@ -53,154 +53,82 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do ...@@ -53,154 +53,82 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
allow(connection_primary_2).to receive(:transaction_open?).and_return(true) allow(connection_primary_2).to receive(:transaction_open?).and_return(true)
allow(connection_unknown).to receive(:transaction_open?).and_return(false) allow(connection_unknown).to receive(:transaction_open?).and_return(false)
allow(::Gitlab::Database).to receive(:db_config_name).and_return('the_db_config_name') allow(::Gitlab::Database).to receive(:db_config_name).and_return('the_db_config_name')
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_replica).and_return(:replica)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_1).and_return(:primary)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_2).and_return(:primary)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_unknown).and_return(nil)
end end
context 'when database load balancing is not enabled' do it 'includes db role data and db_config_name name' do
it 'subscribes and store data into peek views' do Timecop.freeze(2021, 2, 23, 10, 0) do
Timecop.freeze(2021, 2, 23, 10, 0) do ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1)
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2)
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3)
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4)
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4) end
end
expect(subject.results).to match( expect(subject.results).to match(
calls: 4, calls: 4,
summary: { summary: {
"Cached" => 1, "Cached" => 1,
"In a transaction" => 1 "In a transaction" => 1,
}, "Role: Primary" => 2,
duration: '10000.00ms', "Role: Replica" => 1,
warnings: ["active-record duration: 10000.0 over 3000"], "Role: Unknown" => 1
details: contain_exactly( },
a_hash_including( duration: '10000.00ms',
start: be_a(Time), warnings: ["active-record duration: 10000.0 over 3000"],
cached: '', details: contain_exactly(
transaction: '', a_hash_including(
duration: 1000.0, start: be_a(Time),
sql: 'SELECT * FROM users WHERE id = 10', cached: '',
db_config_name: "Config name: the_db_config_name" transaction: '',
), duration: 1000.0,
a_hash_including( sql: 'SELECT * FROM users WHERE id = 10',
start: be_a(Time), db_role: 'Role: Primary',
cached: 'Cached', db_config_name: "Config name: the_db_config_name"
transaction: '', ),
duration: 2000.0, a_hash_including(
sql: 'SELECT * FROM users WHERE id = 10', start: be_a(Time),
db_config_name: "Config name: the_db_config_name" cached: 'Cached',
), transaction: '',
a_hash_including( duration: 2000.0,
start: be_a(Time), sql: 'SELECT * FROM users WHERE id = 10',
cached: '', db_role: 'Role: Replica',
transaction: 'In a transaction', db_config_name: "Config name: the_db_config_name"
duration: 3000.0, ),
sql: 'UPDATE users SET admin = true WHERE id = 10', a_hash_including(
db_config_name: "Config name: the_db_config_name" start: be_a(Time),
), cached: '',
a_hash_including( transaction: 'In a transaction',
start: be_a(Time), duration: 3000.0,
cached: '', sql: 'UPDATE users SET admin = true WHERE id = 10',
transaction: '', db_role: 'Role: Primary',
duration: 4000.0, db_config_name: "Config name: the_db_config_name"
sql: 'SELECT VERSION()', ),
db_config_name: "Config name: the_db_config_name" a_hash_including(
) start: be_a(Time),
cached: '',
transaction: '',
duration: 4000.0,
sql: 'SELECT VERSION()',
db_role: 'Role: Unknown',
db_config_name: "Config name: the_db_config_name"
) )
) )
end )
context 'when the GITLAB_MULTIPLE_DATABASE_METRICS env var is disabled' do
before do
stub_env('GITLAB_MULTIPLE_DATABASE_METRICS', nil)
end
it 'does not include db_config_name field' do
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1)
expect(subject.results[:details][0][:db_config_name]).to be_nil
end
end
end end
context 'when database load balancing is enabled' do context 'when the GITLAB_MULTIPLE_DATABASE_METRICS env var is disabled' do
before do before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) stub_env('GITLAB_MULTIPLE_DATABASE_METRICS', nil)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_replica).and_return(:replica)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_1).and_return(:primary)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_2).and_return(:primary)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_unknown).and_return(nil)
end end
it 'includes db role data and db_config_name name' do it 'does not include db_config_name field' do
Timecop.freeze(2021, 2, 23, 10, 0) do ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1)
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1)
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2)
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3)
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4)
end
expect(subject.results).to match(
calls: 4,
summary: {
"Cached" => 1,
"In a transaction" => 1,
"Role: Primary" => 2,
"Role: Replica" => 1,
"Role: Unknown" => 1
},
duration: '10000.00ms',
warnings: ["active-record duration: 10000.0 over 3000"],
details: contain_exactly(
a_hash_including(
start: be_a(Time),
cached: '',
transaction: '',
duration: 1000.0,
sql: 'SELECT * FROM users WHERE id = 10',
db_role: 'Role: Primary',
db_config_name: "Config name: the_db_config_name"
),
a_hash_including(
start: be_a(Time),
cached: 'Cached',
transaction: '',
duration: 2000.0,
sql: 'SELECT * FROM users WHERE id = 10',
db_role: 'Role: Replica',
db_config_name: "Config name: the_db_config_name"
),
a_hash_including(
start: be_a(Time),
cached: '',
transaction: 'In a transaction',
duration: 3000.0,
sql: 'UPDATE users SET admin = true WHERE id = 10',
db_role: 'Role: Primary',
db_config_name: "Config name: the_db_config_name"
),
a_hash_including(
start: be_a(Time),
cached: '',
transaction: '',
duration: 4000.0,
sql: 'SELECT VERSION()',
db_role: 'Role: Unknown',
db_config_name: "Config name: the_db_config_name"
)
)
)
end
context 'when the GITLAB_MULTIPLE_DATABASE_METRICS env var is disabled' do
before do
stub_env('GITLAB_MULTIPLE_DATABASE_METRICS', nil)
end
it 'does not include db_config_name field' do
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1)
expect(subject.results[:details][0][:db_config_name]).to be_nil expect(subject.results[:details][0][:db_config_name]).to be_nil
end
end end
end end
end end
...@@ -194,7 +194,7 @@ RSpec.describe ApplicationRecord do ...@@ -194,7 +194,7 @@ RSpec.describe ApplicationRecord do
end end
context 'with database load balancing' do context 'with database load balancing' do
let(:session) { double(:session) } let(:session) { Gitlab::Database::LoadBalancing::Session.new }
before do before do
allow(::Gitlab::Database::LoadBalancing::Session).to receive(:current).and_return(session) allow(::Gitlab::Database::LoadBalancing::Session).to receive(:current).and_return(session)
......
...@@ -2816,8 +2816,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -2816,8 +2816,9 @@ 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
expect(control2.count).to eq(control1.count + extra_update_queries + extra_generic_commit_status_validation_queries) expect(control2.count).to eq(control1.count + extra_update_queries + extra_generic_commit_status_validation_queries + extra_load_balancer_queries)
end end
end end
......
...@@ -383,9 +383,6 @@ RSpec.describe Ci::Runner do ...@@ -383,9 +383,6 @@ 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)
allow(Gitlab::Database::LoadBalancing).to receive(:enable?)
.and_return(true)
expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick)
.with(:runner, runner.id) .with(:runner, runner.id)
......
...@@ -47,8 +47,6 @@ RSpec.describe UserProjectAccessChangedService do ...@@ -47,8 +47,6 @@ RSpec.describe UserProjectAccessChangedService do
let(:service) { UserProjectAccessChangedService.new([1, 2]) } let(:service) { UserProjectAccessChangedService.new([1, 2]) }
before do before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
expect(AuthorizedProjectsWorker).to receive(:bulk_perform_and_wait) expect(AuthorizedProjectsWorker).to receive(:bulk_perform_and_wait)
.with([[1], [2]]) .with([[1], [2]])
.and_return(10) .and_return(10)
......
...@@ -2,8 +2,6 @@ ...@@ -2,8 +2,6 @@
RSpec.configure do |config| RSpec.configure do |config|
config.before(:each, :db_load_balancing) do config.before(:each, :db_load_balancing) do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
config = Gitlab::Database::LoadBalancing::Configuration config = Gitlab::Database::LoadBalancing::Configuration
.new(ActiveRecord::Base, [Gitlab::Database.main.config['host']]) .new(ActiveRecord::Base, [Gitlab::Database.main.config['host']])
lb = ::Gitlab::Database::LoadBalancing::LoadBalancer.new(config) lb = ::Gitlab::Database::LoadBalancing::LoadBalancer.new(config)
......
...@@ -39,17 +39,25 @@ RSpec.shared_context 'structured_logger' do ...@@ -39,17 +39,25 @@ RSpec.shared_context 'structured_logger' do
) )
end end
let(:db_payload_defaults) do
metrics =
::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_counter_keys +
::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_duration_keys +
::Gitlab::Metrics::Subscribers::ActiveRecord.db_counter_keys +
[:db_duration_s]
metrics.each_with_object({}) do |key, result|
result[key.to_s] = 0
end
end
let(:end_payload) do let(:end_payload) do
start_payload.merge( start_payload.merge(db_payload_defaults).merge(
'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec', 'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec',
'job_status' => 'done', 'job_status' => 'done',
'duration_s' => 0.0, 'duration_s' => 0.0,
'completed_at' => timestamp.to_f, 'completed_at' => timestamp.to_f,
'cpu_s' => 1.111112, 'cpu_s' => 1.111112
'db_duration_s' => 0.0,
'db_cached_count' => 0,
'db_count' => 0,
'db_write_count' => 0
) )
end end
......
...@@ -15,6 +15,7 @@ RSpec.shared_context 'server metrics with mocked prometheus' do ...@@ -15,6 +15,7 @@ RSpec.shared_context 'server metrics with mocked prometheus' do
let(:redis_seconds_metric) { double('redis seconds metric') } let(:redis_seconds_metric) { double('redis seconds metric') }
let(:elasticsearch_seconds_metric) { double('elasticsearch seconds metric') } let(:elasticsearch_seconds_metric) { double('elasticsearch seconds metric') }
let(:elasticsearch_requests_total) { double('elasticsearch calls total metric') } let(:elasticsearch_requests_total) { double('elasticsearch calls total metric') }
let(:load_balancing_metric) { double('load balancing metric') }
before do before do
allow(Gitlab::Metrics).to receive(:histogram).and_call_original allow(Gitlab::Metrics).to receive(:histogram).and_call_original
...@@ -31,6 +32,7 @@ RSpec.shared_context 'server metrics with mocked prometheus' do ...@@ -31,6 +32,7 @@ RSpec.shared_context 'server metrics with mocked prometheus' do
allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_jobs_retried_total, anything).and_return(retried_total_metric) allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_jobs_retried_total, anything).and_return(retried_total_metric)
allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_redis_requests_total, anything).and_return(redis_requests_total) allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_redis_requests_total, anything).and_return(redis_requests_total)
allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_elasticsearch_requests_total, anything).and_return(elasticsearch_requests_total) allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_elasticsearch_requests_total, anything).and_return(elasticsearch_requests_total)
allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_load_balancing_count, anything).and_return(load_balancing_metric)
allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_running_jobs, anything, {}, :all).and_return(running_jobs_metric) allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_running_jobs, anything, {}, :all).and_return(running_jobs_metric)
allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_concurrency, anything, {}, :all).and_return(concurrency_metric) allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_concurrency, anything, {}, :all).and_return(concurrency_metric)
......
...@@ -4,14 +4,20 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| ...@@ -4,14 +4,20 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role|
let(:db_config_name) { ::Gitlab::Database.db_config_names.first } let(:db_config_name) { ::Gitlab::Database.db_config_names.first }
let(:expected_payload_defaults) do let(:expected_payload_defaults) do
result = {}
metrics = metrics =
::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_counter_keys + ::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_counter_keys +
::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_duration_keys +
::Gitlab::Metrics::Subscribers::ActiveRecord.db_counter_keys ::Gitlab::Metrics::Subscribers::ActiveRecord.db_counter_keys
metrics.each_with_object({}) do |key, result| metrics.each do |key|
result[key] = 0 result[key] = 0
end end
::Gitlab::Metrics::Subscribers::ActiveRecord.load_balancing_metric_duration_keys.each do |key|
result[key] = 0.0
end
result
end end
def transform_hash(hash, another_hash) def transform_hash(hash, another_hash)
...@@ -36,8 +42,8 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| ...@@ -36,8 +42,8 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role|
"db_primary_#{db_config_name}_cached_count": record_cached_query ? 1 : 0, "db_primary_#{db_config_name}_cached_count": record_cached_query ? 1 : 0,
db_primary_count: record_query ? 1 : 0, db_primary_count: record_query ? 1 : 0,
"db_primary_#{db_config_name}_count": record_query ? 1 : 0, "db_primary_#{db_config_name}_count": record_query ? 1 : 0,
db_primary_duration_s: record_query ? 0.002 : 0, db_primary_duration_s: record_query ? 0.002 : 0.0,
"db_primary_#{db_config_name}_duration_s": record_query ? 0.002 : 0, "db_primary_#{db_config_name}_duration_s": record_query ? 0.002 : 0.0,
db_primary_wal_count: record_wal_query ? 1 : 0, db_primary_wal_count: record_wal_query ? 1 : 0,
"db_primary_#{db_config_name}_wal_count": record_wal_query ? 1 : 0, "db_primary_#{db_config_name}_wal_count": record_wal_query ? 1 : 0,
db_primary_wal_cached_count: record_wal_query && record_cached_query ? 1 : 0, db_primary_wal_cached_count: record_wal_query && record_cached_query ? 1 : 0,
...@@ -52,19 +58,29 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| ...@@ -52,19 +58,29 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role|
"db_replica_#{db_config_name}_cached_count": record_cached_query ? 1 : 0, "db_replica_#{db_config_name}_cached_count": record_cached_query ? 1 : 0,
db_replica_count: record_query ? 1 : 0, db_replica_count: record_query ? 1 : 0,
"db_replica_#{db_config_name}_count": record_query ? 1 : 0, "db_replica_#{db_config_name}_count": record_query ? 1 : 0,
db_replica_duration_s: record_query ? 0.002 : 0, db_replica_duration_s: record_query ? 0.002 : 0.0,
"db_replica_#{db_config_name}_duration_s": record_query ? 0.002 : 0, "db_replica_#{db_config_name}_duration_s": record_query ? 0.002 : 0.0,
db_replica_wal_count: record_wal_query ? 1 : 0, db_replica_wal_count: record_wal_query ? 1 : 0,
"db_replica_#{db_config_name}_wal_count": record_wal_query ? 1 : 0, "db_replica_#{db_config_name}_wal_count": record_wal_query ? 1 : 0,
db_replica_wal_cached_count: record_wal_query && record_cached_query ? 1 : 0, db_replica_wal_cached_count: record_wal_query && record_cached_query ? 1 : 0,
"db_replica_#{db_config_name}_wal_cached_count": record_wal_query && record_cached_query ? 1 : 0 "db_replica_#{db_config_name}_wal_cached_count": record_wal_query && record_cached_query ? 1 : 0
}) })
else else
{ transform_hash(expected_payload_defaults, {
db_count: record_query ? 1 : 0, db_count: record_query ? 1 : 0,
db_write_count: record_write_query ? 1 : 0, db_write_count: record_write_query ? 1 : 0,
db_cached_count: record_cached_query ? 1 : 0 db_cached_count: record_cached_query ? 1 : 0,
} db_primary_cached_count: 0,
"db_primary_#{db_config_name}_cached_count": 0,
db_primary_count: 0,
"db_primary_#{db_config_name}_count": 0,
db_primary_duration_s: 0.0,
"db_primary_#{db_config_name}_duration_s": 0.0,
db_primary_wal_count: 0,
"db_primary_#{db_config_name}_wal_count": 0,
db_primary_wal_cached_count: 0,
"db_primary_#{db_config_name}_wal_cached_count": 0
})
end end
expect(described_class.db_counter_payload).to eq(expected) expect(described_class.db_counter_payload).to eq(expected)
...@@ -89,7 +105,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| ...@@ -89,7 +105,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role|
end end
RSpec.shared_examples 'record ActiveRecord metrics in a metrics transaction' do |db_role| RSpec.shared_examples 'record ActiveRecord metrics in a metrics transaction' do |db_role|
let(:db_config_name) { ::Gitlab::Database.db_config_name(ApplicationRecord.connection) } let(:db_config_name) { ::Gitlab::Database.db_config_name(ApplicationRecord.retrieve_connection) }
it 'increments only db counters' do it 'increments only db counters' do
if record_query if record_query
......
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