Merge branch 'jcunha/activate-reactive-caching-only-for-environments' into 'master'

Sets ReactiveCaching limits only for Environment

See merge request gitlab-org/gitlab!34202

app/models/concerns/reactive_caching.rb

@@ -29,7 +29,7 @@ module ReactiveCaching
     self.reactive_cache_lease_timeout = 2.minutes
     self.reactive_cache_refresh_interval = 1.minute
     self.reactive_cache_lifetime = 10.minutes
-    self.reactive_cache_hard_limit = 1.megabyte
+    self.reactive_cache_hard_limit = nil # this value should be set in megabytes. E.g: 1.megabyte
     self.reactive_cache_work_type = :default
     self.reactive_cache_worker_finder = ->(id, *_args) do
       find_by(primary_key => id)
@@ -159,8 +159,12 @@ module ReactiveCaching
       WORK_TYPE.fetch(self.class.reactive_cache_work_type.to_sym)
     end
 
+    def reactive_cache_limit_enabled?
+      !!self.reactive_cache_hard_limit
+    end
+
     def check_exceeded_reactive_cache_limit!(data)
-      return unless Feature.enabled?(:reactive_cache_limit)
+      return unless reactive_cache_limit_enabled?
 
       data_deep_size = Gitlab::Utils::DeepSize.new(data, max_size: self.class.reactive_cache_hard_limit)
 

app/models/environment.rb

@@ -362,6 +362,11 @@ class Environment < ApplicationRecord
   def generate_slug
     self.slug = Gitlab::Slug::Environment.new(name).generate
   end
+
+  # Overrides ReactiveCaching default to activate limit checking behind a FF
+  def reactive_cache_limit_enabled?
+    Feature.enabled?(:reactive_caching_limit_environment, project)
+  end
 end
 
 Environment.prepend_if_ee('EE::Environment')

app/models/merge_request.rb

@@ -26,7 +26,6 @@ class MergeRequest < ApplicationRecord
   self.reactive_cache_key = ->(model) { [model.project.id, model.iid] }
   self.reactive_cache_refresh_interval = 10.minutes
   self.reactive_cache_lifetime = 10.minutes
-  self.reactive_cache_hard_limit = 20.megabytes
 
   SORTING_PREFERENCE_FIELD = :merge_requests_sort
 

spec/models/concerns/reactive_caching_spec.rb

@@ -285,38 +285,30 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
         go!
       end
 
-      context 'when calculated object size exceeds default reactive_cache_hard_limit' do
-        let(:calculation) { -> { 'a' * 2 * 1.megabyte } }
+      context 'when reactive_cache_hard_limit is set' do
+        let(:test_class) { Class.new(cache_class_test) { self.reactive_cache_hard_limit = 1.megabyte } }
+        let(:instance) { test_class.new(666, &calculation) }
+
+        context 'when cache size is over the overridden limit' do
+          let(:calculation) { -> { 'a' * 2 * 1.megabyte } }
 
-        shared_examples 'ExceededReactiveCacheLimit' do
           it 'raises ExceededReactiveCacheLimit exception and does not cache new data' do
             expect { go! }.to raise_exception(ReactiveCaching::ExceededReactiveCacheLimit)
 
             expect(read_reactive_cache(instance)).not_to eq(calculation.call)
           end
-        end
 
-        context 'when reactive_cache_hard_limit feature flag is enabled' do
-          it_behaves_like 'ExceededReactiveCacheLimit'
-
-          context 'when reactive_cache_hard_limit is overridden' do
-            let(:test_class) { Class.new(cache_class_test) { self.reactive_cache_hard_limit = 3.megabytes } }
-            let(:instance) { test_class.new(666, &calculation) }
+          context 'when reactive_cache_limit_enabled? is overriden to return false' do
+            before do
+              allow(instance).to receive(:reactive_cache_limit_enabled?).and_return(false)
+            end
 
             it_behaves_like 'successful cache'
-
-            context 'when cache size is over the overridden limit' do
-              let(:calculation) { -> { 'a' * 4 * 1.megabyte } }
-
-              it_behaves_like 'ExceededReactiveCacheLimit'
-            end
           end
         end
 
-        context 'when reactive_cache_limit feature flag is disabled' do
-          before do
-            stub_feature_flags(reactive_cache_limit: false)
-          end
+        context 'when cache size is within the overridden limit' do
+          let(:calculation) { -> { 'Smaller than 1Mb reactive_cache_hard_limit' } }
 
           it_behaves_like 'successful cache'
         end
@@ -377,7 +369,7 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
     it { expect(subject.reactive_cache_refresh_interval).to be_a(ActiveSupport::Duration) }
     it { expect(subject.reactive_cache_lifetime).to be_a(ActiveSupport::Duration) }
     it { expect(subject.reactive_cache_key).to respond_to(:call) }
-    it { expect(subject.reactive_cache_hard_limit).to be_a(Integer) }
+    it { expect(subject.reactive_cache_hard_limit).to be_nil }
     it { expect(subject.reactive_cache_worker_finder).to respond_to(:call) }
   end
 end

spec/models/environment_spec.rb

@@ -847,6 +847,20 @@ describe Environment, :use_clean_rails_memory_store_caching do
 
     subject { environment.calculate_reactive_cache }
 
+    it 'overrides default reactive_cache_hard_limit to 10 Mb' do
+      expect(described_class.reactive_cache_hard_limit).to eq(10.megabyte)
+    end
+
+    it 'overrides reactive_cache_limit_enabled? with a FF' do
+      environment_with_enabled_ff = FactoryBot.build(:environment)
+      environment_with_disabled_ff = FactoryBot.build(:environment)
+
+      stub_feature_flags(reactive_caching_limit_environment: environment_with_enabled_ff.project)
+
+      expect(environment_with_enabled_ff.send(:reactive_cache_limit_enabled?)).to be_truthy
+      expect(environment_with_disabled_ff.send(:reactive_cache_limit_enabled?)).to be_falsey
+    end
+
     it 'returns cache data from the deployment platform' do
       expect(environment.deployment_platform).to receive(:calculate_reactive_cache_for)
         .with(environment).and_return(pods: %w(pod1 pod2))