Commit 568905a0 authored by Robert May's avatar Robert May Committed by Stan Hu

Swap to UNLINK for Redis set cache

This uses UNLINK in Redis 4 to delete sets. This unlinks the key
from the value but actually deletes the value asynchronously.
With large sets this should give us a reasonable performance boost.

This commit also implements multi-key deletes for RepositorySetCache
methods in the Repository model, meaning only one UNLINK call will be
issued for all sets being expired.
parent 960eff07
---
title: Swap to UNLINK for Redis set cache
merge_request: 27116
author:
type: performance
...@@ -237,7 +237,7 @@ module Gitlab ...@@ -237,7 +237,7 @@ module Gitlab
end end
def expire_redis_set_method_caches(methods) def expire_redis_set_method_caches(methods)
methods.each { |name| redis_set_cache.expire(name) } redis_set_cache.expire(*methods)
end end
def expire_redis_hash_method_caches(methods) def expire_redis_hash_method_caches(methods)
......
...@@ -14,8 +14,16 @@ module Gitlab ...@@ -14,8 +14,16 @@ module Gitlab
"#{key}:set" "#{key}:set"
end end
def expire(key) def expire(*keys)
with { |redis| redis.del(cache_key(key)) } with do |redis|
keys = keys.map { |key| cache_key(key) }
if Feature.enabled?(:repository_set_cache_unlink, default_enabled: true)
redis.unlink(*keys)
else
redis.delete(*keys)
end
end
end end
def exist?(key) def exist?(key)
......
...@@ -211,8 +211,7 @@ describe Gitlab::RepositoryCacheAdapter do ...@@ -211,8 +211,7 @@ describe Gitlab::RepositoryCacheAdapter do
it 'expires the caches of the given methods' do it 'expires the caches of the given methods' do
expect(cache).to receive(:expire).with(:rendered_readme) expect(cache).to receive(:expire).with(:rendered_readme)
expect(cache).to receive(:expire).with(:branch_names) expect(cache).to receive(:expire).with(:branch_names)
expect(redis_set_cache).to receive(:expire).with(:rendered_readme) expect(redis_set_cache).to receive(:expire).with(:rendered_readme, :branch_names)
expect(redis_set_cache).to receive(:expire).with(:branch_names)
expect(redis_hash_cache).to receive(:delete).with(:rendered_readme) expect(redis_hash_cache).to receive(:delete).with(:rendered_readme)
expect(redis_hash_cache).to receive(:delete).with(:branch_names) expect(redis_hash_cache).to receive(:delete).with(:branch_names)
......
...@@ -51,15 +51,44 @@ describe Gitlab::RepositorySetCache, :clean_gitlab_redis_cache do ...@@ -51,15 +51,44 @@ describe Gitlab::RepositorySetCache, :clean_gitlab_redis_cache do
end end
describe '#expire' do describe '#expire' do
it 'expires the given key from the cache' do subject { cache.expire(*keys) }
before do
cache.write(:foo, ['value']) cache.write(:foo, ['value'])
cache.write(:bar, ['value2'])
end
it 'actually wrote the values' do
expect(cache.read(:foo)).to contain_exactly('value') expect(cache.read(:foo)).to contain_exactly('value')
expect(cache.expire(:foo)).to eq(1) expect(cache.read(:bar)).to contain_exactly('value2')
end
context 'single key' do
let(:keys) { %w(foo) }
it { is_expected.to eq(1) }
it 'deletes the given key from the cache' do
subject
expect(cache.read(:foo)).to be_empty expect(cache.read(:foo)).to be_empty
end end
end end
context 'multiple keys' do
let(:keys) { %w(foo bar) }
it { is_expected.to eq(2) }
it 'deletes the given keys from the cache' do
subject
expect(cache.read(:foo)).to be_empty
expect(cache.read(:bar)).to be_empty
end
end
end
describe '#exist?' do describe '#exist?' do
it 'checks whether the key exists' do it 'checks whether the key exists' do
expect(cache.exist?(:foo)).to be(false) expect(cache.exist?(:foo)).to be(false)
......
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