Only track accessibility once
This commit is contained in:
parent
022c38e63e
commit
0dd4c306ca
2 changed files with 46 additions and 11 deletions
|
@ -46,10 +46,6 @@ module Gitlab
|
||||||
def perform
|
def perform
|
||||||
return yield unless Feature.enabled?('git_storage_circuit_breaker')
|
return yield unless Feature.enabled?('git_storage_circuit_breaker')
|
||||||
|
|
||||||
if circuit_broken?
|
|
||||||
raise Gitlab::Git::Storage::CircuitOpen.new("Circuit for #{storage} open", failure_wait_time)
|
|
||||||
end
|
|
||||||
|
|
||||||
check_storage_accessible!
|
check_storage_accessible!
|
||||||
|
|
||||||
yield
|
yield
|
||||||
|
@ -70,14 +66,24 @@ module Gitlab
|
||||||
# When the storage appears not available, and the memoized value is `false`
|
# When the storage appears not available, and the memoized value is `false`
|
||||||
# we might want to try again.
|
# we might want to try again.
|
||||||
def storage_available?
|
def storage_available?
|
||||||
@storage_available ||= Gitlab::Git::Storage::ForkedStorageCheck.storage_available?(storage_path, storage_timeout)
|
return @storage_available if @storage_available
|
||||||
end
|
|
||||||
|
|
||||||
def check_storage_accessible!
|
if @storage_available = Gitlab::Git::Storage::ForkedStorageCheck
|
||||||
if storage_available?
|
.storage_available?(storage_path, storage_timeout)
|
||||||
track_storage_accessible
|
track_storage_accessible
|
||||||
else
|
else
|
||||||
track_storage_inaccessible
|
track_storage_inaccessible
|
||||||
|
end
|
||||||
|
|
||||||
|
@storage_available
|
||||||
|
end
|
||||||
|
|
||||||
|
def check_storage_accessible!
|
||||||
|
if circuit_broken?
|
||||||
|
raise Gitlab::Git::Storage::CircuitOpen.new("Circuit for #{storage} is broken", failure_wait_time)
|
||||||
|
end
|
||||||
|
|
||||||
|
unless storage_available?
|
||||||
raise Gitlab::Git::Storage::Inaccessible.new("#{storage} not accessible", failure_wait_time)
|
raise Gitlab::Git::Storage::Inaccessible.new("#{storage} not accessible", failure_wait_time)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -120,19 +120,48 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#check_storage_accessible!' do
|
describe "storage_available?" do
|
||||||
context 'when the storage is available' do
|
context 'when the storage is available' do
|
||||||
it 'tracks that the storage was accessible an raises the error' do
|
it 'tracks that the storage was accessible an raises the error' do
|
||||||
expect(circuit_breaker).to receive(:track_storage_accessible)
|
expect(circuit_breaker).to receive(:track_storage_accessible)
|
||||||
|
|
||||||
circuit_breaker.check_storage_accessible!
|
circuit_breaker.storage_available?
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'only performs the check once' do
|
||||||
|
expect(Gitlab::Git::Storage::ForkedStorageCheck)
|
||||||
|
.to receive(:storage_available?).once.and_call_original
|
||||||
|
|
||||||
|
2.times { circuit_breaker.storage_available? }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when storage is not available' do
|
||||||
|
let(:circuit_breaker) { described_class.new('broken') }
|
||||||
|
|
||||||
|
it 'tracks that the storage was inaccessible' do
|
||||||
|
expect(circuit_breaker).to receive(:track_storage_inaccessible)
|
||||||
|
|
||||||
|
circuit_breaker.storage_available?
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#check_storage_accessible!' do
|
||||||
|
it 'raises an exception with retry time when the circuit is open' do
|
||||||
|
allow(circuit_breaker).to receive(:circuit_broken?).and_return(true)
|
||||||
|
|
||||||
|
expect { circuit_breaker.check_storage_accessible! }
|
||||||
|
.to raise_error do |exception|
|
||||||
|
expect(exception).to be_kind_of(Gitlab::Git::Storage::CircuitOpen)
|
||||||
|
expect(exception.retry_after).to eq(30)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when the storage is not available' do
|
context 'when the storage is not available' do
|
||||||
let(:circuit_breaker) { described_class.new('broken') }
|
let(:circuit_breaker) { described_class.new('broken') }
|
||||||
|
|
||||||
it 'tracks that the storage was unavailable and raises an error with retry time' do
|
it 'raises an error' do
|
||||||
expect(circuit_breaker).to receive(:track_storage_inaccessible)
|
expect(circuit_breaker).to receive(:track_storage_inaccessible)
|
||||||
|
|
||||||
expect { circuit_breaker.check_storage_accessible! }
|
expect { circuit_breaker.check_storage_accessible! }
|
||||||
|
|
Loading…
Reference in a new issue