Commit df2ee099 authored by Robert May's avatar Robert May Committed by Peter Leitzen

Swap RepositoryHashCache to UNLINK

Improves the delete performance for RepositoryHashCache by
sending a single UNLINK call for multiple keys, rather than
multiple DEL calls for each key. This is the same implementation
as used in Gitlab::SetCache.
parent 2d896d3e
---
title: Swap RepositoryHashCache to UNLINK
merge_request: 39105
author:
type: performance
......@@ -241,7 +241,7 @@ module Gitlab
end
def expire_redis_hash_method_caches(methods)
methods.each { |name| redis_hash_cache.delete(name) }
redis_hash_cache.delete(*methods)
end
# All cached repository methods depend on the existence of a Git repository,
......
......@@ -31,10 +31,18 @@ module Gitlab
"#{type}:#{namespace}:hash"
end
# @param key [String]
# @return [Integer] 0 or 1 depending on success
def delete(key)
with { |redis| redis.del(cache_key(key)) }
# @param keys [String] one or multiple keys to delete
# @return [Integer] the number of keys successfully deleted
def delete(*keys)
return 0 if keys.empty?
with do |redis|
keys = keys.map { |key| cache_key(key) }
Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do
redis.unlink(*keys)
end
end
end
# Check if the provided hash key exists in the hash.
......
......@@ -212,8 +212,7 @@ RSpec.describe Gitlab::RepositoryCacheAdapter do
expect(cache).to receive(:expire).with(:rendered_readme)
expect(cache).to receive(:expire).with(:branch_names)
expect(redis_set_cache).to receive(:expire).with(:rendered_readme, :branch_names)
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(:rendered_readme, :branch_names)
repository.expire_method_caches(%i(rendered_readme branch_names))
end
......
......@@ -48,6 +48,24 @@ RSpec.describe Gitlab::RepositoryHashCache, :clean_gitlab_redis_cache do
context "key doesn't exist" do
it { is_expected.to eq(0) }
end
context "multiple keys" do
before do
cache.write(:test1, test_hash)
cache.write(:test2, test_hash)
end
it "deletes multiple keys" do
cache.delete(:test1, :test2)
expect(cache.read_members(:test1, ["test"])).to eq("test" => nil)
expect(cache.read_members(:test2, ["test"])).to eq("test" => nil)
end
it "returns deleted key count" do
expect(cache.delete(:test1, :test2)).to eq(2)
end
end
end
describe "#key?" do
......
......@@ -587,15 +587,19 @@ RSpec.describe Repository do
end
it "is expired when the branches caches are expired" do
expect(cache).to receive(:delete).with(:merged_branch_names).at_least(:once)
expect(cache).to receive(:delete) do |*args|
expect(args).to include(:merged_branch_names)
end
repository.send(:expire_branches_cache)
repository.expire_branches_cache
end
it "is expired when the repository caches are expired" do
expect(cache).to receive(:delete).with(:merged_branch_names).at_least(:once)
expect(cache).to receive(:delete) do |*args|
expect(args).to include(:merged_branch_names)
end
repository.send(:expire_all_method_caches)
repository.expire_all_method_caches
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