Use multistore pool for RepositoryCache.cache_store

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,

lib/gitlab/repository_cache.rb

@@ -18,23 +18,33 @@ def cache_key(type)
     end
 
     def expire(key)
-      backend.delete(cache_key(key))
+      borrow_multistore_connection do
+        backend.delete(cache_key(key))
+      end
     end
 
     def fetch(key, &block)
-      backend.fetch(cache_key(key), &block)
+      borrow_multistore_connection do
+        backend.fetch(cache_key(key), &block)
+      end
     end
 
     def exist?(key)
-      backend.exist?(cache_key(key))
+      borrow_multistore_connection do
+        backend.exist?(cache_key(key))
+      end
     end
 
     def read(key)
-      backend.read(cache_key(key))
+      borrow_multistore_connection do
+        backend.read(cache_key(key))
+      end
     end
 
     def write(key, value, *args)
-      backend.write(cache_key(key), value, *args)
+      borrow_multistore_connection do
+        backend.write(cache_key(key), value, *args)
+      end
     end
 
     def fetch_without_caching_false(key, &block)
@@ -49,6 +59,24 @@ def fetch_without_caching_false(key, &block)
       value
     end
 
+    def borrow_multistore_connection
+      if Feature.disabled?(:use_primary_store_as_default_for_repository_cache, type: :gitlab_com_derisk) &&
+          Feature.disabled?(:use_primary_and_secondary_stores_for_repository_cache, type: :gitlab_com_derisk)
+        return yield
+      end
+
+      # TODO: to be removed after repository cache migration alongside Gitlab::Redis::ClusterRepositoryCache
+      # See https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/2854
+      #
+      # Borrows connections for multistore before letting Rails RedisCacheStore perform redis operations
+      # RedisCacheStore performs another `.with` but does not check out an extra connection since it re-uses the
+      # previously checked out connection on the same thread.
+      # See https://github.com/mperham/connection_pool/blob/v2.4.1/lib/connection_pool.rb#L122
+      Gitlab::Redis::RepositoryCache.with do |_c|
+        yield
+      end
+    end
+
     def self.store
       Gitlab::Redis::RepositoryCache.cache_store
     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

spec/lib/gitlab/repository_cache_spec.rb

@@ -56,19 +56,85 @@
     end
   end
 
+  shared_examples 'borrows connections for multistore' do
+    before do
+      if Gitlab::Redis::RepositoryCache.instance_variable_defined?(:@multistore_pool)
+        Gitlab::Redis::RepositoryCache.remove_instance_variable(:@multistore_pool)
+      end
+    end
+
+    it 'borrows connections for the multistore' do
+      expect_next_instance_of(Gitlab::Redis::MultiStore) do |m|
+        expect(m).to receive(:with_borrowed_connection).and_call_original
+      end
+
+      operation
+    end
+  end
+
+  shared_examples 'compatiblity with multistore' do
+    it_behaves_like 'borrows connections for multistore'
+
+    context 'with use_primary_and_secondary_stores_for_repository_cache disabled' do
+      before do
+        stub_feature_flags(
+          use_primary_store_as_default_for_repository_cache: true,
+          use_primary_and_secondary_stores_for_repository_cache: false
+        )
+      end
+
+      it_behaves_like 'borrows connections for multistore'
+    end
+
+    context 'with use_primary_store_as_default_for_repository_cache disabled' do
+      before do
+        stub_feature_flags(
+          use_primary_store_as_default_for_repository_cache: false,
+          use_primary_and_secondary_stores_for_repository_cache: true
+        )
+      end
+
+      it_behaves_like 'borrows connections for multistore'
+    end
+
+    context 'with both feature flags disabled' do
+      before do
+        stub_feature_flags(
+          use_primary_store_as_default_for_repository_cache: false,
+          use_primary_and_secondary_stores_for_repository_cache: false
+        )
+      end
+
+      it 'does not borrow connections for the multistore' do
+        expect(Gitlab::Redis::RepositoryCache).not_to receive(:with)
+
+        operation
+      end
+    end
+  end
+
   describe '#expire' do
+    subject(:operation) { cache.expire(:foo) }
+
     it 'expires the given key from the cache' do
-      cache.expire(:foo)
+      operation
+
       expect(backend).to have_received(:delete).with("foo:#{namespace}")
     end
+
+    it_behaves_like 'compatiblity with multistore'
   end
 
   describe '#fetch' do
+    subject(:operation) { cache.fetch(:bar) }
+
     it 'fetches the given key from the cache' do
-      cache.fetch(:bar)
+      operation
       expect(backend).to have_received(:fetch).with("bar:#{namespace}")
     end
 
+    it_behaves_like 'compatiblity with multistore'
+
     it 'accepts a block' do
       p = -> {}
 
@@ -78,11 +144,15 @@
   end
 
   describe '#write' do
+    subject(:operation) { cache.write(:test, 'test') }
+
     it 'writes the given key and value to the cache' do
-      cache.write(:test, 'test')
+      operation
       expect(backend).to have_received(:write).with("test:#{namespace}", 'test')
     end
 
+    it_behaves_like 'compatiblity with multistore'
+
     it 'passes additional options to the backend' do
       cache.write(:test, 'test', expires_in: 10.minutes)
       expect(backend).to have_received(:write).with("test:#{namespace}", 'test', expires_in: 10.minutes)
@@ -173,4 +243,26 @@
       end
     end
   end
+
+  describe '#exist?' do
+    subject(:operation) { cache.exist?(:test) }
+
+    it 'calls exists? on backend' do
+      operation
+      expect(backend).to have_received(:exist?).with("test:#{namespace}")
+    end
+
+    it_behaves_like 'compatiblity with multistore'
+  end
+
+  describe '#read' do
+    subject(:operation) { cache.read(:test) }
+
+    it 'calls read on backend' do
+      operation
+      expect(backend).to have_received(:read).with("test:#{namespace}")
+    end
+
+    it_behaves_like 'compatiblity with multistore'
+  end
 end