Commit 0134e539 authored by Marius Bobin's avatar Marius Bobin Committed by Stan Hu

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

This reverts merge request !78944
parent 30f2a1d8
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
module Ci module Ci
class InstanceVariable < Ci::ApplicationRecord class InstanceVariable < Ci::ApplicationRecord
extend Gitlab::ProcessMemoryCache::Helper
include Ci::NewHasVariable include Ci::NewHasVariable
include Ci::Maskable include Ci::Maskable
include Limitable include Limitable
...@@ -35,23 +36,15 @@ module Ci ...@@ -35,23 +36,15 @@ module Ci
cached_data[:unprotected] cached_data[:unprotected]
end end
def invalidate_memory_cache(key)
cache_backend.delete(key)
end
private private
def cached_data 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_records = unscoped.all.to_a
{ all: all_records, unprotected: all_records.reject(&:protected?) } { all: all_records, unprotected: all_records.reject(&:protected?) }
end end
end end
def cache_backend
Gitlab::ProcessMemoryCache.cache_backend
end
end end
end end
end end
# 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
...@@ -252,7 +252,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do ...@@ -252,7 +252,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do
extra_jobs = 2 extra_jobs = 2
non_handled_sql_queries = 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` # 2. Ci::Variable Load => `Project#ci_variables_for` => already cached with `Gitlab::SafeRequestStore`
extra_jobs * non_handled_sql_queries extra_jobs * non_handled_sql_queries
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment