Add Gitlab.revision to Gitlab::JsonCache cache_key by default

Update Gitlab::JsonCache to add Gitlab.revision to the cache_key by
default instead of using Gitlab::VERSION and Rails.version. This change
will result to broadcast messages (and anything that uses JsonCache
going forward) cache invalidation on every deploy which should prevent
the stale data issue encountered in
https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6372.

For backwards compatibility, Gitlab::JsonCache accepts
{ cache_key_strategy: :version } (adds Gitlab::VERSION and
Rails.version to the cache_key) which will invalidate the cache on every
bump of Gitlab::VERSION (every release, once a month) and Rails.version.

Changelog: changed

app/models/broadcast_message.rb

@@ -53,7 +53,7 @@ class BroadcastMessage < ApplicationRecord
 
     def cache
       ::Gitlab::SafeRequestStore.fetch(:broadcast_message_json_cache) do
-        Gitlab::JsonCache.new(cache_key_with_version: false)
+        Gitlab::JsonCache.new
       end
     end
 

ee/app/models/license.rb

@@ -315,7 +315,7 @@ class License < ApplicationRecord
 
     def cache
       Gitlab::SafeRequestStore[:license_cache] ||=
-        Gitlab::JsonCache.new(namespace: :ee, backend: ::Gitlab::ProcessMemoryCache.cache_backend)
+        Gitlab::JsonCache.new(namespace: :ee, backend: ::Gitlab::ProcessMemoryCache.cache_backend, cache_key_strategy: :version)
     end
 
     def all_plans

ee/lib/gitlab/geo.rb

@@ -115,11 +115,11 @@ module Gitlab
 
     def self.l1_cache
       SafeRequestStore[:geo_l1_cache] ||=
-        Gitlab::JsonCache.new(namespace: :geo, backend: ::Gitlab::ProcessMemoryCache.cache_backend)
+        Gitlab::JsonCache.new(namespace: :geo, backend: ::Gitlab::ProcessMemoryCache.cache_backend, cache_key_strategy: :version)
     end
 
     def self.l2_cache
-      SafeRequestStore[:geo_l2_cache] ||= Gitlab::JsonCache.new(namespace: :geo)
+      SafeRequestStore[:geo_l2_cache] ||= Gitlab::JsonCache.new(namespace: :geo, cache_key_strategy: :version)
     end
 
     def self.cache_value(raw_key, as: nil, &block)

lib/gitlab/json_cache.rb

@@ -2,12 +2,17 @@
 
 module Gitlab
   class JsonCache
-    attr_reader :backend, :cache_key_with_version, :namespace
+    attr_reader :backend, :namespace
+
+    STRATEGY_KEY_COMPONENTS = {
+      revision: Gitlab.revision,
+      version: [Gitlab::VERSION, Rails.version]
+    }.freeze
 
     def initialize(options = {})
       @backend = options.fetch(:backend, Rails.cache)
       @namespace = options.fetch(:namespace, nil)
-      @cache_key_with_version = options.fetch(:cache_key_with_version, true)
+      @cache_key_strategy = options.fetch(:cache_key_strategy, :revision)
     end
 
     def active?
@@ -19,13 +24,12 @@ module Gitlab
     end
 
     def cache_key(key)
-      expanded_cache_key = [namespace, key].compact
-
-      if cache_key_with_version
-        expanded_cache_key << [Gitlab::VERSION, Rails.version]
-      end
+      expanded_cache_key = [namespace, key, *strategy_key_component].compact
+      expanded_cache_key.join(':').freeze
+    end
 
-      expanded_cache_key.flatten.join(':').freeze
+    def strategy_key_component
+      STRATEGY_KEY_COMPONENTS.fetch(@cache_key_strategy)
     end
 
     def expire(key)

spec/lib/gitlab/json_cache_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Gitlab::JsonCache do
   let(:backend) { double('backend').as_null_object }
   let(:namespace) { 'geo' }
   let(:key) { 'foo' }
-  let(:expanded_key) { "#{namespace}:#{key}:#{Gitlab::VERSION}:#{Rails.version}" }
+  let(:expanded_key) { "#{namespace}:#{key}:#{Gitlab.revision}" }
 
   subject(:cache) { described_class.new(namespace: namespace, backend: backend) }
 
@@ -35,69 +35,63 @@ RSpec.describe Gitlab::JsonCache do
   end
 
   describe '#cache_key' do
-    context 'when namespace is not defined' do
-      context 'when cache_key_with_version is true' do
-        it 'expands out the key with GitLab, and Rails versions' do
-          cache = described_class.new(cache_key_with_version: true)
+    using RSpec::Parameterized::TableSyntax
 
-          cache_key = cache.cache_key(key)
-
-          expect(cache_key).to eq("#{key}:#{Gitlab::VERSION}:#{Rails.version}")
-        end
-      end
+    where(:namespace, :cache_key_strategy, :expanded_key) do
+      nil       | :revision | "#{key}:#{Gitlab.revision}"
+      nil       | :version  | "#{key}:#{Gitlab::VERSION}:#{Rails.version}"
+      namespace | :revision | "#{namespace}:#{key}:#{Gitlab.revision}"
+      namespace | :version  | "#{namespace}:#{key}:#{Gitlab::VERSION}:#{Rails.version}"
+    end
 
-      context 'when cache_key_with_version is false' do
-        it 'returns the key' do
-          cache = described_class.new(namespace: nil, cache_key_with_version: false)
+    with_them do
+      let(:cache) { described_class.new(namespace: namespace, cache_key_strategy: cache_key_strategy) }
 
-          cache_key = cache.cache_key(key)
+      subject { cache.cache_key(key) }
 
-          expect(cache_key).to eq(key)
-        end
-      end
+      it { is_expected.to eq expanded_key }
     end
 
-    context 'when namespace is nil' do
-      context 'when cache_key_with_version is true' do
-        it 'expands out the key with GitLab, and Rails versions' do
-          cache = described_class.new(cache_key_with_version: true)
-
-          cache_key = cache.cache_key(key)
+    context 'when cache_key_strategy is unknown' do
+      let(:cache) { described_class.new(namespace: namespace, cache_key_strategy: 'unknown') }
 
-          expect(cache_key).to eq("#{key}:#{Gitlab::VERSION}:#{Rails.version}")
-        end
+      it 'raises KeyError' do
+        expect { cache.cache_key('key') }.to raise_error(KeyError)
       end
+    end
+  end
 
-      context 'when cache_key_with_version is false' do
-        it 'returns the key' do
-          cache = described_class.new(namespace: nil, cache_key_with_version: false)
+  describe '#namespace' do
+    it 'defaults to nil' do
+      cache = described_class.new
+      expect(cache.namespace).to be_nil
+    end
+  end
 
-          cache_key = cache.cache_key(key)
+  describe '#strategy_key_component' do
+    subject { cache.strategy_key_component }
 
-          expect(cache_key).to eq(key)
-        end
-      end
+    it 'defaults to Gitlab.revision' do
+      expect(described_class.new.strategy_key_component).to eq Gitlab.revision
     end
 
-    context 'when namespace is set' do
-      context 'when cache_key_with_version is true' do
-        it 'expands out the key with namespace and Rails version' do
-          cache = described_class.new(namespace: namespace, cache_key_with_version: true)
+    context 'when cache_key_strategy is :revision' do
+      let(:cache) { described_class.new(cache_key_strategy: :revision) }
 
-          cache_key = cache.cache_key(key)
+      it { is_expected.to eq Gitlab.revision }
+    end
 
-          expect(cache_key).to eq("#{namespace}:#{key}:#{Gitlab::VERSION}:#{Rails.version}")
-        end
-      end
+    context 'when cache_key_strategy is :version' do
+      let(:cache) { described_class.new(cache_key_strategy: :version) }
 
-      context 'when cache_key_with_version is false' do
-        it 'expands out the key with namespace' do
-          cache = described_class.new(namespace: namespace, cache_key_with_version: false)
+      it { is_expected.to eq [Gitlab::VERSION, Rails.version] }
+    end
 
-          cache_key = cache.cache_key(key)
+    context 'when cache_key_strategy is invalid' do
+      let(:cache) { described_class.new(cache_key_strategy: 'unknown') }
 
-          expect(cache_key).to eq("#{namespace}:#{key}")
-        end
+      it 'raises KeyError' do
+        expect { subject }.to raise_error(KeyError)
       end
     end
   end

spec/models/broadcast_message_spec.rb

@@ -286,9 +286,9 @@ RSpec.describe BroadcastMessage do
     it 'flushes the Redis cache' do
       message = create(:broadcast_message)
 
-      expect(Rails.cache).to receive(:delete).with(described_class::CACHE_KEY)
-      expect(Rails.cache).to receive(:delete).with(described_class::BANNER_CACHE_KEY)
-      expect(Rails.cache).to receive(:delete).with(described_class::NOTIFICATION_CACHE_KEY)
+      expect(Rails.cache).to receive(:delete).with("#{described_class::CACHE_KEY}:#{Gitlab.revision}")
+      expect(Rails.cache).to receive(:delete).with("#{described_class::BANNER_CACHE_KEY}:#{Gitlab.revision}")
+      expect(Rails.cache).to receive(:delete).with("#{described_class::NOTIFICATION_CACHE_KEY}:#{Gitlab.revision}")
 
       message.flush_redis_cache
     end