Merge branch 'qmnguyen0711/rename-use-replica-if-possible' into 'master'

Rename use_replica_if_possible to fallback_to_replicas_for_ambiguous_queries

See merge request gitlab-org/gitlab!59086

ee/app/models/ee/application_record_helpers.rb

@@ -10,7 +10,7 @@ module EE
 
       override :with_fast_read_statement_timeout
       def with_fast_read_statement_timeout(timeout_ms = 5000)
-        ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+        ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
           super
         end
       end

ee/lib/gitlab/database/load_balancing/connection_proxy.rb

@@ -60,7 +60,7 @@ module Gitlab
         end
 
         def transaction(*args, &block)
-          if ::Gitlab::Database::LoadBalancing::Session.current.use_replica?
+          if ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries?
             track_read_only_transaction!
             read_using_load_balancer(:transaction, args, &block)
           else
@@ -73,7 +73,7 @@ module Gitlab
 
         # Delegates all unknown messages to a read-write connection.
         def method_missing(name, *args, &block)
-          if ::Gitlab::Database::LoadBalancing::Session.current.use_replica?
+          if ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries?
             read_using_load_balancer(name, args, &block)
           else
             write_using_load_balancer(name, args, &block)

ee/lib/gitlab/database/load_balancing/session.rb

@@ -55,22 +55,27 @@ module Gitlab
           @ignore_writes = false
         end
 
-        # Indicate that all the SQL statements from anywhere inside this block
-        # should use a replica. This is a weak enforcement. The queries
-        # prioritize using a primary over a replica in those cases:
+        # Indicate that the ambiguous SQL statements from anywhere inside this
+        # block should use a replica. The ambiguous statements include:
+        # - Transactions.
+        # - Custom queries (via exec_query, execute, etc.)
+        # - In-flight connection configuration change (SET LOCAL statement_timeout = 5000)
+        #
+        # This is a weak enforcement. This helper incorporates well with
+        # primary stickiness:
         # - If the queries are about to write
         # - The current session already performed writes
         # - It prefers to use primary, aka, use_primary or use_primary! were called
-        def use_replica_if_possible(&blk)
-          used_replica = @use_replica
-          @use_replica = true
+        def fallback_to_replicas_for_ambiguous_queries(&blk)
+          previous_flag = @fallback_to_replicas_for_ambiguous_queries
+          @fallback_to_replicas_for_ambiguous_queries = true
           yield
         ensure
-          @use_replica = used_replica
+          @fallback_to_replicas_for_ambiguous_queries = previous_flag
         end
 
-        def use_replica?
-          @use_replica == true && !use_primary? && !performed_write?
+        def fallback_to_replicas_for_ambiguous_queries?
+          @fallback_to_replicas_for_ambiguous_queries == true && !use_primary? && !performed_write?
         end
 
         def write!

ee/spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb

@@ -115,11 +115,11 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do
         .and_return(session)
     end
 
-    context 'session prefers to use a replica' do
+    context 'session fallbacks ambiguous queries to replicas' do
       let(:replica) { double(:connection) }
 
       before do
-        allow(session).to receive(:use_replica?).and_return(true)
+        allow(session).to receive(:fallback_to_replicas_for_ambiguous_queries?).and_return(true)
         allow(session).to receive(:use_primary?).and_return(false)
         allow(replica).to receive(:transaction).and_yield
         allow(replica).to receive(:select)
@@ -148,11 +148,11 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do
       end
     end
 
-    context 'session does not prefer to use a replica' do
+    context 'session does not fallback to replicas for ambiguous queries' do
       let(:primary) { double(:connection) }
 
       before do
-        allow(session).to receive(:use_replica?).and_return(false)
+        allow(session).to receive(:fallback_to_replicas_for_ambiguous_queries?).and_return(false)
         allow(session).to receive(:use_primary?).and_return(true)
         allow(primary).to receive(:transaction).and_yield
         allow(primary).to receive(:select)
@@ -200,13 +200,13 @@ RSpec.describe Gitlab::Database::LoadBalancing::ConnectionProxy do
         .not_to raise_error
     end
 
-    context 'current session prefers to use a replica' do
+    context 'current session prefers to fallback ambiguous queries to replicas' do
       let(:session) { double(:session) }
 
       before do
         allow(Gitlab::Database::LoadBalancing::Session).to receive(:current)
           .and_return(session)
-        allow(session).to receive(:use_replica?).and_return(true)
+        allow(session).to receive(:fallback_to_replicas_for_ambiguous_queries?).and_return(true)
         allow(session).to receive(:use_primary?).and_return(false)
       end
 

ee/spec/lib/gitlab/database/load_balancing/session_spec.rb

@@ -139,37 +139,37 @@ RSpec.describe Gitlab::Database::LoadBalancing::Session do
     end
   end
 
-  describe '#use_replica_if_possible' do
+  describe '#fallback_to_replicas_for_ambiguous_queries' do
     let(:instance) { described_class.new }
 
-    it 'uses replica during block' do
+    it 'sets the flag inside the block' do
       expect do |blk|
-        instance.use_replica_if_possible do
-          expect(instance.use_replica?).to eq(true)
+        instance.fallback_to_replicas_for_ambiguous_queries do
+          expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(true)
 
           # call yield probe
           blk.to_proc.call
         end
       end.to yield_control
 
-      expect(instance.use_replica?).to eq(false)
+      expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
     end
 
     it 'restores state after use' do
       expect do |blk|
-        instance.use_replica_if_possible do
-          instance.use_replica_if_possible do
-            expect(instance.use_replica?).to eq(true)
+        instance.fallback_to_replicas_for_ambiguous_queries do
+          instance.fallback_to_replicas_for_ambiguous_queries do
+            expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(true)
 
             # call yield probe
             blk.to_proc.call
           end
 
-          expect(instance.use_replica?).to eq(true)
+          expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(true)
         end
       end.to yield_control
 
-      expect(instance.use_replica?).to eq(false)
+      expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
     end
 
     context 'when primary was used before' do
@@ -178,18 +178,18 @@ RSpec.describe Gitlab::Database::LoadBalancing::Session do
       end
 
       it 'uses primary during block' do
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
 
         expect do |blk|
-          instance.use_replica_if_possible do
-            expect(instance.use_replica?).to eq(false)
+          instance.fallback_to_replicas_for_ambiguous_queries do
+            expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
 
             # call yield probe
             blk.to_proc.call
           end
         end.to yield_control
 
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
       end
     end
 
@@ -199,82 +199,82 @@ RSpec.describe Gitlab::Database::LoadBalancing::Session do
       end
 
       it 'uses primary during block' do
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
 
         expect do |blk|
-          instance.use_replica_if_possible do
-            expect(instance.use_replica?).to eq(false)
+          instance.fallback_to_replicas_for_ambiguous_queries do
+            expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
 
             # call yield probe
             blk.to_proc.call
           end
         end.to yield_control
 
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
       end
     end
 
     context 'when primary was used inside the block' do
       it 'uses primary aterward' do
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
 
-        instance.use_replica_if_possible do
-          expect(instance.use_replica?).to eq(true)
+        instance.fallback_to_replicas_for_ambiguous_queries do
+          expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(true)
 
           instance.use_primary!
 
-          expect(instance.use_replica?).to eq(false)
+          expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
         end
 
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
       end
 
       it 'restores state after use' do
-        instance.use_replica_if_possible do
-          instance.use_replica_if_possible do
-            expect(instance.use_replica?).to eq(true)
+        instance.fallback_to_replicas_for_ambiguous_queries do
+          instance.fallback_to_replicas_for_ambiguous_queries do
+            expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(true)
 
             instance.use_primary!
 
-            expect(instance.use_replica?).to eq(false)
+            expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
           end
 
-          expect(instance.use_replica?).to eq(false)
+          expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
         end
 
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
       end
     end
 
     context 'when a write was performed inside the block' do
       it 'uses primary aterward' do
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
 
-        instance.use_replica_if_possible do
-          expect(instance.use_replica?).to eq(true)
+        instance.fallback_to_replicas_for_ambiguous_queries do
+          expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(true)
 
           instance.write!
 
-          expect(instance.use_replica?).to eq(false)
+          expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
         end
 
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
       end
 
       it 'restores state after use' do
-        instance.use_replica_if_possible do
-          instance.use_replica_if_possible do
-            expect(instance.use_replica?).to eq(true)
+        instance.fallback_to_replicas_for_ambiguous_queries do
+          instance.fallback_to_replicas_for_ambiguous_queries do
+            expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(true)
 
             instance.write!
 
-            expect(instance.use_replica?).to eq(false)
+            expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
           end
 
-          expect(instance.use_replica?).to eq(false)
+          expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
         end
 
-        expect(instance.use_replica?).to eq(false)
+        expect(instance.fallback_to_replicas_for_ambiguous_queries?).to eq(false)
       end
     end
   end

ee/spec/lib/gitlab/database/load_balancing_spec.rb

@@ -561,10 +561,10 @@ RSpec.describe Gitlab::Database::LoadBalancing do
           false, [:replica, :primary]
         ],
 
-        # use_replica_if_possible
+        # fallback_to_replicas_for_ambiguous_queries
         [
           -> {
-            ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+            ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
               model.first
               model.where(name: 'test1').to_a
             end
@@ -572,10 +572,10 @@ RSpec.describe Gitlab::Database::LoadBalancing do
           false, [:replica, :replica]
         ],
 
-        # use_replica_if_possible for read-only transaction
+        # fallback_to_replicas_for_ambiguous_queries for read-only transaction
         [
           -> {
-            ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+            ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
               model.transaction do
                 model.first
                 model.where(name: 'test1').to_a
@@ -585,20 +585,20 @@ RSpec.describe Gitlab::Database::LoadBalancing do
           false, [:replica, :replica]
         ],
 
-        # A custom read query inside use_replica_if_possible
+        # A custom read query inside fallback_to_replicas_for_ambiguous_queries
         [
           -> {
-            ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+            ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
               model.connection.exec_query("SELECT 1")
             end
           },
           false, [:replica]
         ],
 
-        # A custom read query inside a transaction use_replica_if_possible
+        # A custom read query inside a transaction fallback_to_replicas_for_ambiguous_queries
         [
           -> {
-            ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+            ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
               model.transaction do
                 model.connection.exec_query("SET LOCAL statement_timeout = 5000")
                 model.count
@@ -608,33 +608,33 @@ RSpec.describe Gitlab::Database::LoadBalancing do
           true, [:replica, :replica, :replica, :replica]
         ],
 
-        # use_replica_if_possible after a write
+        # fallback_to_replicas_for_ambiguous_queries after a write
         [
           -> {
             model.create!(name: 'Test1')
-            ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+            ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
               model.first
             end
           },
           false, [:primary, :primary]
         ],
 
-        # use_replica_if_possible after use_primary!
+        # fallback_to_replicas_for_ambiguous_queries after use_primary!
         [
           -> {
             ::Gitlab::Database::LoadBalancing::Session.current.use_primary!
-            ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+            ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
               model.first
             end
           },
           false, [:primary]
         ],
 
-        # use_replica_if_possible inside use_primary
+        # fallback_to_replicas_for_ambiguous_queries inside use_primary
         [
           -> {
             ::Gitlab::Database::LoadBalancing::Session.current.use_primary do
-              ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+              ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
                 model.first
               end
             end
@@ -642,10 +642,10 @@ RSpec.describe Gitlab::Database::LoadBalancing do
           false, [:primary]
         ],
 
-        # use_primary inside use_replica_if_possible
+        # use_primary inside fallback_to_replicas_for_ambiguous_queries
         [
           -> {
-            ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+            ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
               ::Gitlab::Database::LoadBalancing::Session.current.use_primary do
                 model.first
               end
@@ -654,10 +654,10 @@ RSpec.describe Gitlab::Database::LoadBalancing do
           false, [:primary]
         ],
 
-        # A write query inside use_replica_if_possible
+        # A write query inside fallback_to_replicas_for_ambiguous_queries
         [
           -> {
-            ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+            ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
               model.first
               model.delete_all
               model.where(name: 'test1').to_a
@@ -758,12 +758,12 @@ RSpec.describe Gitlab::Database::LoadBalancing do
       end
     end
 
-    context 'a write inside a transaction inside use_replica_if_possible block' do
+    context 'a write inside a transaction inside fallback_to_replicas_for_ambiguous_queries block' do
       include_context 'LoadBalancing setup'
 
       it 'raises an exception' do
         expect do
-          ::Gitlab::Database::LoadBalancing::Session.current.use_replica_if_possible do
+          ::Gitlab::Database::LoadBalancing::Session.current.fallback_to_replicas_for_ambiguous_queries do
             model.transaction do
               model.first
               model.create!(name: 'hello')

ee/spec/models/ee/application_record_helpers_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe ApplicationRecord do
 
     before do
       allow(::Gitlab::Database::LoadBalancing::Session).to receive(:current).and_return(session)
-      allow(session).to receive(:use_replica_if_possible).and_yield
+      allow(session).to receive(:fallback_to_replicas_for_ambiguous_queries).and_yield
     end
 
     it 'yields control' do