From af84e4be30c4cda47ba7d68592959a7b59e48409 Mon Sep 17 00:00:00 2001
From: Nikola Milojevic <nmilojevic@gitlab.com>
Date: Thu, 6 May 2021 15:37:48 +0000
Subject: [PATCH] Don't update db WAL location if already provided
---
.../sidekiq_client_middleware.rb | 6 +++
.../sidekiq_client_middleware_spec.rb | 40 +++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/ee/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb b/ee/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb
index 77426f46cebdd..a346fd4b63215 100644
--- a/ee/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb
+++ b/ee/lib/gitlab/database/load_balancing/sidekiq_client_middleware.rb
@@ -20,6 +20,8 @@ def mark_data_consistency_location(worker_class, job)
return unless worker_class.include?(::ApplicationWorker)
return unless worker_class.get_data_consistency_feature_flag_enabled?
+ return if location_already_provided?(job)
+
job['worker_data_consistency'] = worker_class.get_data_consistency
return if worker_class.get_data_consistency == :always
@@ -31,6 +33,10 @@ def mark_data_consistency_location(worker_class, job)
end
end
+ def location_already_provided?(job)
+ job['database_replica_location'] || job['database_write_location']
+ end
+
def load_balancer
LoadBalancing.proxy.load_balancer
end
diff --git a/ee/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb b/ee/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb
index 7ce86e49fc105..7b71da587fb20 100644
--- a/ee/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb
+++ b/ee/spec/lib/gitlab/database/load_balancing/sidekiq_client_middleware_spec.rb
@@ -82,6 +82,38 @@ def perform(*args)
end
end
+ shared_examples_for 'database location was already provided' do |provided_database_location, other_location|
+ shared_examples_for 'does not set database location again' do |write_performed|
+ before do
+ allow(Gitlab::Database::LoadBalancing::Session.current).to receive(:performed_write?).and_return(write_performed)
+ end
+
+ it 'does not set database locations again' do
+ middleware.call(worker_class, job, double(:queue), redis_pool) { 10 }
+
+ expect(job[provided_database_location]).to eq(old_location)
+ expect(job[other_location]).to be_nil
+ end
+ end
+
+ let(:old_location) { '0/D525E3A8' }
+ let(:new_location) { 'AB/12345' }
+ let(:job) { { "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", provided_database_location => old_location } }
+
+ before do
+ allow(middleware).to receive_message_chain(:load_balancer, :primary_write_location).and_return(new_location)
+ allow(middleware).to receive_message_chain(:load_balancer, :database_replica_location).and_return(new_location)
+ end
+
+ context "when write was performed" do
+ include_examples 'does not set database location again', true
+ end
+
+ context "when write was not performed" do
+ include_examples 'does not set database location again', false
+ end
+ end
+
let(:queue) { 'default' }
let(:redis_pool) { Sidekiq.redis_pool }
let(:worker_class) { 'TestDataConsistencyWorker' }
@@ -104,6 +136,14 @@ def perform(*args)
include_examples 'does not pass database locations'
end
+ context 'database write location was already provided' do
+ include_examples 'database location was already provided', 'database_write_location', 'database_replica_location'
+ end
+
+ context 'database replica location was already provided' do
+ include_examples 'database location was already provided', 'database_replica_location', 'database_write_location'
+ end
+
context 'when worker data consistency is :always' do
include_context 'data consistency worker class', :always, :load_balancing_for_test_data_consistency_worker
--
GitLab