Revert "Use multistore pool for RepositoryCache.cache_store"

This reverts commit d0c26297.

lib/gitlab/redis/multi_store_connection_pool.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Redis
+    class MultiStoreConnectionPool < ::ConnectionPool
+      # We intercept the returned connection and borrow the connections
+      # before yielding the block.
+      def with
+        super do |conn|
+          next yield conn unless conn.is_a?(Gitlab::Redis::MultiStore)
+
+          conn.with_borrowed_connection do
+            yield conn
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/redis/multi_store_wrapper.rb

@@ -4,16 +4,15 @@ module Gitlab
   module Redis
     class MultiStoreWrapper < Wrapper
       class << self
+        # overrides Wrapper's pool to use the multistore connection pool
         def with
           multistore_pool.with do |multistore|
-            multistore.with_borrowed_connection do
-              yield multistore
-            end
+            yield multistore
           end
         end
 
         def multistore_pool
-          @multistore_pool ||= ConnectionPool.new(size: pool_size, name: pool_name) { multistore }
+          @multistore_pool ||= MultiStoreConnectionPool.new(size: pool_size, name: pool_name) { multistore }
         end
 
         def pool_name

lib/gitlab/redis/repository_cache.rb

@@ -14,7 +14,7 @@ def config_fallback
 
         def cache_store
           @cache_store ||= RepositoryCacheStore.new(
-            redis: pool,
+            redis: multistore_pool,
             pool: false,
             compress: Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_REDIS_CACHE_COMPRESSION', '1')),
             namespace: Cache::CACHE_NAMESPACE,

spec/lib/gitlab/redis/multi_store_connection_pool_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Redis::MultiStoreConnectionPool, feature_category: :scalability do
+  describe '#with' do
+    let(:conn_a) { Redis.new(url: 'redis://localhost:6379') }
+    let(:conn_b) { Redis.new(url: 'redis://localhost:6380') }
+    let(:pool_a) { ConnectionPool.new(size: 2) { conn_a } }
+    let(:pool_b) { ConnectionPool.new(size: 2) { conn_b } }
+    let(:multistore) { Gitlab::Redis::MultiStore.new(pool_a, pool_b, 'test') }
+    let(:multistore_pool) { described_class.new(size: 2) { multistore } }
+
+    before do
+      skip_default_enabled_yaml_check
+    end
+
+    it 'extends ConnectionPool' do
+      expect(multistore_pool.is_a?(::ConnectionPool)).to eq(true)
+    end
+
+    shared_examples 'handles connection borrowing' do
+      it 'yields a multistore with already borrowed connections' do
+        multistore_pool.with do |ms|
+          expect(ms).to be_an_instance_of(Gitlab::Redis::MultiStore)
+          expect(ms.primary_store).to eq(conn_a)
+          expect(ms.secondary_store).to eq(conn_b)
+        end
+      end
+    end
+
+    context 'with both feature flags enabled' do
+      before do
+        stub_feature_flags(use_primary_store_as_default_for_test: true,
+          use_primary_and_secondary_stores_for_test: true)
+      end
+
+      it_behaves_like 'handles connection borrowing'
+    end
+
+    context 'with use_primary_and_secondary_stores_for_test disabled' do
+      before do
+        stub_feature_flags(use_primary_store_as_default_for_test: true,
+          use_primary_and_secondary_stores_for_test: false)
+      end
+
+      it_behaves_like 'handles connection borrowing'
+    end
+
+    context 'with use_primary_store_as_default_for_test disabled' do
+      before do
+        stub_feature_flags(use_primary_store_as_default_for_test: false,
+          use_primary_and_secondary_stores_for_test: true)
+      end
+
+      it_behaves_like 'handles connection borrowing'
+    end
+
+    context 'with feature flags disabled' do
+      before do
+        stub_feature_flags(use_primary_store_as_default_for_test: false,
+          use_primary_and_secondary_stores_for_test: false)
+      end
+
+      it_behaves_like 'handles connection borrowing'
+    end
+
+    context 'when non-MultiStore is provided' do
+      let(:multistore) { conn_a }
+
+      it 'passes connection through without errors' do
+        multistore_pool.with do |c|
+          expect(c).to eq(conn_a)
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/redis/repository_cache_spec.rb

@@ -10,6 +10,10 @@
     it 'has a default ttl of 8 hours' do
       expect(described_class.cache_store.options[:expires_in]).to eq(8.hours)
     end
+
+    it 'uses a pool of multistore connections' do
+      expect(described_class.cache_store.redis.checkout).to be_an_instance_of(Gitlab::Redis::MultiStore)
+    end
   end
 
   it 'migrates from self to ClusterRepositoryCache' do