Commit 2a6c55af authored by Grzegorz Bizon's avatar Grzegorz Bizon

Merge branch 'sh-build-trace-chunk-store-thread-safety' into 'master'

Improve thread safety of Ci::BuildTraceChunk data stores

See merge request gitlab-org/gitlab!46717
parents 302eef2b 7e9ea39c
......@@ -22,20 +22,26 @@ module Ci
FailedToPersistDataError = Class.new(StandardError)
# Note: The ordering of this enum is related to the precedence of persist store.
# Note: The ordering of this hash is related to the precedence of persist store.
# The bottom item takes the highest precedence, and the top item takes the lowest precedence.
enum data_store: {
DATA_STORES = {
redis: 1,
database: 2,
fog: 3
}
}.freeze
STORE_TYPES = DATA_STORES.keys.map do |store|
[store, "Ci::BuildTraceChunks::#{store.capitalize}".constantize]
end.to_h.freeze
enum data_store: DATA_STORES
scope :live, -> { redis }
scope :persisted, -> { not_redis.order(:chunk_index) }
class << self
def all_stores
@all_stores ||= self.data_stores.keys
STORE_TYPES.keys
end
def persistable_store
......@@ -44,12 +50,14 @@ module Ci
end
def get_store_class(store)
@stores ||= {}
store = store.to_sym
raise "Unknown store type: #{store}" unless STORE_TYPES.key?(store)
# Can't memoize this because the feature flag may alter this
return fog_store_class.new if store.to_sym == :fog
return fog_store_class.new if store == :fog
@stores[store] ||= "Ci::BuildTraceChunks::#{store.capitalize}".constantize.new
STORE_TYPES[store].new
end
##
......
---
title: Improve thread safety of Ci::BuildTraceChunk data stores
merge_request: 46717
author:
type: fixed
......@@ -100,15 +100,15 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
subject { described_class.all_stores }
it 'returns a correctly ordered array' do
is_expected.to eq(%w[redis database fog])
is_expected.to eq(%i[redis database fog])
end
it 'returns redis store as the lowest precedence' do
expect(subject.first).to eq('redis')
expect(subject.first).to eq(:redis)
end
it 'returns fog store as the highest precedence' do
expect(subject.last).to eq('fog')
expect(subject.last).to eq(:fog)
end
end
......@@ -163,6 +163,30 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
end
end
describe '#get_store_class' do
using RSpec::Parameterized::TableSyntax
where(:data_store, :expected_store) do
:redis | Ci::BuildTraceChunks::Redis
:database | Ci::BuildTraceChunks::Database
:fog | Ci::BuildTraceChunks::Fog
end
with_them do
context "with store" do
it 'returns an instance of the right class' do
expect(expected_store).to receive(:new).twice.and_call_original
expect(described_class.get_store_class(data_store.to_s)).to be_a(expected_store)
expect(described_class.get_store_class(data_store.to_sym)).to be_a(expected_store)
end
end
end
it 'raises an error' do
expect { described_class.get_store_class('unknown') }.to raise_error('Unknown store type: unknown')
end
end
describe '#append' do
subject { build_trace_chunk.append(new_data, offset) }
......
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