diff options
| -rw-r--r-- | app/models/ci/build.rb | 2 | ||||
| -rw-r--r-- | app/models/ci/build_trace_chunk.rb | 146 | ||||
| -rw-r--r-- | app/models/ci/build_trace_chunks/database.rb | 29 | ||||
| -rw-r--r-- | app/models/ci/build_trace_chunks/fog.rb | 59 | ||||
| -rw-r--r-- | app/models/ci/build_trace_chunks/redis.rb | 51 | ||||
| -rw-r--r-- | app/services/concerns/exclusive_lease_lock.rb | 21 | ||||
| -rw-r--r-- | app/workers/ci/build_trace_chunk_flush_worker.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/build-chunks-on-object-storage.yml | 6 | ||||
| -rw-r--r-- | spec/models/ci/build_trace_chunk_spec.rb | 4 |
9 files changed, 234 insertions, 86 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 41446946a5e..8c90232405e 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -385,7 +385,7 @@ module Ci end def erase_old_trace! - update_column(:trace, nil) + update_column(:trace, nil) if old_trace end def needs_touch? diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb index 4856f10846c..179c5830678 100644 --- a/app/models/ci/build_trace_chunk.rb +++ b/app/models/ci/build_trace_chunk.rb @@ -1,6 +1,7 @@ module Ci class BuildTraceChunk < ActiveRecord::Base include FastDestroyAll + include ExclusiveLeaseLock extend Gitlab::Ci::Model belongs_to :build, class_name: "Ci::Build", foreign_key: :build_id @@ -10,45 +11,50 @@ module Ci WriteError = Class.new(StandardError) CHUNK_SIZE = 128.kilobytes - CHUNK_REDIS_TTL = 1.week WRITE_LOCK_RETRY = 10 WRITE_LOCK_SLEEP = 0.01.seconds WRITE_LOCK_TTL = 1.minute + # Note: The ordering of this enum is related to the precedence of persist store. + # The bottom item takes the higest precedence, and the top item takes the lowest precedence. enum data_store: { redis: 1, - db: 2 + database: 2, + fog: 3 } class << self - def redis_data_key(build_id, chunk_index) - "gitlab:ci:trace:#{build_id}:chunks:#{chunk_index}" + def all_stores + @all_stores ||= self.data_stores.keys end - def redis_data_keys - redis.pluck(:build_id, :chunk_index).map do |data| - redis_data_key(data.first, data.second) - end + def persist_store + # get first available store from the back of the list + all_stores.reverse.find { |store| get_store_class(store).available? } end - def redis_delete_data(keys) - return if keys.empty? - - Gitlab::Redis::SharedState.with do |redis| - redis.del(keys) - end + def get_store_class(store) + @stores ||= {} + @stores[store] ||= "Ci::BuildTraceChunks::#{store.capitalize}".constantize.new end ## # FastDestroyAll concerns def begin_fast_destroy - redis_data_keys + all_stores.each_with_object({}) do |store, result| + relation = public_send(store) # rubocop:disable GitlabSecurity/PublicSend + keys = get_store_class(store).keys(relation) + + result[store] = keys if keys.present? + end end ## # FastDestroyAll concerns def finalize_fast_destroy(keys) - redis_delete_data(keys) + keys.each do |store, value| + get_store_class(store).delete_keys(value) + end end end @@ -69,7 +75,13 @@ module Ci raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0 raise ArgumentError, 'Chunk size overflow' if CHUNK_SIZE < (offset + new_data.bytesize) - set_data(data.byteslice(0, offset) + new_data) + in_lock(*lock_params) do + self.reload if self.persisted? + + unsafe_set_data!(data.byteslice(0, offset) + new_data) + end + + schedule_to_persist if full? end def size @@ -88,50 +100,50 @@ module Ci (start_offset...end_offset) end - def use_database! - in_lock do - break if db? - break unless size > 0 + def persisted? + !redis? + end + + def persist! + in_lock(*lock_params) do + self.reload if self.persisted? - self.update!(raw_data: data, data_store: :db) - self.class.redis_delete_data([redis_data_key]) + unsafe_move_to!(self.class.persist_store) end end private + def unsafe_move_to!(new_store) + return if data_store == new_store.to_s + return unless size > 0 + + old_store_class = self.class.get_store_class(data_store) + + get_data.tap do |the_data| + self.raw_data = nil + self.data_store = new_store + unsafe_set_data!(the_data) + end + + old_store_class.delete_data(self) + end + def get_data - if redis? - redis_data - elsif db? - raw_data - else - raise 'Unsupported data store' - end&.force_encoding(Encoding::BINARY) # Redis/Database return UTF-8 string as default + self.class.get_store_class(data_store).data(self)&.force_encoding(Encoding::BINARY) # Redis/Database return UTF-8 string as default end - def set_data(value) + def unsafe_set_data!(value) raise ArgumentError, 'too much data' if value.bytesize > CHUNK_SIZE - in_lock do - if redis? - redis_set_data(value) - elsif db? - self.raw_data = value - else - raise 'Unsupported data store' - end - - @data = value + self.class.get_store_class(data_store).set_data(self, value) + @data = value - save! if changed? - end - - schedule_to_db if full? + save! if changed? end - def schedule_to_db - return if db? + def schedule_to_persist + return if persisted? Ci::BuildTraceChunkFlushWorker.perform_async(id) end @@ -140,41 +152,11 @@ module Ci size == CHUNK_SIZE end - def redis_data - Gitlab::Redis::SharedState.with do |redis| - redis.get(redis_data_key) - end - end - - def redis_set_data(data) - Gitlab::Redis::SharedState.with do |redis| - redis.set(redis_data_key, data, ex: CHUNK_REDIS_TTL) - end - end - - def redis_data_key - self.class.redis_data_key(build_id, chunk_index) - end - - def in_lock - write_lock_key = "trace_write:#{build_id}:chunks:#{chunk_index}" - - lease = Gitlab::ExclusiveLease.new(write_lock_key, timeout: WRITE_LOCK_TTL) - retry_count = 0 - - until uuid = lease.try_obtain - # Keep trying until we obtain the lease. To prevent hammering Redis too - # much we'll wait for a bit between retries. - sleep(WRITE_LOCK_SLEEP) - break if WRITE_LOCK_RETRY < (retry_count += 1) - end - - raise WriteError, 'Failed to obtain write lock' unless uuid - - self.reload if self.persisted? - return yield - ensure - Gitlab::ExclusiveLease.cancel(write_lock_key, uuid) + def lock_params + ["trace_write:#{build_id}:chunks:#{chunk_index}", + { ttl: WRITE_LOCK_TTL, + retry_max: WRITE_LOCK_RETRY, + sleep_sec: WRITE_LOCK_SLEEP }] end end end diff --git a/app/models/ci/build_trace_chunks/database.rb b/app/models/ci/build_trace_chunks/database.rb new file mode 100644 index 00000000000..3666d77c790 --- /dev/null +++ b/app/models/ci/build_trace_chunks/database.rb @@ -0,0 +1,29 @@ +module Ci + module BuildTraceChunks + class Database + def available? + true + end + + def keys(relation) + [] + end + + def delete_keys(keys) + # no-op + end + + def data(model) + model.raw_data + end + + def set_data(model, data) + model.raw_data = data + end + + def delete_data(model) + model.update_columns(raw_data: nil) unless model.raw_data.nil? + end + end + end +end diff --git a/app/models/ci/build_trace_chunks/fog.rb b/app/models/ci/build_trace_chunks/fog.rb new file mode 100644 index 00000000000..7506c40a39d --- /dev/null +++ b/app/models/ci/build_trace_chunks/fog.rb @@ -0,0 +1,59 @@ +module Ci + module BuildTraceChunks + class Fog + def available? + object_store.enabled + end + + def data(model) + connection.get_object(bucket_name, key(model))[:body] + end + + def set_data(model, data) + connection.put_object(bucket_name, key(model), data) + end + + def delete_data(model) + delete_keys([[model.build_id, model.chunk_index]]) + end + + def keys(relation) + return [] unless available? + + relation.pluck(:build_id, :chunk_index) + end + + def delete_keys(keys) + keys.each do |key| + connection.delete_object(bucket_name, key_raw(*key)) + end + end + + private + + def key(model) + key_raw(model.build_id, model.chunk_index) + end + + def key_raw(build_id, chunk_index) + "tmp/builds/#{build_id.to_i}/chunks/#{chunk_index.to_i}.log" + end + + def bucket_name + return unless available? + + object_store.remote_directory + end + + def connection + return unless available? + + @connection ||= ::Fog::Storage.new(object_store.connection.to_hash.deep_symbolize_keys) + end + + def object_store + Gitlab.config.artifacts.object_store + end + end + end +end diff --git a/app/models/ci/build_trace_chunks/redis.rb b/app/models/ci/build_trace_chunks/redis.rb new file mode 100644 index 00000000000..fdb6065e2a0 --- /dev/null +++ b/app/models/ci/build_trace_chunks/redis.rb @@ -0,0 +1,51 @@ +module Ci + module BuildTraceChunks + class Redis + CHUNK_REDIS_TTL = 1.week + + def available? + true + end + + def data(model) + Gitlab::Redis::SharedState.with do |redis| + redis.get(key(model)) + end + end + + def set_data(model, data) + Gitlab::Redis::SharedState.with do |redis| + redis.set(key(model), data, ex: CHUNK_REDIS_TTL) + end + end + + def delete_data(model) + delete_keys([[model.build_id, model.chunk_index]]) + end + + def keys(relation) + relation.pluck(:build_id, :chunk_index) + end + + def delete_keys(keys) + return if keys.empty? + + keys = keys.map { |key| key_raw(*key) } + + Gitlab::Redis::SharedState.with do |redis| + redis.del(keys) + end + end + + private + + def key(model) + key_raw(model.build_id, model.chunk_index) + end + + def key_raw(build_id, chunk_index) + "gitlab:ci:trace:#{build_id.to_i}:chunks:#{chunk_index.to_i}" + end + end + end +end diff --git a/app/services/concerns/exclusive_lease_lock.rb b/app/services/concerns/exclusive_lease_lock.rb new file mode 100644 index 00000000000..6c8bc25ea16 --- /dev/null +++ b/app/services/concerns/exclusive_lease_lock.rb @@ -0,0 +1,21 @@ +module ExclusiveLeaseLock + extend ActiveSupport::Concern + + def in_lock(key, ttl: 1.minute, retry_max: 10, sleep_sec: 0.01.seconds) + lease = Gitlab::ExclusiveLease.new(key, timeout: ttl) + retry_count = 0 + + until uuid = lease.try_obtain + # Keep trying until we obtain the lease. To prevent hammering Redis too + # much we'll wait for a bit. + sleep(sleep_sec) + break if retry_max < (retry_count += 1) + end + + raise WriteError, 'Failed to obtain write lock' unless uuid + + return yield + ensure + Gitlab::ExclusiveLease.cancel(key, uuid) + end +end diff --git a/app/workers/ci/build_trace_chunk_flush_worker.rb b/app/workers/ci/build_trace_chunk_flush_worker.rb index 218d6688bd9..8e08ccbc414 100644 --- a/app/workers/ci/build_trace_chunk_flush_worker.rb +++ b/app/workers/ci/build_trace_chunk_flush_worker.rb @@ -5,7 +5,7 @@ module Ci def perform(build_trace_chunk_id) ::Ci::BuildTraceChunk.find_by(id: build_trace_chunk_id).try do |build_trace_chunk| - build_trace_chunk.use_database! + build_trace_chunk.persist! end end end diff --git a/changelogs/unreleased/build-chunks-on-object-storage.yml b/changelogs/unreleased/build-chunks-on-object-storage.yml new file mode 100644 index 00000000000..9f36dfee378 --- /dev/null +++ b/changelogs/unreleased/build-chunks-on-object-storage.yml @@ -0,0 +1,6 @@ +--- +title: Use object storage as the first class persistable store for new live trace + architecture +merge_request: 19515 +author: +type: changed diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index b5a6d959ccb..f0c36067c87 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -286,8 +286,8 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end - describe '#use_database!' do - subject { build_trace_chunk.use_database! } + describe '#persist!' do + subject { build_trace_chunk.persist! } context 'when data_store is redis' do let(:data_store) { :redis } |