Commit b72d2438 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Keep track of all storage keys in a set

That way we don't need the to scan the entire keyspace to get all
known keys
parent 4820a195
Pipeline #14628099 failed with stages
in 73 minutes and 45 seconds
---
title: Keep track of all circuitbreaker keys in a set
merge_request: 15613
author:
type: performance
......@@ -15,6 +15,7 @@ module Gitlab
Failing = Class.new(Inaccessible)
REDIS_KEY_PREFIX = 'storage_accessible:'.freeze
REDIS_KNOWN_KEYS = "#{REDIS_KEY_PREFIX}known_keys_set".freeze
def self.redis
Gitlab::Redis::SharedState
......
......@@ -13,10 +13,8 @@ module Gitlab
delegate :last_failure, :failure_count, to: :failure_info
def self.reset_all!
pattern = "#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}*"
Gitlab::Git::Storage.redis.with do |redis|
all_storage_keys = redis.scan_each(match: pattern).to_a
all_storage_keys = redis.zrange(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, -1)
redis.del(*all_storage_keys) unless all_storage_keys.empty?
end
......@@ -135,23 +133,29 @@ module Gitlab
redis.hset(cache_key, :last_failure, last_failure.to_i)
redis.hincrby(cache_key, :failure_count, 1)
redis.expire(cache_key, failure_reset_time)
maintain_known_keys(redis)
end
end
end
def track_storage_accessible
return if no_failures?
@failure_info = FailureInfo.new(nil, 0)
Gitlab::Git::Storage.redis.with do |redis|
redis.pipelined do
redis.hset(cache_key, :last_failure, nil)
redis.hset(cache_key, :failure_count, 0)
maintain_known_keys(redis)
end
end
end
def maintain_known_keys(redis)
expire_time = Time.now.to_i + failure_reset_time
redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, expire_time, cache_key)
redis.zremrangebyscore(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, '-inf', Time.now.to_i)
end
def get_failure_info
last_failure, failure_count = Gitlab::Git::Storage.redis.with do |redis|
redis.hmget(cache_key, :last_failure, :failure_count)
......
......@@ -4,8 +4,8 @@ module Gitlab
class Health
attr_reader :storage_name, :info
def self.pattern_for_storage(storage_name)
"#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}#{storage_name}:*"
def self.prefix_for_storage(storage_name)
"#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}#{storage_name}:"
end
def self.for_all_storages
......@@ -25,26 +25,15 @@ module Gitlab
private_class_method def self.all_keys_for_storages(storage_names, redis)
keys_per_storage = {}
all_keys = redis.zrange(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, -1)
redis.pipelined do
storage_names.each do |storage_name|
pattern = pattern_for_storage(storage_name)
matched_keys = redis.scan_each(match: pattern)
storage_names.each do |storage_name|
prefix = prefix_for_storage(storage_name)
keys_per_storage[storage_name] = matched_keys
end
keys_per_storage[storage_name] = all_keys.select { |key| key.starts_with?(prefix) }
end
# We need to make sure each lazy-loaded `Enumerator` for matched keys
# is loaded into an array.
#
# Otherwise it would be loaded in the second `Redis#pipelined` block
# within `.load_for_keys`. In this pipelined call, the active
# Redis-client changes again, so the values would not be available
# until the end of that pipelined-block.
keys_per_storage.each do |storage_name, key_future|
keys_per_storage[storage_name] = key_future.to_a
end
keys_per_storage
end
private_class_method def self.load_for_keys(keys_per_storage, redis)
......
......@@ -27,6 +27,7 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
def set_in_redis(name, value)
Gitlab::Git::Storage.redis.with do |redis|
redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, cache_key)
redis.hmset(cache_key, name, value)
end.first
end
......@@ -181,6 +182,24 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
expect(circuit_breaker.last_failure).to be_nil
end
it 'maintains known storage keys' do
Timecop.freeze do
# Insert an old key to expire
old_entry = Time.now.to_i - 3.days.to_i
Gitlab::Git::Storage.redis.with do |redis|
redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, old_entry, 'to_be_removed')
end
circuit_breaker.perform { '' }
known_keys = Gitlab::Git::Storage.redis.with do |redis|
redis.zrange(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, -1)
end
expect(known_keys).to contain_exactly(cache_key)
end
end
it 'only performs the accessibility check once' do
expect(Gitlab::Git::Storage::ForkedStorageCheck)
.to receive(:storage_available?).once.and_call_original
......
......@@ -6,6 +6,7 @@ describe Gitlab::Git::Storage::Health, clean_gitlab_redis_shared_state: true, br
def set_in_redis(cache_key, value)
Gitlab::Git::Storage.redis.with do |redis|
redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, cache_key)
redis.hmset(cache_key, :failure_count, value)
end.first
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