Don't use the redis set cache yet
For zero-downtime deployed in a mixed code environment between 12.2 and 12.3, the branch and tag name cache is incorrectly invalidated - a push to an old machine will not clear the redis set version of the cache on the new machine. This commit ensures that, in 12.3, both set and non-set versions of the cache are invalidated, but does not write or consult the set version of the cache. . In 12.4, it will be safe to switch branch and tag names to the redis set cache both it and the legacy cache will be invalidated appropriately in such a mixed code environment. This delays the full implementation of the feature by one release, but in the absence of a credible feature-flagging strategy, and amidst an abundance of caution about the effects of too-eager cache expiration, I believe this is the best approach available to us.
This commit is contained in:
parent
4cd6c91d5f
commit
034f0340a3
|
@ -239,13 +239,13 @@ class Repository
|
|||
def branch_exists?(branch_name)
|
||||
return false unless raw_repository
|
||||
|
||||
branch_names_include?(branch_name)
|
||||
branch_names.include?(branch_name)
|
||||
end
|
||||
|
||||
def tag_exists?(tag_name)
|
||||
return false unless raw_repository
|
||||
|
||||
tag_names_include?(tag_name)
|
||||
tag_names.include?(tag_name)
|
||||
end
|
||||
|
||||
def ref_exists?(ref)
|
||||
|
@ -569,10 +569,10 @@ class Repository
|
|||
end
|
||||
|
||||
delegate :branch_names, to: :raw_repository
|
||||
cache_method_as_redis_set :branch_names, fallback: []
|
||||
cache_method :branch_names, fallback: []
|
||||
|
||||
delegate :tag_names, to: :raw_repository
|
||||
cache_method_as_redis_set :tag_names, fallback: []
|
||||
cache_method :tag_names, fallback: []
|
||||
|
||||
delegate :branch_count, :tag_count, :has_visible_content?, to: :raw_repository
|
||||
cache_method :branch_count, fallback: 0
|
||||
|
|
|
@ -1,5 +0,0 @@
|
|||
---
|
||||
title: Cache branch and tag names as Redis sets
|
||||
merge_request: 30476
|
||||
author:
|
||||
type: performance
|
|
@ -1223,66 +1223,36 @@ describe Repository do
|
|||
end
|
||||
|
||||
describe '#branch_exists?' do
|
||||
let(:branch) { repository.root_ref }
|
||||
it 'uses branch_names' do
|
||||
allow(repository).to receive(:branch_names).and_return(['foobar'])
|
||||
|
||||
subject { repository.branch_exists?(branch) }
|
||||
|
||||
it 'delegates to branch_names when the cache is empty' do
|
||||
repository.expire_branches_cache
|
||||
|
||||
expect(repository).to receive(:branch_names).and_call_original
|
||||
is_expected.to eq(true)
|
||||
end
|
||||
|
||||
it 'uses redis set caching when the cache is filled' do
|
||||
repository.branch_names # ensure the branch name cache is filled
|
||||
|
||||
expect(repository)
|
||||
.to receive(:branch_names_include?)
|
||||
.with(branch)
|
||||
.and_call_original
|
||||
|
||||
is_expected.to eq(true)
|
||||
expect(repository.branch_exists?('foobar')).to eq(true)
|
||||
expect(repository.branch_exists?('master')).to eq(false)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#tag_exists?' do
|
||||
let(:tag) { repository.tags.first.name }
|
||||
it 'uses tag_names' do
|
||||
allow(repository).to receive(:tag_names).and_return(['foobar'])
|
||||
|
||||
subject { repository.tag_exists?(tag) }
|
||||
|
||||
it 'delegates to tag_names when the cache is empty' do
|
||||
repository.expire_tags_cache
|
||||
|
||||
expect(repository).to receive(:tag_names).and_call_original
|
||||
is_expected.to eq(true)
|
||||
end
|
||||
|
||||
it 'uses redis set caching when the cache is filled' do
|
||||
repository.tag_names # ensure the tag name cache is filled
|
||||
|
||||
expect(repository)
|
||||
.to receive(:tag_names_include?)
|
||||
.with(tag)
|
||||
.and_call_original
|
||||
|
||||
is_expected.to eq(true)
|
||||
expect(repository.tag_exists?('foobar')).to eq(true)
|
||||
expect(repository.tag_exists?('master')).to eq(false)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#branch_names', :clean_gitlab_redis_cache do
|
||||
describe '#branch_names', :use_clean_rails_memory_store_caching do
|
||||
let(:fake_branch_names) { ['foobar'] }
|
||||
|
||||
it 'gets cached across Repository instances' do
|
||||
allow(repository.raw_repository).to receive(:branch_names).once.and_return(fake_branch_names)
|
||||
|
||||
expect(repository.branch_names).to match_array(fake_branch_names)
|
||||
expect(repository.branch_names).to eq(fake_branch_names)
|
||||
|
||||
fresh_repository = Project.find(project.id).repository
|
||||
expect(fresh_repository.object_id).not_to eq(repository.object_id)
|
||||
|
||||
expect(fresh_repository.raw_repository).not_to receive(:branch_names)
|
||||
expect(fresh_repository.branch_names).to match_array(fake_branch_names)
|
||||
expect(fresh_repository.branch_names).to eq(fake_branch_names)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in New Issue