8 files changed, 153 insertions, 137 deletions

diff --git a/spec/lib/gitlab/redis/cache_spec.rb b/spec/lib/gitlab/redis/cache_spec.rb
index 82ff8a26199..64615c4d9ad 100644
--- a/spec/lib/gitlab/redis/cache_spec.rb
+++ b/spec/lib/gitlab/redis/cache_spec.rb
@@ -26,5 +26,22 @@ RSpec.describe Gitlab::Redis::Cache do
 
       expect(described_class.active_support_config[:expires_in]).to eq(1.day)
     end
+
+    context 'when encountering an error' do
+      let(:cache) { ActiveSupport::Cache::RedisCacheStore.new(**described_class.active_support_config) }
+
+      subject { cache.read('x') }
+
+      before do
+        described_class.with do |redis|
+          allow(redis).to receive(:get).and_raise(::Redis::CommandError)
+        end
+      end
+
+      it 'logs error' do
+        expect(::Gitlab::ErrorTracking).to receive(:log_exception)
+        subject
+      end
+    end
   end
 end
diff --git a/spec/lib/gitlab/redis/cluster_rate_limiting_spec.rb b/spec/lib/gitlab/redis/cluster_rate_limiting_spec.rb
new file mode 100644
index 00000000000..3eba3233f08
--- /dev/null
+++ b/spec/lib/gitlab/redis/cluster_rate_limiting_spec.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Redis::ClusterRateLimiting, feature_category: :redis do
+  include_examples "redis_new_instance_shared_examples", 'cluster_rate_limiting', Gitlab::Redis::Cache
+end
diff --git a/spec/lib/gitlab/redis/db_load_balancing_spec.rb b/spec/lib/gitlab/redis/db_load_balancing_spec.rb
new file mode 100644
index 00000000000..d633413ddec
--- /dev/null
+++ b/spec/lib/gitlab/redis/db_load_balancing_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Redis::DbLoadBalancing, feature_category: :scalability do
+  include_examples "redis_new_instance_shared_examples", 'db_load_balancing', Gitlab::Redis::SharedState
+  include_examples "redis_shared_examples"
+
+  describe '#pool' do
+    let(:config_new_format_host) { "spec/fixtures/config/redis_new_format_host.yml" }
+    let(:config_new_format_socket) { "spec/fixtures/config/redis_new_format_socket.yml" }
+
+    subject { described_class.pool }
+
+    before do
+      allow(described_class).to receive(:config_file_name).and_return(config_new_format_host)
+
+      # Override rails root to avoid having our fixtures overwritten by `redis.yml` if it exists
+      allow(Gitlab::Redis::SharedState).to receive(:rails_root).and_return(mktmpdir)
+      allow(Gitlab::Redis::SharedState).to receive(:config_file_name).and_return(config_new_format_socket)
+    end
+
+    around do |example|
+      clear_pool
+      example.run
+    ensure
+      clear_pool
+    end
+
+    it 'instantiates an instance of MultiStore' do
+      subject.with do |redis_instance|
+        expect(redis_instance).to be_instance_of(::Gitlab::Redis::MultiStore)
+
+        expect(redis_instance.primary_store.connection[:id]).to eq("redis://test-host:6379/99")
+        expect(redis_instance.secondary_store.connection[:id]).to eq("unix:///path/to/redis.sock/0")
+
+        expect(redis_instance.instance_name).to eq('DbLoadBalancing')
+      end
+    end
+
+    it_behaves_like 'multi store feature flags', :use_primary_and_secondary_stores_for_db_load_balancing,
+      :use_primary_store_as_default_for_db_load_balancing
+  end
+
+  describe '#raw_config_hash' do
+    it 'has a legacy default URL' do
+      expect(subject).to receive(:fetch_config).and_return(false)
+
+      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6382')
+    end
+  end
+end
diff --git a/spec/lib/gitlab/redis/duplicate_jobs_spec.rb b/spec/lib/gitlab/redis/duplicate_jobs_spec.rb
deleted file mode 100644
index 4d46a567032..00000000000
--- a/spec/lib/gitlab/redis/duplicate_jobs_spec.rb
+++ /dev/null
@@ -1,84 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Redis::DuplicateJobs do
-  # Note: this is a pseudo-store in front of `SharedState`, meant only as a tool
-  # to move away from `Sidekiq.redis` for duplicate job data. Thus, we use the
-  # same store configuration as the former.
-  let(:instance_specific_config_file) { "config/redis.shared_state.yml" }
-  let(:environment_config_file_name) { "GITLAB_REDIS_SHARED_STATE_CONFIG_FILE" }
-
-  include_examples "redis_shared_examples"
-
-  describe '#pool' do
-    subject { described_class.pool }
-
-    around do |example|
-      clear_pool
-      example.run
-    ensure
-      clear_pool
-    end
-
-    context 'store connection settings' do
-      let(:config_new_format_host) { "spec/fixtures/config/redis_new_format_host.yml" }
-      let(:config_new_format_socket) { "spec/fixtures/config/redis_new_format_socket.yml" }
-
-      before do
-        allow(Gitlab::Redis::SharedState).to receive(:config_file_name).and_return(config_new_format_host)
-        allow(Gitlab::Redis::Queues).to receive(:config_file_name).and_return(config_new_format_socket)
-      end
-
-      it 'instantiates an instance of MultiStore' do
-        subject.with do |redis_instance|
-          expect(redis_instance).to be_instance_of(::Gitlab::Redis::MultiStore)
-
-          expect(redis_instance.primary_store.connection[:id]).to eq("redis://test-host:6379/99")
-          expect(redis_instance.primary_store.connection[:namespace]).to be_nil
-          expect(redis_instance.secondary_store.connection[:id]).to eq("unix:///path/to/redis.sock/0")
-          expect(redis_instance.secondary_store.connection[:namespace]).to eq("resque:gitlab")
-
-          expect(redis_instance.instance_name).to eq('DuplicateJobs')
-        end
-      end
-    end
-
-    # Make sure they current namespace is respected for the secondary store but omitted from the primary
-    context 'key namespaces' do
-      let(:key) { 'key' }
-      let(:value) { '123' }
-
-      it 'writes keys to SharedState with no prefix, and to Queues with the "resque:gitlab:" prefix' do
-        subject.with do |redis_instance|
-          redis_instance.set(key, value)
-        end
-
-        Gitlab::Redis::SharedState.with do |redis_instance|
-          expect(redis_instance.get(key)).to eq(value)
-        end
-
-        Gitlab::Redis::Queues.with do |redis_instance|
-          expect(redis_instance.get("resque:gitlab:#{key}")).to eq(value)
-        end
-      end
-    end
-
-    it_behaves_like 'multi store feature flags', :use_primary_and_secondary_stores_for_duplicate_jobs,
-                                                 :use_primary_store_as_default_for_duplicate_jobs
-  end
-
-  describe '#raw_config_hash' do
-    it 'has a legacy default URL' do
-      expect(subject).to receive(:fetch_config) { false }
-
-      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6382')
-    end
-  end
-
-  describe '#store_name' do
-    it 'returns the name of the SharedState store' do
-      expect(described_class.store_name).to eq('SharedState')
-    end
-  end
-end
diff --git a/spec/lib/gitlab/redis/multi_store_spec.rb b/spec/lib/gitlab/redis/multi_store_spec.rb
index f198ba90d0a..423a7e80ead 100644
--- a/spec/lib/gitlab/redis/multi_store_spec.rb
+++ b/spec/lib/gitlab/redis/multi_store_spec.rb
@@ -210,7 +210,7 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
       end
     end
 
-    RSpec.shared_examples_for 'fallback read from the secondary store' do
+    RSpec.shared_examples_for 'fallback read from the non-default store' do
       let(:counter) { Gitlab::Metrics::NullMetric.instance }
 
       before do
@@ -218,7 +218,7 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
       end
 
       it 'fallback and execute on secondary instance' do
-        expect(secondary_store).to receive(name).with(*expected_args).and_call_original
+        expect(multi_store.fallback_store).to receive(name).with(*expected_args).and_call_original
 
         subject
       end
@@ -242,7 +242,7 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
 
       context 'when fallback read from the secondary instance raises an exception' do
         before do
-          allow(secondary_store).to receive(name).with(*expected_args).and_raise(StandardError)
+          allow(multi_store.fallback_store).to receive(name).with(*expected_args).and_raise(StandardError)
         end
 
         it 'fails with exception' do
@@ -283,10 +283,12 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
             subject
           end
 
-          it 'does not execute on the secondary store' do
-            expect(secondary_store).not_to receive(name)
+          unless params[:block]
+            it 'does not execute on the secondary store' do
+              expect(secondary_store).not_to receive(name)
 
-            subject
+              subject
+            end
           end
 
           include_examples 'reads correct value'
@@ -294,7 +296,7 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
 
         context 'when reading from primary instance is raising an exception' do
           before do
-            allow(primary_store).to receive(name).with(*expected_args).and_raise(StandardError)
+            allow(multi_store.default_store).to receive(name).with(*expected_args).and_raise(StandardError)
             allow(Gitlab::ErrorTracking).to receive(:log_exception)
           end
 
@@ -305,16 +307,16 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
             subject
           end
 
-          include_examples 'fallback read from the secondary store'
+          include_examples 'fallback read from the non-default store'
         end
 
-        context 'when reading from empty primary instance' do
+        context 'when reading from empty default instance' do
           before do
-            # this ensures a cache miss without having to stub primary store
-            primary_store.flushdb
+            # this ensures a cache miss without having to stub the default store
+            multi_store.default_store.flushdb
           end
 
-          include_examples 'fallback read from the secondary store'
+          include_examples 'fallback read from the non-default store'
         end
 
         context 'when the command is executed within pipelined block' do
@@ -344,8 +346,16 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
           end
 
           context 'when block is provided' do
-            it 'yields to the block' do
+            it 'both stores yields to the block' do
               expect(primary_store).to receive(name).and_yield(value)
+              expect(secondary_store).to receive(name).and_yield(value)
+
+              subject
+            end
+
+            it 'both stores to execute' do
+              expect(primary_store).to receive(name).with(*expected_args).and_call_original
+              expect(secondary_store).to receive(name).with(*expected_args).and_call_original
 
               subject
             end
@@ -412,7 +422,6 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
       multi_store.mget(values) do |v|
         multi_store.sadd(skey, v)
         multi_store.scard(skey)
-        v # mget receiving block returns the last line of the block for cache-hit check
       end
     end
 
@@ -422,9 +431,9 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
         expect(primary_store).to receive(:send).with(:sadd, skey, %w[1 2 3]).and_call_original
         expect(primary_store).to receive(:send).with(:scard, skey).and_call_original
 
-        expect(secondary_store).not_to receive(:send).with(:mget, values).and_call_original
-        expect(secondary_store).not_to receive(:send).with(:sadd, skey, %w[1 2 3]).and_call_original
-        expect(secondary_store).not_to receive(:send).with(:scard, skey).and_call_original
+        expect(secondary_store).to receive(:send).with(:mget, values).and_call_original
+        expect(secondary_store).to receive(:send).with(:sadd, skey, %w[10 20 30]).and_call_original
+        expect(secondary_store).to receive(:send).with(:scard, skey).and_call_original
 
         subject
       end
@@ -451,7 +460,15 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
       end
 
       context 'when primary instance is default store' do
-        it_behaves_like 'primary instance executes block'
+        it 'ensures only primary instance is executing the block' do
+          expect(secondary_store).not_to receive(:send)
+
+          expect(primary_store).to receive(:send).with(:mget, values).and_call_original
+          expect(primary_store).to receive(:send).with(:sadd, skey, %w[1 2 3]).and_call_original
+          expect(primary_store).to receive(:send).with(:scard, skey).and_call_original
+
+          subject
+        end
       end
 
       context 'when secondary instance is default store' do
@@ -464,9 +481,7 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
           expect(secondary_store).to receive(:send).with(:sadd, skey, %w[10 20 30]).and_call_original
           expect(secondary_store).to receive(:send).with(:scard, skey).and_call_original
 
-          expect(primary_store).not_to receive(:send).with(:mget, values).and_call_original
-          expect(primary_store).not_to receive(:send).with(:sadd, skey, %w[10 20 30]).and_call_original
-          expect(primary_store).not_to receive(:send).with(:scard, skey).and_call_original
+          expect(primary_store).not_to receive(:send)
 
           subject
         end
@@ -700,7 +715,7 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
         it 'runs block on correct Redis instance' do
           if both_stores
             expect(primary_store).to receive(name).with(*expected_args).and_call_original
-            expect(secondary_store).not_to receive(name)
+            expect(secondary_store).to receive(name).with(*expected_args).and_call_original
 
             expect(primary_store).to receive(:incr).with(rvalue)
             expect(secondary_store).to receive(:incr).with(rvalue)
diff --git a/spec/lib/gitlab/redis/rate_limiting_spec.rb b/spec/lib/gitlab/redis/rate_limiting_spec.rb
index e79c070df93..d82228426f0 100644
--- a/spec/lib/gitlab/redis/rate_limiting_spec.rb
+++ b/spec/lib/gitlab/redis/rate_limiting_spec.rb
@@ -4,4 +4,21 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Redis::RateLimiting do
   include_examples "redis_new_instance_shared_examples", 'rate_limiting', Gitlab::Redis::Cache
+
+  describe '.cache_store' do
+    context 'when encountering an error' do
+      subject { described_class.cache_store.read('x') }
+
+      before do
+        described_class.with do |redis|
+          allow(redis).to receive(:get).and_raise(::Redis::CommandError)
+        end
+      end
+
+      it 'logs error' do
+        expect(::Gitlab::ErrorTracking).to receive(:log_exception)
+        subject
+      end
+    end
+  end
 end
diff --git a/spec/lib/gitlab/redis/repository_cache_spec.rb b/spec/lib/gitlab/redis/repository_cache_spec.rb
index b11e9ebf1f3..2c167a6eb62 100644
--- a/spec/lib/gitlab/redis/repository_cache_spec.rb
+++ b/spec/lib/gitlab/redis/repository_cache_spec.rb
@@ -4,46 +4,33 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Redis::RepositoryCache, feature_category: :scalability do
   include_examples "redis_new_instance_shared_examples", 'repository_cache', Gitlab::Redis::Cache
-  include_examples "redis_shared_examples"
 
-  describe '#pool' do
-    let(:config_new_format_host) { "spec/fixtures/config/redis_new_format_host.yml" }
-    let(:config_new_format_socket) { "spec/fixtures/config/redis_new_format_socket.yml" }
-
-    subject { described_class.pool }
+  describe '#raw_config_hash' do
+    it 'has a legacy default URL' do
+      expect(subject).to receive(:fetch_config).and_return(false)
 
-    before do
-      allow(described_class).to receive(:config_file_name).and_return(config_new_format_host)
-      allow(Gitlab::Redis::Cache).to receive(:config_file_name).and_return(config_new_format_socket)
+      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6380')
     end
+  end
 
-    around do |example|
-      clear_pool
-      example.run
-    ensure
-      clear_pool
+  describe '.cache_store' do
+    it 'has a default ttl of 8 hours' do
+      expect(described_class.cache_store.options[:expires_in]).to eq(8.hours)
     end
 
-    it 'instantiates an instance of MultiStore' do
-      subject.with do |redis_instance|
-        expect(redis_instance).to be_instance_of(::Gitlab::Redis::MultiStore)
+    context 'when encountering an error' do
+      subject { described_class.cache_store.read('x') }
 
-        expect(redis_instance.primary_store.connection[:id]).to eq("redis://test-host:6379/99")
-        expect(redis_instance.secondary_store.connection[:id]).to eq("unix:///path/to/redis.sock/0")
-
-        expect(redis_instance.instance_name).to eq('RepositoryCache')
+      before do
+        described_class.with do |redis|
+          allow(redis).to receive(:get).and_raise(::Redis::CommandError)
+        end
       end
-    end
 
-    it_behaves_like 'multi store feature flags', :use_primary_and_secondary_stores_for_repository_cache,
-                    :use_primary_store_as_default_for_repository_cache
-  end
-
-  describe '#raw_config_hash' do
-    it 'has a legacy default URL' do
-      expect(subject).to receive(:fetch_config).and_return(false)
-
-      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6380')
+      it 'logs error' do
+        expect(::Gitlab::ErrorTracking).to receive(:log_exception)
+        subject
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/redis/sidekiq_status_spec.rb b/spec/lib/gitlab/redis/sidekiq_status_spec.rb
index e7cf229b494..bbfec13e6c8 100644
--- a/spec/lib/gitlab/redis/sidekiq_status_spec.rb
+++ b/spec/lib/gitlab/redis/sidekiq_status_spec.rb
@@ -14,10 +14,15 @@ RSpec.describe Gitlab::Redis::SidekiqStatus do
   describe '#pool' do
     let(:config_new_format_host) { "spec/fixtures/config/redis_new_format_host.yml" }
     let(:config_new_format_socket) { "spec/fixtures/config/redis_new_format_socket.yml" }
+    let(:rails_root) { mktmpdir }
 
     subject { described_class.pool }
 
     before do
+      # Override rails root to avoid having our fixtures overwritten by `redis.yml` if it exists
+      allow(Gitlab::Redis::SharedState).to receive(:rails_root).and_return(rails_root)
+      allow(Gitlab::Redis::Queues).to receive(:rails_root).and_return(rails_root)
+
       allow(Gitlab::Redis::SharedState).to receive(:config_file_name).and_return(config_new_format_host)
       allow(Gitlab::Redis::Queues).to receive(:config_file_name).and_return(config_new_format_socket)
     end