Merge branch 'yorick/lb-fix-pool-size' into 'master'

Dynamically read pool sizes for LB configurations

See merge request gitlab-org/gitlab!70060

lib/gitlab/database/load_balancing/configuration.rb

@@ -7,7 +7,7 @@ module Gitlab
       class Configuration
         attr_accessor :hosts, :max_replication_difference,
                       :max_replication_lag_time, :replica_check_interval,
-                      :service_discovery, :pool_size, :model
+                      :service_discovery, :model
 
         # Creates a configuration object for the given ActiveRecord model.
         def self.for_model(model)
@@ -15,10 +15,6 @@ module Gitlab
           lb_cfg = cfg[:load_balancing] || {}
           config = new(model)
 
-          if (size = cfg[:pool])
-            config.pool_size = size
-          end
-
           if (diff = lb_cfg[:max_replication_difference])
             config.max_replication_difference = diff
           end
@@ -54,7 +50,6 @@ module Gitlab
           @replica_check_interval = 60.0
           @model = model
           @hosts = hosts
-          @pool_size = Database.default_pool_size
           @service_discovery = {
             nameserver: 'localhost',
             port: 8600,
@@ -66,6 +61,17 @@ module Gitlab
           }
         end
 
+        def pool_size
+          # The pool size may change when booting up GitLab, as GitLab enforces
+          # a certain number of threads. If a Configuration is memoized, this
+          # can lead to incorrect pool sizes.
+          #
+          # To support this scenario, we always attempt to read the pool size
+          # from the model's configuration.
+          @model.connection_db_config.configuration_hash[:pool] ||
+            Database.default_pool_size
+        end
+
         def load_balancing_enabled?
           hosts.any? || service_discovery_enabled?
         end

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

@@ -3,14 +3,14 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Database::LoadBalancing::Configuration do
-  describe '.for_model' do
-    let(:model) do
-      config = ActiveRecord::DatabaseConfigurations::HashConfig
-        .new('main', 'test', configuration_hash)
+  let(:model) do
+    config = ActiveRecord::DatabaseConfigurations::HashConfig
+      .new('main', 'test', configuration_hash)
 
-      double(:model, connection_db_config: config)
-    end
+    double(:model, connection_db_config: config)
+  end
 
+  describe '.for_model' do
     context 'when load balancing is not configured' do
       let(:configuration_hash) { {} }
 
@@ -142,4 +142,34 @@ RSpec.describe Gitlab::Database::LoadBalancing::Configuration do
       expect(config.service_discovery_enabled?).to eq(false)
     end
   end
+
+  describe '#pool_size' do
+    context 'when a custom pool size is used' do
+      let(:configuration_hash) { { pool: 4 } }
+
+      it 'always reads the value from the model configuration' do
+        config = described_class.new(model)
+
+        expect(config.pool_size).to eq(4)
+
+        # We can't modify `configuration_hash` as it's only used to populate the
+        # internal hash used by ActiveRecord; instead of it being used as-is.
+        allow(model.connection_db_config)
+          .to receive(:configuration_hash)
+          .and_return({ pool: 42 })
+
+        expect(config.pool_size).to eq(42)
+      end
+    end
+
+    context 'when the pool size is nil' do
+      let(:configuration_hash) { {} }
+
+      it 'returns the default pool size' do
+        config = described_class.new(model)
+
+        expect(config.pool_size).to eq(Gitlab::Database.default_pool_size)
+      end
+    end
+  end
 end