Merge branch 'yorick/reduce-lb-overhead' into 'master'

Don't retry errors when there are no replicas

See merge request gitlab-org/gitlab!71489

lib/gitlab/database/load_balancing/load_balancer.rb

@@ -30,6 +30,10 @@ module Gitlab
             end
         end
 
+        def primary_only?
+          @primary_only
+        end
+
         def disconnect!(timeout: 120)
           host_list.hosts.each { |host| host.disconnect!(timeout: timeout) }
         end
@@ -151,6 +155,17 @@ module Gitlab
 
         # Yields a block, retrying it upon error using an exponential backoff.
         def retry_with_backoff(retries = 3, time = 2)
+          # In CI we only use the primary, but databases may not always be
+          # available (or take a few seconds to become available). Retrying in
+          # this case can slow down CI jobs. In addition, retrying with _only_
+          # a primary being present isn't all that helpful.
+          #
+          # To prevent this from happening, we don't make any attempt at
+          # retrying unless one or more replicas are used. This matches the
+          # behaviour from before we enabled load balancing code even if no
+          # replicas were configured.
+          return yield if primary_only?
+
           retried = 0
           last_error = nil
 
@@ -176,6 +191,11 @@ module Gitlab
 
         def connection_error?(error)
           case error
+          when ActiveRecord::NoDatabaseError
+            # Retrying this error isn't going to magically make the database
+            # appear. It also slows down CI jobs that are meant to create the
+            # database in the first place.
+            false
           when ActiveRecord::StatementInvalid, ActionView::Template::Error
             # After connecting to the DB Rails will wrap query errors using this
             # class.

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

@@ -274,6 +274,14 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
 
       expect { lb.retry_with_backoff { raise } }.to raise_error(RuntimeError)
     end
+
+    it 'skips retries when only the primary is used' do
+      allow(lb).to receive(:primary_only?).and_return(true)
+
+      expect(lb).not_to receive(:sleep)
+
+      expect { lb.retry_with_backoff { raise } }.to raise_error(RuntimeError)
+    end
   end
 
   describe '#connection_error?' do
@@ -283,6 +291,12 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
       expect(lb.connection_error?(error)).to eq(true)
     end
 
+    it 'returns false for a missing database error' do
+      error = ActiveRecord::NoDatabaseError.new
+
+      expect(lb.connection_error?(error)).to eq(false)
+    end
+
     it 'returns true for a wrapped connection error' do
       wrapped = wrapped_exception(ActiveRecord::StatementInvalid, ActiveRecord::ConnectionNotEstablished)