Merge branch 'revert-2390a213' into 'master'

Revert "Merge branch 'sh-fix-instance-variable-caching' into 'master'"

See merge request gitlab-org/gitlab!79314

app/models/ci/instance_variable.rb

@@ -2,6 +2,7 @@
 
 module Ci
   class InstanceVariable < Ci::ApplicationRecord
+    extend Gitlab::ProcessMemoryCache::Helper
     include Ci::NewHasVariable
     include Ci::Maskable
     include Limitable
@@ -35,23 +36,15 @@ module Ci
         cached_data[:unprotected]
       end
 
-      def invalidate_memory_cache(key)
-        cache_backend.delete(key)
-      end
-
       private
 
       def cached_data
-        cache_backend.fetch(:ci_instance_variable_data, expires_in: 30.seconds) do
+        fetch_memory_cache(:ci_instance_variable_data) do
           all_records = unscoped.all.to_a
 
           { all: all_records, unprotected: all_records.reject(&:protected?) }
         end
       end
-
-      def cache_backend
-        Gitlab::ProcessMemoryCache.cache_backend
-      end
     end
   end
 end

lib/gitlab/process_memory_cache/helper.rb

+# frozen_string_literal: true
+
+module Gitlab
+  class ProcessMemoryCache
+    module Helper
+      def fetch_memory_cache(key, &payload)
+        cache = cache_backend.read(key)
+
+        if cache && !stale_cache?(key, cache)
+          cache[:data]
+        else
+          store_cache(key, &payload)
+        end
+      end
+
+      def invalidate_memory_cache(key)
+        touch_cache_timestamp(key)
+      end
+
+      private
+
+      def touch_cache_timestamp(key, time = Time.current.to_f)
+        shared_backend.write(key, time)
+      end
+
+      def stale_cache?(key, cache_info)
+        shared_timestamp = shared_backend.read(key)
+        return true unless shared_timestamp
+
+        shared_timestamp.to_f > cache_info[:cached_at].to_f
+      end
+
+      def store_cache(key)
+        data = yield
+        time = Time.current.to_f
+
+        cache_backend.write(key, data: data, cached_at: time)
+        touch_cache_timestamp(key, time) unless shared_backend.read(key)
+        data
+      end
+
+      def shared_backend
+        Rails.cache
+      end
+
+      def cache_backend
+        ::Gitlab::ProcessMemoryCache.cache_backend
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb

@@ -252,7 +252,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do
         extra_jobs = 2
         non_handled_sql_queries = 2
 
-        # 1. Ci::InstanceVariable Load => `Ci::InstanceVariable#cached_data` => already cached with `ProcessMemoryCache`
+        # 1. Ci::InstanceVariable Load => `Ci::InstanceVariable#cached_data` => already cached with `fetch_memory_cache`
         # 2. Ci::Variable Load => `Project#ci_variables_for` => already cached with `Gitlab::SafeRequestStore`
 
         extra_jobs * non_handled_sql_queries

spec/lib/gitlab/process_memory_cache/helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::ProcessMemoryCache::Helper, :use_clean_rails_memory_store_caching do
+  let(:minimal_test_class) do
+    Class.new do
+      include Gitlab::ProcessMemoryCache::Helper
+
+      def cached_content
+        fetch_memory_cache(:cached_content_instance_key) { expensive_computation }
+      end
+
+      def clear_cached_content
+        invalidate_memory_cache(:cached_content_instance_key)
+      end
+    end
+  end
+
+  before do
+    stub_const("MinimalTestClass", minimal_test_class)
+  end
+
+  subject { MinimalTestClass.new }
+
+  describe '.fetch_memory_cache' do
+    it 'memoizes the result' do
+      is_expected.to receive(:expensive_computation).once.and_return(1)
+
+      2.times do
+        expect(subject.cached_content).to eq(1)
+      end
+    end
+
+    it 'resets the cache when the shared key is missing', :aggregate_failures do
+      allow(Rails.cache).to receive(:read).with(:cached_content_instance_key).and_return(nil)
+      is_expected.to receive(:expensive_computation).thrice.and_return(1, 2, 3)
+
+      3.times do |index|
+        expect(subject.cached_content).to eq(index + 1)
+      end
+    end
+
+    it 'does not set the shared timestamp if it is already present', :redis do
+      subject.clear_cached_content
+      is_expected.to receive(:expensive_computation).once.and_return(1)
+
+      expect { subject.cached_content }.not_to change { Rails.cache.read(:cached_content_instance_key) }
+    end
+  end
+
+  describe '.invalidate_memory_cache' do
+    it 'invalidates the cache' do
+      is_expected.to receive(:expensive_computation).twice.and_return(1, 2)
+
+      expect { subject.clear_cached_content }.to change { subject.cached_content }
+    end
+  end
+end