diff --git a/lib/gitlab/database/load_balancing/primary_host.rb b/lib/gitlab/database/load_balancing/primary_host.rb
index e379652c260f04d97edbe3557be98d1bb8af625e..7070cc54d4bb5afb971a3488fda0566a8ef8b55d 100644
--- a/lib/gitlab/database/load_balancing/primary_host.rb
+++ b/lib/gitlab/database/load_balancing/primary_host.rb
@@ -11,6 +11,12 @@ module LoadBalancing
# balancing is enabled, but no replicas have been configured (= the
# default case).
class PrimaryHost
+ WAL_ERROR_MESSAGE = <<~MSG.strip
+ Obtaining WAL information when not using any replicas results in
+ redundant queries, and may break installations that don't support
+ streaming replication (e.g. AWS' Aurora database).
+ MSG
+
def initialize(load_balancer)
@load_balancer = load_balancer
end
@@ -51,30 +57,16 @@ def online?
end
def primary_write_location
- @load_balancer.primary_write_location
+ raise NotImplementedError, WAL_ERROR_MESSAGE
end
def database_replica_location
- row = query_and_release(<<-SQL.squish)
- SELECT pg_last_wal_replay_lsn()::text AS location
- SQL
-
- row['location'] if row.any?
- rescue *Host::CONNECTION_ERRORS
- nil
+ raise NotImplementedError, WAL_ERROR_MESSAGE
end
def caught_up?(_location)
true
end
-
- def query_and_release(sql)
- connection.select_all(sql).first || {}
- rescue StandardError
- {}
- ensure
- release_connection
- end
end
end
end
diff --git a/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb b/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb
index ebf90ebf9d0ce0c5b04f20feff79e58229d9e1af..62dfe75a85128fc79f3a7fe521fc7567fed38c72 100644
--- a/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb
+++ b/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb
@@ -42,6 +42,9 @@ def set_data_consistency_locations!(job)
end
def wal_location_for(load_balancer)
+ # When only using the primary there's no need for any WAL queries.
+ return if load_balancer.primary_only?
+
if ::Gitlab::Database::LoadBalancing::Session.current.use_primary?
load_balancer.primary_write_location
else
diff --git a/lib/gitlab/database/load_balancing/sticking.rb b/lib/gitlab/database/load_balancing/sticking.rb
index aa5808e61b01276a8b8b3dc16d51dc70be112eea..d34174c9ea807f1ce0fd01e53fa8dcf5315b30e8 100644
--- a/lib/gitlab/database/load_balancing/sticking.rb
+++ b/lib/gitlab/database/load_balancing/sticking.rb
@@ -104,6 +104,10 @@ def bulk_stick(namespace, ids)
end
def with_primary_write_location
+ # When only using the primary, there's no point in getting write
+ # locations, as the primary is always in sync with itself.
+ return if @load_balancer.primary_only?
+
location = @load_balancer.primary_write_location
return if location.blank?
diff --git a/spec/lib/gitlab/database/load_balancing/primary_host_spec.rb b/spec/lib/gitlab/database/load_balancing/primary_host_spec.rb
index a0e63a7ee4e74cf4c4144a185dd498023178ae44..45d8180897197be35ec1f2624577963cceb78825 100644
--- a/spec/lib/gitlab/database/load_balancing/primary_host_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/primary_host_spec.rb
@@ -63,9 +63,8 @@
end
describe '#primary_write_location' do
- it 'returns the write location of the primary' do
- expect(host.primary_write_location).to be_an_instance_of(String)
- expect(host.primary_write_location).not_to be_empty
+ it 'raises NotImplementedError' do
+ expect { host.primary_write_location }.to raise_error(NotImplementedError)
end
end
@@ -76,51 +75,8 @@
end
describe '#database_replica_location' do
- let(:connection) { double(:connection) }
-
- it 'returns the write ahead location of the replica', :aggregate_failures do
- expect(host)
- .to receive(:query_and_release)
- .and_return({ 'location' => '0/D525E3A8' })
-
- expect(host.database_replica_location).to be_an_instance_of(String)
- end
-
- it 'returns nil when the database query returned no rows' do
- expect(host).to receive(:query_and_release).and_return({})
-
- expect(host.database_replica_location).to be_nil
- end
-
- it 'returns nil when the database connection fails' do
- allow(host).to receive(:connection).and_raise(PG::Error)
-
- expect(host.database_replica_location).to be_nil
- end
- end
-
- describe '#query_and_release' do
- it 'executes a SQL query' do
- results = host.query_and_release('SELECT 10 AS number')
-
- expect(results).to be_an_instance_of(Hash)
- expect(results['number'].to_i).to eq(10)
- end
-
- it 'releases the connection after running the query' do
- expect(host)
- .to receive(:release_connection)
- .once
-
- host.query_and_release('SELECT 10 AS number')
- end
-
- it 'returns an empty Hash in the event of an error' do
- expect(host.connection)
- .to receive(:select_all)
- .and_raise(RuntimeError, 'kittens')
-
- expect(host.query_and_release('SELECT 10 AS number')).to eq({})
+ it 'raises NotImplementedError' do
+ expect { host.database_replica_location }.to raise_error(NotImplementedError)
end
end
end
diff --git a/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb
index 1d00cba0ecadfda2e38cb9210cc0207a9f06826b..08dd6a0a78840ae36588c6367e25ee0a0590c06b 100644
--- a/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb
@@ -21,7 +21,7 @@ def run_middleware
middleware.call(worker_class, job, nil, nil) {}
end
- describe '#call' do
+ describe '#call', :database_replica do
shared_context 'data consistency worker class' do |data_consistency, feature_flag|
let(:expected_consistency) { data_consistency }
let(:worker_class) do
diff --git a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb
index 4b0eb8858ddc59f091ab9f2c520c569a6eddbccb..8ceda52ee8501ed3ffa3e575db0cb4514734163c 100644
--- a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb
@@ -188,6 +188,10 @@
end
it 'sticks an entity to the primary', :aggregate_failures do
+ allow(ActiveRecord::Base.connection.load_balancer)
+ .to receive(:primary_only?)
+ .and_return(false)
+
ids.each do |id|
expect(sticking)
.to receive(:set_write_location_for)
@@ -199,6 +203,12 @@
subject
end
+
+ it 'does not update the write location when no replicas are used' do
+ expect(sticking).not_to receive(:set_write_location_for)
+
+ subject
+ end
end
describe '#stick' do
@@ -221,12 +231,22 @@
.to receive(:primary_write_location)
.and_return('foo')
+ allow(ActiveRecord::Base.connection.load_balancer)
+ .to receive(:primary_only?)
+ .and_return(false)
+
expect(sticking)
.to receive(:set_write_location_for)
.with(:user, 42, 'foo')
sticking.mark_primary_write_location(:user, 42)
end
+
+ it 'does nothing when no replicas are used' do
+ expect(sticking).not_to receive(:set_write_location_for)
+
+ sticking.mark_primary_write_location(:user, 42)
+ end
end
describe '#unstick' do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 0c9df33b8abf1a1e1a56aaec34b001325a073691..98b55ccb76b0faf99901428c8560f92ad5ea5fc0 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -2791,17 +2791,7 @@ def create_pipeline(status, ref, sha)
extra_update_queries = 4 # transition ... => :canceled, queue pop
extra_generic_commit_status_validation_queries = 2 # name_uniqueness_across_types
- # 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)
end
end