Merge branch 'check-wal-before-sleep' into 'master'

Prevent Sidekiq sleep when DB replica is up-to-date

See merge request gitlab-org/gitlab!76669

lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb

@@ -6,7 +6,7 @@ module Gitlab
       class SidekiqServerMiddleware
         JobReplicaNotUpToDate = Class.new(StandardError)
 
-        MINIMUM_DELAY_INTERVAL = 1
+        MINIMUM_DELAY_INTERVAL_SECONDS = 0.8
 
         def call(worker, job, _queue)
           worker_class = worker.class
@@ -46,11 +46,13 @@ module Gitlab
 
           return :primary_no_wal if wal_locations.blank?
 
+          # Happy case: we can read from a replica.
+          return replica_strategy(worker_class, job) if databases_in_sync?(wal_locations)
+
           sleep_if_needed(job)
 
           if databases_in_sync?(wal_locations)
-            # Happy case: we can read from a replica.
-            retried_before?(worker_class, job) ? :replica_retried : :replica
+            replica_strategy(worker_class, job)
           elsif can_retry?(worker_class, job)
             # Optimistic case: The worker allows retries and we have retries left.
             :retry
@@ -61,9 +63,9 @@ module Gitlab
         end
 
         def sleep_if_needed(job)
-          remaining_delay = MINIMUM_DELAY_INTERVAL - (Time.current.to_f - job['created_at'].to_f)
+          remaining_delay = MINIMUM_DELAY_INTERVAL_SECONDS - (Time.current.to_f - job['created_at'].to_f)
 
-          sleep remaining_delay if remaining_delay > 0 && remaining_delay < MINIMUM_DELAY_INTERVAL
+          sleep remaining_delay if remaining_delay > 0 && remaining_delay < MINIMUM_DELAY_INTERVAL_SECONDS
         end
 
         def get_wal_locations(job)
@@ -80,6 +82,10 @@ module Gitlab
           worker_class.get_data_consistency == :delayed && not_yet_retried?(job)
         end
 
+        def replica_strategy(worker_class, job)
+          retried_before?(worker_class, job) ? :replica_retried : :replica
+        end
+
         def retried_before?(worker_class, job)
           worker_class.get_data_consistency == :delayed && !not_yet_retried?(job)
         end

spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb

@@ -122,7 +122,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
 
     shared_examples_for 'sleeps when necessary' do
       context 'when WAL locations are blank', :freeze_time do
-        let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", "wal_locations" => {}, "created_at" => Time.current.to_f - (described_class::MINIMUM_DELAY_INTERVAL - 0.3) } }
+        let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", "wal_locations" => {}, "created_at" => Time.current.to_f - (described_class::MINIMUM_DELAY_INTERVAL_SECONDS - 0.3) } }
 
         it 'does not sleep' do
           expect(middleware).not_to receive(:sleep)
@@ -135,17 +135,39 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
         let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'wal_locations' => wal_locations, "created_at" => Time.current.to_f - elapsed_time } }
 
         context 'when delay interval has not elapsed' do
-          let(:elapsed_time) { described_class::MINIMUM_DELAY_INTERVAL - 0.3 }
+          let(:elapsed_time) { described_class::MINIMUM_DELAY_INTERVAL_SECONDS - 0.3 }
+
+          context 'when replica is up to date' do
+            before do
+              Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
+                allow(lb).to receive(:select_up_to_date_host).and_return(true)
+              end
+            end
+
+            it 'does not sleep' do
+              expect(middleware).not_to receive(:sleep)
+
+              run_middleware
+            end
+          end
+
+          context 'when replica is not up to date' do
+            before do
+              Gitlab::Database::LoadBalancing.each_load_balancer do |lb|
+                allow(lb).to receive(:select_up_to_date_host).and_return(false, true)
+              end
+            end
 
             it 'sleeps until the minimum delay is reached' do
-            expect(middleware).to receive(:sleep).with(be_within(0.01).of(described_class::MINIMUM_DELAY_INTERVAL - elapsed_time))
+              expect(middleware).to receive(:sleep).with(be_within(0.01).of(described_class::MINIMUM_DELAY_INTERVAL_SECONDS - elapsed_time))
 
               run_middleware
             end
           end
+        end
 
         context 'when delay interval has elapsed' do
-          let(:elapsed_time) { described_class::MINIMUM_DELAY_INTERVAL + 0.3 }
+          let(:elapsed_time) { described_class::MINIMUM_DELAY_INTERVAL_SECONDS + 0.3 }
 
           it 'does not sleep' do
             expect(middleware).not_to receive(:sleep)
@@ -179,7 +201,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware, :clean_
 
       context 'when delay interval has not elapsed', :freeze_time do
         let(:job) { { "retry" => 3, "job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e", 'wal_locations' => wal_locations, "created_at" => Time.current.to_f - elapsed_time } }
-        let(:elapsed_time) { described_class::MINIMUM_DELAY_INTERVAL - 0.3 }
+        let(:elapsed_time) { described_class::MINIMUM_DELAY_INTERVAL_SECONDS - 0.3 }
 
         it 'does not sleep' do
           expect(middleware).not_to receive(:sleep)