Commit 02111f4d authored by Douwe Maan's avatar Douwe Maan

Merge branch 'bvl-circuitbreaker-keys-set' into 'master'

Keep track of all storage keys in a set

See merge request gitlab-org/gitlab-ce!15613
parents 5ffbea9a b72d2438
---
title: Keep track of all circuitbreaker keys in a set
merge_request: 15613
author:
type: performance
...@@ -15,6 +15,7 @@ module Gitlab ...@@ -15,6 +15,7 @@ module Gitlab
Failing = Class.new(Inaccessible) Failing = Class.new(Inaccessible)
REDIS_KEY_PREFIX = 'storage_accessible:'.freeze REDIS_KEY_PREFIX = 'storage_accessible:'.freeze
REDIS_KNOWN_KEYS = "#{REDIS_KEY_PREFIX}known_keys_set".freeze
def self.redis def self.redis
Gitlab::Redis::SharedState Gitlab::Redis::SharedState
......
...@@ -13,10 +13,8 @@ module Gitlab ...@@ -13,10 +13,8 @@ module Gitlab
delegate :last_failure, :failure_count, to: :failure_info delegate :last_failure, :failure_count, to: :failure_info
def self.reset_all! def self.reset_all!
pattern = "#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}*"
Gitlab::Git::Storage.redis.with do |redis| 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? redis.del(*all_storage_keys) unless all_storage_keys.empty?
end end
...@@ -135,23 +133,29 @@ module Gitlab ...@@ -135,23 +133,29 @@ module Gitlab
redis.hset(cache_key, :last_failure, last_failure.to_i) redis.hset(cache_key, :last_failure, last_failure.to_i)
redis.hincrby(cache_key, :failure_count, 1) redis.hincrby(cache_key, :failure_count, 1)
redis.expire(cache_key, failure_reset_time) redis.expire(cache_key, failure_reset_time)
maintain_known_keys(redis)
end end
end end
end end
def track_storage_accessible def track_storage_accessible
return if no_failures?
@failure_info = FailureInfo.new(nil, 0) @failure_info = FailureInfo.new(nil, 0)
Gitlab::Git::Storage.redis.with do |redis| Gitlab::Git::Storage.redis.with do |redis|
redis.pipelined do redis.pipelined do
redis.hset(cache_key, :last_failure, nil) redis.hset(cache_key, :last_failure, nil)
redis.hset(cache_key, :failure_count, 0) redis.hset(cache_key, :failure_count, 0)
maintain_known_keys(redis)
end end
end 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 def get_failure_info
last_failure, failure_count = Gitlab::Git::Storage.redis.with do |redis| last_failure, failure_count = Gitlab::Git::Storage.redis.with do |redis|
redis.hmget(cache_key, :last_failure, :failure_count) redis.hmget(cache_key, :last_failure, :failure_count)
......
...@@ -4,8 +4,8 @@ module Gitlab ...@@ -4,8 +4,8 @@ module Gitlab
class Health class Health
attr_reader :storage_name, :info attr_reader :storage_name, :info
def self.pattern_for_storage(storage_name) def self.prefix_for_storage(storage_name)
"#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}#{storage_name}:*" "#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}#{storage_name}:"
end end
def self.for_all_storages def self.for_all_storages
...@@ -25,26 +25,15 @@ module Gitlab ...@@ -25,26 +25,15 @@ module Gitlab
private_class_method def self.all_keys_for_storages(storage_names, redis) private_class_method def self.all_keys_for_storages(storage_names, redis)
keys_per_storage = {} keys_per_storage = {}
all_keys = redis.zrange(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, -1)
redis.pipelined do storage_names.each do |storage_name|
storage_names.each do |storage_name| prefix = prefix_for_storage(storage_name)
pattern = pattern_for_storage(storage_name)
matched_keys = redis.scan_each(match: pattern)
keys_per_storage[storage_name] = matched_keys keys_per_storage[storage_name] = all_keys.select { |key| key.starts_with?(prefix) }
end
end end
# We need to make sure each lazy-loaded `Enumerator` for matched keys keys_per_storage
# 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
end end
private_class_method def self.load_for_keys(keys_per_storage, redis) 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: ...@@ -27,6 +27,7 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
def set_in_redis(name, value) def set_in_redis(name, value)
Gitlab::Git::Storage.redis.with do |redis| Gitlab::Git::Storage.redis.with do |redis|
redis.zadd(Gitlab::Git::Storage::REDIS_KNOWN_KEYS, 0, cache_key)
redis.hmset(cache_key, name, value) redis.hmset(cache_key, name, value)
end.first end.first
end end
...@@ -181,6 +182,24 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: ...@@ -181,6 +182,24 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
expect(circuit_breaker.last_failure).to be_nil expect(circuit_breaker.last_failure).to be_nil
end 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 it 'only performs the accessibility check once' do
expect(Gitlab::Git::Storage::ForkedStorageCheck) expect(Gitlab::Git::Storage::ForkedStorageCheck)
.to receive(:storage_available?).once.and_call_original .to receive(:storage_available?).once.and_call_original
......
...@@ -6,6 +6,7 @@ describe Gitlab::Git::Storage::Health, clean_gitlab_redis_shared_state: true, br ...@@ -6,6 +6,7 @@ describe Gitlab::Git::Storage::Health, clean_gitlab_redis_shared_state: true, br
def set_in_redis(cache_key, value) def set_in_redis(cache_key, value)
Gitlab::Git::Storage.redis.with do |redis| 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) redis.hmset(cache_key, :failure_count, value)
end.first end.first
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