Merge branch '334989-remove-ff-load_balancing_improved_caught_up_hosts_check' into 'master'

Remove replica selection change FF

See merge request gitlab-org/gitlab!65545

config/feature_flags/development/load_balancing_improved_caught_up_hosts_check.yml

----
-name: load_balancing_improved_caught_up_hosts_check
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65248
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334989
-milestone: '14.1'
-type: development
-group: group::memory
-default_enabled: false

lib/gitlab/database/load_balancing/load_balancer.rb

@@ -147,16 +147,6 @@ module Gitlab
           raise 'Failed to determine the write location of the primary database'
         end
 
-        # FF disabled: Returns true if all hosts have caught up to the given transaction write location.
-        # FF enabled: Returns true if there was at least one host that has caught up with the given transaction and sets it.
-        def all_caught_up?(location)
-          if ::Feature.enabled?(:load_balancing_improved_caught_up_hosts_check)
-            select_up_to_date_host(location)
-          else
-            @host_list.hosts.all? { |host| host.caught_up?(location) }
-          end
-        end
-
         # Returns true if there was at least one host that has caught up with the given transaction.
         #
         # In case of a retry, this method also stores the set of hosts that have caught up.

lib/gitlab/database/load_balancing/sticking.rb

@@ -33,10 +33,10 @@ module Gitlab
 
           return true unless location
 
-          load_balancer.all_caught_up?(location).tap do |caught_up|
-            ActiveSupport::Notifications.instrument('caught_up_replica_pick.load_balancing', { result: caught_up } )
+          load_balancer.select_up_to_date_host(location).tap do |found|
+            ActiveSupport::Notifications.instrument('caught_up_replica_pick.load_balancing', { result: found } )
 
-            unstick(namespace, id) if caught_up
+            unstick(namespace, id) if found
           end
         end
 

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

@@ -306,38 +306,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
     end
   end
 
-  describe '#all_caught_up?' do
-    it 'delegates execution to #select_up_to_date_host' do
-      expect(lb).to receive(:select_up_to_date_host).with('foo').and_return(true)
-
-      expect(lb.all_caught_up?('foo')).to eq(true)
-    end
-
-    context 'when :load_balancing_improved_caught_up_hosts_check FF is disabled' do
-      before do
-        stub_feature_flags(load_balancing_improved_caught_up_hosts_check: false)
-      end
-
-      it 'returns true if all hosts caught up to the write location' do
-        expect(lb.host_list.hosts).to all(receive(:caught_up?).with('foo').and_return(true))
-
-        expect(lb.all_caught_up?('foo')).to eq(true)
-      end
-
-      it 'returns false if a host has not yet caught up' do
-        expect(lb.host_list.hosts[0]).to receive(:caught_up?)
-          .with('foo')
-          .and_return(true)
-
-        expect(lb.host_list.hosts[1]).to receive(:caught_up?)
-          .with('foo')
-          .and_return(false)
-
-        expect(lb.all_caught_up?('foo')).to eq(false)
-      end
-    end
-  end
-
   describe '#retry_with_backoff' do
     it 'returns the value returned by the block' do
       value = lb.retry_with_backoff { 10 }

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

@@ -64,7 +64,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
           .with(:user, 42)
           .and_return(nil)
 
-        expect(lb).not_to receive(:all_caught_up?)
+        expect(lb).not_to receive(:select_up_to_date_host)
 
         expect(described_class.all_caught_up?(:user, 42)).to eq(true)
       end
@@ -72,7 +72,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
 
     context 'when all secondaries have caught up' do
       before do
-        allow(lb).to receive(:all_caught_up?).with('foo').and_return(true)
+        allow(lb).to receive(:select_up_to_date_host).with('foo').and_return(true)
       end
 
       it 'returns true, and unsticks' do
@@ -93,7 +93,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
 
     context 'when the secondaries have not yet caught up' do
       before do
-        allow(lb).to receive(:all_caught_up?).with('foo').and_return(false)
+        allow(lb).to receive(:select_up_to_date_host).with('foo').and_return(false)
       end
 
       it 'returns false' do
@@ -123,7 +123,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
         .with(:user, 42)
         .and_return(nil)
 
-      expect(lb).not_to receive(:all_caught_up?)
+      expect(lb).not_to receive(:select_up_to_date_host)
 
       described_class.unstick_or_continue_sticking(:user, 42)
     end
@@ -133,7 +133,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
         .with(:user, 42)
         .and_return('foo')
 
-      allow(lb).to receive(:all_caught_up?).with('foo').and_return(true)
+      allow(lb).to receive(:select_up_to_date_host).with('foo').and_return(true)
 
       expect(described_class).to receive(:unstick).with(:user, 42)
 
@@ -145,7 +145,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do
         .with(:user, 42)
         .and_return('foo')
 
-      allow(lb).to receive(:all_caught_up?).with('foo').and_return(false)
+      allow(lb).to receive(:select_up_to_date_host).with('foo').and_return(false)
 
       expect(Gitlab::Database::LoadBalancing::Session.current)
         .to receive(:use_primary!)