diff options
12 files changed, 427 insertions, 436 deletions
diff --git a/lib/gitlab/ci/trace/chunked_file/chunk_store/base.rb b/lib/gitlab/ci/trace/chunked_file/chunk_store/base.rb index e2645918a40..6e104a6d764 100644 --- a/lib/gitlab/ci/trace/chunked_file/chunk_store/base.rb +++ b/lib/gitlab/ci/trace/chunked_file/chunk_store/base.rb @@ -22,18 +22,32 @@ module Gitlab raise NotImplementedError end + # Write data to chunk store. Always overwrite. + # + # @param [String] data + # @return [Fixnum] length of the data after writing def write!(data) raise NotImplementedError end + # Append data to chunk store + # + # @param [String] data + # @return [Fixnum] length of the appended def append!(data) raise NotImplementedError end + # Truncate data to chunk store + # + # @param [String] offset def truncate!(offset) raise NotImplementedError end + # Delete data from chunk store + # + # @param [String] offset def delete! raise NotImplementedError end diff --git a/lib/gitlab/ci/trace/chunked_file/chunk_store/database.rb b/lib/gitlab/ci/trace/chunked_file/chunk_store/database.rb index a7db214f428..3c2805e83f7 100644 --- a/lib/gitlab/ci/trace/chunked_file/chunk_store/database.rb +++ b/lib/gitlab/ci/trace/chunked_file/chunk_store/database.rb @@ -48,6 +48,8 @@ module Gitlab end def get + puts "#{self.class.name} - #{__callee__}: params[:chunk_index]: #{params[:chunk_index]}" + job_trace_chunk.data end @@ -56,9 +58,10 @@ module Gitlab end def write!(data) + raise NotImplementedError, 'Partial writing is not supported' unless params[:buffer_size] == data&.length + raise NotImplementedError, 'UPDATE (Overwriting data) is not supported' if job_trace_chunk.data + puts "#{self.class.name} - #{__callee__}: data.length: #{data.length.inspect} params[:chunk_index]: #{params[:chunk_index]}" - raise NotImplementedError, 'Partial write is not supported' unless params[:buffer_size] == data&.length - raise NotImplementedError, 'UPDATE is not supported' if job_trace_chunk.data job_trace_chunk.data = data job_trace_chunk.save! @@ -75,7 +78,10 @@ module Gitlab end def delete! + raise ActiveRecord::RecordNotFound, 'Could not find deletable record' unless job_trace_chunk.persisted? + puts "#{self.class.name} - #{__callee__}: params[:chunk_index]: #{params[:chunk_index]}" + job_trace_chunk.destroy! end end diff --git a/lib/gitlab/ci/trace/chunked_file/chunk_store/redis.rb b/lib/gitlab/ci/trace/chunked_file/chunk_store/redis.rb index cb45cd5fba5..c87275319d9 100644 --- a/lib/gitlab/ci/trace/chunked_file/chunk_store/redis.rb +++ b/lib/gitlab/ci/trace/chunked_file/chunk_store/redis.rb @@ -51,6 +51,9 @@ module Gitlab end end + BufferKeyNotFoundError = Class.new(StandardError) + WriteError = Class.new(StandardError) + attr_reader :buffer_key def initialize(buffer_key, **params) @@ -64,6 +67,8 @@ module Gitlab end def get + puts "#{self.class.name} - #{__callee__}: params[:chunk_index]: #{params[:chunk_index]}" + Gitlab::Redis::Cache.with do |redis| redis.get(buffer_key) end @@ -76,35 +81,47 @@ module Gitlab end def write!(data) + raise ArgumentError, 'Could not write empty data' unless data.present? + puts "#{self.class.name} - #{__callee__}: data.length: #{data.length.inspect} params[:chunk_index]: #{params[:chunk_index]}" Gitlab::Redis::Cache.with do |redis| - redis.set(buffer_key, data) + unless redis.set(buffer_key, data) == 'OK' + raise WriteError, 'Failed to write' + end + redis.strlen(buffer_key) end end def append!(data) + raise ArgumentError, 'Could not write empty data' unless data.present? + puts "#{self.class.name} - #{__callee__}: data.length: #{data.length.inspect} params[:chunk_index]: #{params[:chunk_index]}" Gitlab::Redis::Cache.with do |redis| - redis.append(buffer_key, data) - data.length + raise BufferKeyNotFoundError, 'Buffer key is not found' unless redis.exists(buffer_key) + + original_size = size + new_size = redis.append(buffer_key, data) + appended_size = new_size - original_size + + raise WriteError, 'Failed to append' unless appended_size == data.length + + appended_size end end def truncate!(offset) - puts "#{self.class.name} - #{__callee__}: offset: #{offset.inspect} params[:chunk_index]: #{params[:chunk_index]}" - Gitlab::Redis::Cache.with do |redis| - return 0 unless redis.exists(buffer_key) - - truncated_data = redis.getrange(buffer_key, 0, offset) - redis.set(buffer_key, truncated_data) - end + raise NotImplementedError end def delete! puts "#{self.class.name} - #{__callee__}: params[:chunk_index]: #{params[:chunk_index]}" Gitlab::Redis::Cache.with do |redis| - redis.del(buffer_key) + raise BufferKeyNotFoundError, 'Buffer key is not found' unless redis.exists(buffer_key) + + unless redis.del(buffer_key) == 1 + raise WriteError, 'Failed to delete' + end end end end diff --git a/lib/gitlab/ci/trace/chunked_file/chunked_io.rb b/lib/gitlab/ci/trace/chunked_file/chunked_io.rb index 5a2c56a687b..f28955264c8 100644 --- a/lib/gitlab/ci/trace/chunked_file/chunked_io.rb +++ b/lib/gitlab/ci/trace/chunked_file/chunked_io.rb @@ -8,7 +8,7 @@ module Gitlab class Trace module ChunkedFile class ChunkedIO - extend ChunkedFile::Concerns::Opener + # extend ChunkedFile::Concerns::Opener include ChunkedFile::Concerns::Errors include ChunkedFile::Concerns::Hooks include ChunkedFile::Concerns::Callbacks @@ -22,13 +22,21 @@ module Gitlab alias_method :pos, :tell - def initialize(job_id, size, mode = 'rb') - @size = size + def initialize(job_id, size = nil, mode = 'rb', &block) + raise NotImplementedError, "Mode 'w' is not supported" if mode.include?('w') + + @size = size || calculate_size(job_id) @tell = 0 @job_id = job_id @mode = mode - raise NotImplementedError, "Mode 'w' is not supported" if mode.include?('w') + if block_given? + begin + yield self + ensure + self.close + end + end end def close @@ -128,7 +136,7 @@ module Gitlab end def present? - chunk_store.chunks_count(job_id) > 0 + chunks_count > 0 end def delete @@ -177,19 +185,21 @@ module Gitlab end def write_chunk(data) + written_size = 0 + chunk_store.open(job_id, chunk_index, params_for_store) do |store| with_callbacks(:write_chunk, store) do - written_size = if buffer_size == data.length + written_size = if buffer_size == data.length || store.size == 0 store.write!(data) else store.append!(data) end raise WriteError, 'Written size mismatch' unless data.length == written_size - - written_size end end + + written_size end def truncate_chunk(offset) @@ -228,7 +238,7 @@ module Gitlab end def chunks_count - (size / buffer_size) + (size / buffer_size.to_f).ceil end def first_chunk? @@ -246,6 +256,10 @@ module Gitlab def buffer_size raise NotImplementedError end + + def calculate_size(job_id) + chunk_store.chunks_size(job_id) + end end end end diff --git a/lib/gitlab/ci/trace/chunked_file/concerns/hooks.rb b/lib/gitlab/ci/trace/chunked_file/concerns/hooks.rb deleted file mode 100644 index 290a3a15805..00000000000 --- a/lib/gitlab/ci/trace/chunked_file/concerns/hooks.rb +++ /dev/null @@ -1,63 +0,0 @@ -module Gitlab - module Ci - class Trace - module ChunkedFile - module Concerns - module Hooks - extend ActiveSupport::Concern - - included do - class_attribute :_before_methods, :_after_methods, - :instance_writer => false - self._before_methods = Hash.new [] - self._after_methods = Hash.new [] - end - - class_methods do - def before_method(kind, callback) - self._before_methods = self._before_methods. - merge kind => _before_methods[kind] + [callback] - end - - def after_method(kind, callback) - self._after_methods = self._after_methods. - merge kind => _after_methods[kind] + [callback] - end - end - - def method_added(method_name) - return if self.class._before_methods.values.include?(method_name) - return if self.class._after_methods.values.include?(method_name) - return if hooked_methods.include?(method_name) - - add_hooks_to(method_name) - end - - private - - def hooked_methods - @hooked_methods ||= [] - end - - def add_hooks_to(method_name) - hooked_methods << method_name - - original_method = instance_method(method_name) - - # re-define the method, but notice how we reference the original - # method definition - define_method(method_name) do |*args, &block| - self.class._before_methods[method_name].each { |hook| method(hook).call } - - # now invoke the original method - original_method.bind(self).call(*args, &block).tap do - self.class._after_methods[method_name].each { |hook| method(hook).call } - end - end - end - end - end - end - end - end -end diff --git a/lib/gitlab/ci/trace/chunked_file/concerns/permissions.rb b/lib/gitlab/ci/trace/chunked_file/concerns/permissions.rb index f364237f07b..11d8fd0cdfc 100644 --- a/lib/gitlab/ci/trace/chunked_file/concerns/permissions.rb +++ b/lib/gitlab/ci/trace/chunked_file/concerns/permissions.rb @@ -6,28 +6,19 @@ module Gitlab module Permissions extend ActiveSupport::Concern + WRITABLE_MODE = %w[a] + READABLE_MODE = %w[r +] + included do PermissionError = Class.new(StandardError) attr_reader :write_lock_uuid - - # mode checks - before_method :read, :can_read! - before_method :readline, :can_read! - before_method :each_line, :can_read! - before_method :write, :can_write! - before_method :truncate, :can_write! - - # write_lock - before_method :write, :check_lock! - before_method :truncate, :check_lock! - before_method :delete, :check_lock! end def initialize(job_id, size, mode = 'rb') - if /(w|a)/ =~ mode + if WRITABLE_MODE.any? { |m| mode.include?(m) } @write_lock_uuid = Gitlab::ExclusiveLease - .new(write_lock_key, timeout: 1.hour.to_i).try_obtain + .new(write_lock_key(job_id), timeout: 1.hour.to_i).try_obtain raise PermissionError, 'Already opened by another process' unless write_lock_uuid end @@ -37,25 +28,63 @@ module Gitlab def close if write_lock_uuid - Gitlab::ExclusiveLease.cancel(write_lock_key, write_lock_uuid) + Gitlab::ExclusiveLease.cancel(write_lock_key(job_id), write_lock_uuid) end super end - def check_lock! - raise PermissionError, 'Could not modify the file without lock' unless write_lock_uuid + def read(*args) + can_read! + + super + end + + def readline(*args) + can_read! + + super + end + + def each_line(*args) + can_read! + + super end + def write(*args) + can_write! + + super + end + + def truncate(*args) + can_write! + + super + end + + def delete(*args) + can_write! + + super + end + + private + def can_read! - raise IOError, 'not opened for reading' unless /(r|+)/ =~ mode + unless READABLE_MODE.any? { |m| mode.include?(m) } + raise IOError, 'not opened for reading' + end end def can_write! - raise IOError, 'not opened for writing' unless /(w|a)/ =~ mode + unless WRITABLE_MODE.any? { |m| mode.include?(m) } + raise IOError, 'not opened for writing' + end end - def write_lock_key + def write_lock_key(job_id) "live_trace:operation:write:#{job_id}" end end diff --git a/lib/gitlab/ci/trace/chunked_file/live_trace.rb b/lib/gitlab/ci/trace/chunked_file/live_trace.rb index 6976686553f..fe2205144cb 100644 --- a/lib/gitlab/ci/trace/chunked_file/live_trace.rb +++ b/lib/gitlab/ci/trace/chunked_file/live_trace.rb @@ -12,10 +12,6 @@ module Gitlab after_callback :write_chunk, :stash_to_database - def initialize(job_id, mode) - super(job_id, calculate_size(job_id), mode) - end - def stash_to_database(store) # Once data is filled into redis, move the data to database if store.filled? diff --git a/spec/lib/gitlab/ci/trace/chunked_file/chunk_store/database_spec.rb b/spec/lib/gitlab/ci/trace/chunked_file/chunk_store/database_spec.rb index 74fb81d7a53..c84398ca481 100644 --- a/spec/lib/gitlab/ci/trace/chunked_file/chunk_store/database_spec.rb +++ b/spec/lib/gitlab/ci/trace/chunked_file/chunk_store/database_spec.rb @@ -1,13 +1,13 @@ require 'spec_helper' describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Database do + let(:job) { create(:ci_build) } let(:job_id) { job.id } let(:chunk_index) { 0 } let(:buffer_size) { 256 } let(:job_trace_chunk) { ::Ci::JobTraceChunk.new(job_id: job_id, chunk_index: chunk_index) } let(:params) { { buffer_size: buffer_size } } - let(:trace) { 'A' * buffer_size } - let(:job) { create(:ci_build) } + let(:data) { 'A' * buffer_size } describe '.open' do subject { described_class.open(job_id, chunk_index, params) } @@ -35,7 +35,7 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Database do context 'when job_trace_chunk exists' do before do - described_class.new(job_trace_chunk, params).write!(trace) + described_class.new(job_trace_chunk, params).write!(data) end it { is_expected.to be_truthy } @@ -51,17 +51,17 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Database do context 'when job_trace_chunk exists' do before do - described_class.new(job_trace_chunk, params).write!(trace) + described_class.new(job_trace_chunk, params).write!(data) end it { is_expected.to eq(1) } context 'when two chunks exists' do let(:job_trace_chunk_2) { ::Ci::JobTraceChunk.new(job_id: job_id, chunk_index: chunk_index + 1) } - let(:trace_2) { 'B' * buffer_size } + let(:data_2) { 'B' * buffer_size } before do - described_class.new(job_trace_chunk_2, params).write!(trace_2) + described_class.new(job_trace_chunk_2, params).write!(data_2) end it { is_expected.to eq(2) } @@ -78,18 +78,18 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Database do context 'when job_trace_chunk exists' do before do - described_class.new(job_trace_chunk, params).write!(trace) + described_class.new(job_trace_chunk, params).write!(data) end - it { is_expected.to eq(trace.length) } + it { is_expected.to eq(data.length) } context 'when two chunks exists' do let(:job_trace_chunk_2) { ::Ci::JobTraceChunk.new(job_id: job_id, chunk_index: chunk_index + 1) } - let(:trace_2) { 'B' * buffer_size } - let(:chunks_size) { trace.length + trace_2.length } + let(:data_2) { 'B' * buffer_size } + let(:chunks_size) { data.length + data_2.length } before do - described_class.new(job_trace_chunk_2, params).write!(trace_2) + described_class.new(job_trace_chunk_2, params).write!(data_2) end it { is_expected.to eq(chunks_size) } @@ -101,15 +101,48 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Database do end end + describe '.delete_all' do + subject { described_class.delete_all(job_id) } + + context 'when job_trace_chunk exists' do + before do + described_class.new(job_trace_chunk, params).write!(data) + end + + it 'deletes all' do + expect { subject }.to change { described_class.chunks_count(job_id) }.by(-1) + end + + context 'when two chunks exists' do + let(:job_trace_chunk_2) { ::Ci::JobTraceChunk.new(job_id: job_id, chunk_index: chunk_index + 1) } + let(:data_2) { 'B' * buffer_size } + + before do + described_class.new(job_trace_chunk_2, params).write!(data_2) + end + + it 'deletes all' do + expect { subject }.to change { described_class.chunks_count(job_id) }.by(-2) + end + end + end + + context 'when buffer_key does not exist' do + it 'deletes all' do + expect { subject }.not_to change { described_class.chunks_count(job_id) } + end + end + end + describe '#get' do subject { described_class.new(job_trace_chunk, params).get } context 'when job_trace_chunk exists' do before do - described_class.new(job_trace_chunk, params).write!(trace) + described_class.new(job_trace_chunk, params).write!(data) end - it { is_expected.to eq(trace) } + it { is_expected.to eq(data) } end context 'when job_trace_chunk does not exist' do @@ -122,10 +155,10 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Database do context 'when job_trace_chunk exists' do before do - described_class.new(job_trace_chunk, params).write!(trace) + described_class.new(job_trace_chunk, params).write!(data) end - it { is_expected.to eq(trace.length) } + it { is_expected.to eq(data.length) } end context 'when job_trace_chunk does not exist' do @@ -134,45 +167,39 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Database do end describe '#write!' do - subject { described_class.new(job_trace_chunk, params).write!(trace) } + subject { described_class.new(job_trace_chunk, params).write!(data) } context 'when job_trace_chunk exists' do before do - described_class.new(job_trace_chunk, params).write!(trace) + described_class.new(job_trace_chunk, params).write!(data) end - it { expect { subject }.to raise_error('UPDATE is not supported') } + it { expect { subject }.to raise_error('UPDATE (Overwriting data) is not supported') } end context 'when job_trace_chunk does not exist' do let(:expected_data) { ::Ci::JobTraceChunk.find_by(job_id: job_id, chunk_index: chunk_index).data } it 'writes' do - is_expected.to eq(trace.length) + is_expected.to eq(data.length) - expect(expected_data).to eq(trace) + expect(expected_data).to eq(data) end end context 'when data is nil' do - let(:trace) { nil } + let(:data) { nil } - it { expect { subject }.to raise_error('Partial write is not supported') } + it { expect { subject }.to raise_error('Partial writing is not supported') } end end - describe '#truncate!' do - subject { described_class.new(job_trace_chunk, params).truncate!(0) } - - it { expect { subject }.to raise_error(NotImplementedError) } - end - describe '#delete!' do subject { described_class.new(job_trace_chunk, params).delete! } context 'when job_trace_chunk exists' do before do - described_class.new(job_trace_chunk, params).write!(trace) + described_class.new(job_trace_chunk, params).write!(data) end it 'deletes' do @@ -187,14 +214,8 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Database do end context 'when job_trace_chunk does not exist' do - it 'deletes' do - expect(::Ci::JobTraceChunk.exists?(job_id: job_id, chunk_index: chunk_index)) - .to be_falsy - - subject - - expect(::Ci::JobTraceChunk.exists?(job_id: job_id, chunk_index: chunk_index)) - .to be_falsy + it 'raises an error' do + expect { subject }.to raise_error('Could not find deletable record') end end end diff --git a/spec/lib/gitlab/ci/trace/chunked_file/chunk_store/redis_spec.rb b/spec/lib/gitlab/ci/trace/chunked_file/chunk_store/redis_spec.rb index 83423ac2a33..f1fb64225c9 100644 --- a/spec/lib/gitlab/ci/trace/chunked_file/chunk_store/redis_spec.rb +++ b/spec/lib/gitlab/ci/trace/chunked_file/chunk_store/redis_spec.rb @@ -1,12 +1,13 @@ require 'spec_helper' describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_cache do - let(:job_id) { 1 } + let(:job) { create(:ci_build) } + let(:job_id) { job.id } let(:chunk_index) { 0 } let(:buffer_size) { 128.kilobytes } let(:buffer_key) { described_class.buffer_key(job_id, chunk_index) } let(:params) { { buffer_size: buffer_size } } - let(:trace) { 'Here is the trace' } + let(:data) { 'Here is the trace' } describe '.open' do subject { described_class.open(job_id, chunk_index, params) } @@ -34,7 +35,7 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_ context 'when buffer_key exists' do before do - described_class.new(buffer_key, params).write!(trace) + described_class.new(buffer_key, params).write!(data) end it { is_expected.to be_truthy } @@ -50,17 +51,17 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_ context 'when buffer_key exists' do before do - described_class.new(buffer_key, params).write!(trace) + described_class.new(buffer_key, params).write!(data) end it { is_expected.to eq(1) } context 'when two chunks exists' do let(:buffer_key_2) { described_class.buffer_key(job_id, chunk_index + 1) } - let(:trace_2) { 'Another trace' } + let(:data_2) { 'Another data' } before do - described_class.new(buffer_key_2, params).write!(trace_2) + described_class.new(buffer_key_2, params).write!(data_2) end it { is_expected.to eq(2) } @@ -77,18 +78,18 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_ context 'when buffer_key exists' do before do - described_class.new(buffer_key, params).write!(trace) + described_class.new(buffer_key, params).write!(data) end - it { is_expected.to eq(trace.length) } + it { is_expected.to eq(data.length) } context 'when two chunks exists' do let(:buffer_key_2) { described_class.buffer_key(job_id, chunk_index + 1) } - let(:trace_2) { 'Another trace' } - let(:chunks_size) { trace.length + trace_2.length } + let(:data_2) { 'Another data' } + let(:chunks_size) { data.length + data_2.length } before do - described_class.new(buffer_key_2, params).write!(trace_2) + described_class.new(buffer_key_2, params).write!(data_2) end it { is_expected.to eq(chunks_size) } @@ -100,6 +101,39 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_ end end + describe '.delete_all' do + subject { described_class.delete_all(job_id) } + + context 'when buffer_key exists' do + before do + described_class.new(buffer_key, params).write!(data) + end + + it 'deletes all' do + expect { subject }.to change { described_class.chunks_count(job_id) }.by(-1) + end + + context 'when two chunks exists' do + let(:buffer_key_2) { described_class.buffer_key(job_id, chunk_index + 1) } + let(:data_2) { 'Another data' } + + before do + described_class.new(buffer_key_2, params).write!(data_2) + end + + it 'deletes all' do + expect { subject }.to change { described_class.chunks_count(job_id) }.by(-2) + end + end + end + + context 'when buffer_key does not exist' do + it 'deletes all' do + expect { subject }.not_to change { described_class.chunks_count(job_id) } + end + end + end + describe '.buffer_key' do subject { described_class.buffer_key(job_id, chunk_index) } @@ -111,10 +145,10 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_ context 'when buffer_key exists' do before do - described_class.new(buffer_key, params).write!(trace) + described_class.new(buffer_key, params).write!(data) end - it { is_expected.to eq(trace) } + it { is_expected.to eq(data) } end context 'when buffer_key does not exist' do @@ -127,10 +161,10 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_ context 'when buffer_key exists' do before do - described_class.new(buffer_key, params).write!(trace) + described_class.new(buffer_key, params).write!(data) end - it { is_expected.to eq(trace.length) } + it { is_expected.to eq(data.length) } end context 'when buffer_key does not exist' do @@ -139,91 +173,72 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_ end describe '#write!' do - subject { described_class.new(buffer_key, params).write!(trace) } + subject { described_class.new(buffer_key, params).write!(data) } context 'when buffer_key exists' do before do - described_class.new(buffer_key, params).write!('Already data in the chunk') + described_class.new(buffer_key, params).write!('Already data in the data') end it 'overwrites' do - is_expected.to eq(trace.length) + is_expected.to eq(data.length) Gitlab::Redis::Cache.with do |redis| - expect(redis.get(buffer_key)).to eq(trace) + expect(redis.get(buffer_key)).to eq(data) end end end context 'when buffer_key does not exist' do it 'writes' do - is_expected.to eq(trace.length) + is_expected.to eq(data.length) Gitlab::Redis::Cache.with do |redis| - expect(redis.get(buffer_key)).to eq(trace) + expect(redis.get(buffer_key)).to eq(data) end end end context 'when data is nil' do - let(:trace) { nil } + let(:data) { nil } it 'clears value' do - is_expected.to eq(0) + expect { described_class.new(buffer_key, params).write!(data) } + .to raise_error('Could not write empty data') end end end - describe '#truncate!' do - subject { described_class.new(buffer_key, params).truncate!(offset) } - - let(:offset) { 5 } + describe '#append!' do + subject { described_class.new(buffer_key, params).append!(data) } context 'when buffer_key exists' do + let(:written_chunk) { 'Already data in the data' } + before do - described_class.new(buffer_key, params).write!(trace) + described_class.new(buffer_key, params).write!(written_chunk) end - it 'truncates' do - Gitlab::Redis::Cache.with do |redis| - expect(redis.get(buffer_key)).to eq(trace) - end - - subject + it 'appends' do + is_expected.to eq(data.length) Gitlab::Redis::Cache.with do |redis| - expect(redis.get(buffer_key)).to eq(trace.slice(0..offset)) - end - end - - context 'when offset is larger than data size' do - let(:offset) { 100 } - - it 'truncates' do - Gitlab::Redis::Cache.with do |redis| - expect(redis.get(buffer_key)).to eq(trace) - end - - subject - - Gitlab::Redis::Cache.with do |redis| - expect(redis.get(buffer_key)).to eq(trace.slice(0..offset)) - end + expect(redis.get(buffer_key)).to eq(written_chunk + data) end end end context 'when buffer_key does not exist' do - it 'truncates' do - Gitlab::Redis::Cache.with do |redis| - expect(redis.get(buffer_key)).to be_nil - end + it 'raises an error' do + expect { subject }.to raise_error(described_class::BufferKeyNotFoundError) + end + end - subject + context 'when data is nil' do + let(:data) { nil } - Gitlab::Redis::Cache.with do |redis| - expect(redis.get(buffer_key)).to be_nil - end + it 'raises an error' do + expect { subject }.to raise_error('Could not write empty data') end end end @@ -233,7 +248,7 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_ context 'when buffer_key exists' do before do - described_class.new(buffer_key, params).write!(trace) + described_class.new(buffer_key, params).write!(data) end it 'deletes' do @@ -250,16 +265,8 @@ describe Gitlab::Ci::Trace::ChunkedFile::ChunkStore::Redis, :clean_gitlab_redis_ end context 'when buffer_key does not exist' do - it 'deletes' do - Gitlab::Redis::Cache.with do |redis| - expect(redis.exists(buffer_key)).to be_falsy - end - - subject - - Gitlab::Redis::Cache.with do |redis| - expect(redis.exists(buffer_key)).to be_falsy - end + it 'raises an error' do + expect { subject }.to raise_error(described_class::BufferKeyNotFoundError) end end end diff --git a/spec/lib/gitlab/ci/trace/chunked_file/chunked_io_spec.rb b/spec/lib/gitlab/ci/trace/chunked_file/chunked_io_spec.rb index 048cad9f2e0..ffee27ca6e2 100644 --- a/spec/lib/gitlab/ci/trace/chunked_file/chunked_io_spec.rb +++ b/spec/lib/gitlab/ci/trace/chunked_file/chunked_io_spec.rb @@ -3,10 +3,9 @@ require 'spec_helper' describe Gitlab::Ci::Trace::ChunkedFile::ChunkedIO, :clean_gitlab_redis_cache do include ChunkedIOHelpers - let(:chunked_io) { described_class.new(job_id, size, mode) } + let(:chunked_io) { described_class.new(job_id, nil, mode) } let(:job) { create(:ci_build) } let(:job_id) { job.id } - let(:size) { sample_trace_size } let(:mode) { 'rb' } describe 'ChunkStore is Redis', :partial_support do diff --git a/spec/support/chunked_io/chunked_io_helpers.rb b/spec/support/chunked_io/chunked_io_helpers.rb index c12ad743c00..35b13bccdb4 100644 --- a/spec/support/chunked_io/chunked_io_helpers.rb +++ b/spec/support/chunked_io/chunked_io_helpers.rb @@ -1,6 +1,6 @@ module ChunkedIOHelpers def fill_trace_to_chunks(data) - stream = described_class.new(job_id, data.length, 'wb') + stream = described_class.new(job_id, nil, 'a+b') stream.write(data) stream.close end @@ -13,27 +13,23 @@ module ChunkedIOHelpers end end - def sample_trace_size - sample_trace_raw.length - end - - def sample_trace_raw_for_live_trace - File.read(expand_fixture_path('trace/sample_trace')) - end + # def sample_trace_raw_for_live_trace + # File.read(expand_fixture_path('trace/sample_trace')) + # end - def sample_trace_size_for_live_trace - sample_trace_raw_for_live_trace.length - end + # def sample_trace_size_for_live_trace + # sample_trace_raw_for_live_trace.length + # end - def fill_trace_to_chunks_for_live_trace(data) - stream = described_class.new(job_id, 'wb') - stream.write(data) - stream.close - end + # def fill_trace_to_chunks_for_live_trace(data) + # stream = described_class.new(job_id, 'a+b') + # stream.write(data) + # stream.close + # end - def stub_chunk_store_get_failed - allow_any_instance_of(chunk_store).to receive(:get).and_return(nil) - end + # def stub_chunk_store_get_failed + # allow_any_instance_of(chunk_store).to receive(:get).and_return(nil) + # end def set_smaller_buffer_size_than(file_size) blocks = (file_size / 128) diff --git a/spec/support/shared_examples/lib/gitlab/ci/trace/chunked_file/chunked_io_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/ci/trace/chunked_file/chunked_io_shared_examples.rb index 8bb3a6d8ff9..98199995182 100644 --- a/spec/support/shared_examples/lib/gitlab/ci/trace/chunked_file/chunked_io_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/ci/trace/chunked_file/chunked_io_shared_examples.rb @@ -8,53 +8,112 @@ shared_examples "ChunkedIO shared tests" do let(:mode) { 'rb' } it 'raises no exception' do - described_class.new(job_id, size, mode) - - expect { described_class.new(job_id, size, mode) }.not_to raise_error + expect { described_class.new(job_id, nil, mode) }.not_to raise_error + expect { described_class.new(job_id, nil, mode) }.not_to raise_error end end - context 'when mode is write' do + context 'when mode is append' do let(:mode) { 'a+b' } it 'raises an exception' do - described_class.new(job_id, size, mode) - - expect { described_class.new(job_id, size, mode) }.to raise_error('Already opened by another process') + expect { described_class.new(job_id, nil, mode) }.not_to raise_error + expect { described_class.new(job_id, nil, mode) }.to raise_error('Already opened by another process') end context 'when closed after open' do it 'does not raise an exception' do - described_class.new(job_id, size, mode).close - - expect { described_class.new(job_id, size, mode) }.not_to raise_error + expect { described_class.new(job_id, nil, mode).close }.not_to raise_error + expect { described_class.new(job_id, nil, mode) }.not_to raise_error end end end + + context 'when mode is write' do + let(:mode) { 'wb' } + + it 'raises no exception' do + expect { described_class.new(job_id, nil, mode) }.to raise_error("Mode 'w' is not supported") + end + end + end + + describe 'Permissions', :partial_support do + before do + fill_trace_to_chunks(sample_trace_raw) + end + + context "when mode is 'a+b'" do + let(:mode) { 'a+b' } + + it 'can write' do + expect { described_class.new(job_id, nil, mode).write('abc') } + .not_to raise_error + end + + it 'can read' do + expect { described_class.new(job_id, nil, mode).read(10) } + .not_to raise_error + end + end + + context "when mode is 'ab'" do + let(:mode) { 'ab' } + + it 'can write' do + expect { described_class.new(job_id, nil, mode).write('abc') } + .not_to raise_error + end + + it 'can not read' do + expect { described_class.new(job_id, nil, mode).read(10) } + .to raise_error('not opened for reading') + end + end + + context "when mode is 'rb'" do + let(:mode) { 'rb' } + + it 'can not write' do + expect { described_class.new(job_id, nil, mode).write('abc') } + .to raise_error('not opened for writing') + end + + it 'can read' do + expect { described_class.new(job_id, nil, mode).read(10) } + .not_to raise_error + end + end end describe '#seek' do subject { chunked_io.seek(pos, where) } + before do + set_smaller_buffer_size_than(sample_trace_raw.length) + fill_trace_to_chunks(sample_trace_raw) + end + context 'when moves pos to end of the file' do let(:pos) { 0 } let(:where) { IO::SEEK_END } - it { is_expected.to eq(size) } + it { is_expected.to eq(sample_trace_raw.length) } end context 'when moves pos to middle of the file' do - let(:pos) { size / 2 } + let(:pos) { sample_trace_raw.length / 2 } let(:where) { IO::SEEK_SET } - it { is_expected.to eq(size / 2) } + it { is_expected.to eq(pos) } end context 'when moves pos around' do it 'matches the result' do expect(chunked_io.seek(0)).to eq(0) expect(chunked_io.seek(100, IO::SEEK_CUR)).to eq(100) - expect { chunked_io.seek(size + 1, IO::SEEK_CUR) }.to raise_error('new position is outside of file') + expect { chunked_io.seek(sample_trace_raw.length + 1, IO::SEEK_CUR) } + .to raise_error('new position is outside of file') end end end @@ -62,9 +121,14 @@ shared_examples "ChunkedIO shared tests" do describe '#eof?' do subject { chunked_io.eof? } + before do + set_smaller_buffer_size_than(sample_trace_raw.length) + fill_trace_to_chunks(sample_trace_raw) + end + context 'when current pos is at end of the file' do before do - chunked_io.seek(size, IO::SEEK_SET) + chunked_io.seek(sample_trace_raw.length, IO::SEEK_SET) end it { is_expected.to be_truthy } @@ -84,39 +148,39 @@ shared_examples "ChunkedIO shared tests" do context 'when buffer size is smaller than file size' do before do - set_smaller_buffer_size_than(size) + set_smaller_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end it 'yields lines' do - expect { |b| described_class.new(job_id, size, 'rb').each_line(&b) } + expect { |b| described_class.new(job_id, nil, 'rb').each_line(&b) } .to yield_successive_args(*string_io.each_line.to_a) end end context 'when buffer size is larger than file size', :partial_support do before do - set_larger_buffer_size_than(size) + set_larger_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end it 'calls get_chunk only once' do expect(chunk_store).to receive(:open).once.and_call_original - described_class.new(job_id, size, 'rb').each_line { |line| } + described_class.new(job_id, nil, 'rb').each_line { |line| } end end end describe '#read' do - subject { described_class.new(job_id, size, 'rb').read(length) } + subject { described_class.new(job_id, nil, 'rb').read(length) } - context 'when read whole size' do + context 'when read the whole size' do let(:length) { nil } context 'when buffer size is smaller than file size' do before do - set_smaller_buffer_size_than(size) + set_smaller_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -127,7 +191,7 @@ shared_examples "ChunkedIO shared tests" do context 'when buffer size is larger than file size', :partial_support do before do - set_larger_buffer_size_than(size) + set_larger_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -142,7 +206,7 @@ shared_examples "ChunkedIO shared tests" do context 'when buffer size is smaller than file size' do before do - set_smaller_buffer_size_than(size) + set_smaller_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -153,7 +217,7 @@ shared_examples "ChunkedIO shared tests" do context 'when buffer size is larger than file size', :partial_support do before do - set_larger_buffer_size_than(size) + set_larger_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -164,11 +228,11 @@ shared_examples "ChunkedIO shared tests" do end context 'when tries to read oversize' do - let(:length) { size + 1000 } + let(:length) { sample_trace_raw.length + 1000 } context 'when buffer size is smaller than file size' do before do - set_smaller_buffer_size_than(size) + set_smaller_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -179,7 +243,7 @@ shared_examples "ChunkedIO shared tests" do context 'when buffer size is larger than file size', :partial_support do before do - set_larger_buffer_size_than(size) + set_larger_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -194,7 +258,7 @@ shared_examples "ChunkedIO shared tests" do context 'when buffer size is smaller than file size' do before do - set_smaller_buffer_size_than(size) + set_smaller_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -205,7 +269,7 @@ shared_examples "ChunkedIO shared tests" do context 'when buffer size is larger than file size', :partial_support do before do - set_larger_buffer_size_than(size) + set_larger_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -214,21 +278,6 @@ shared_examples "ChunkedIO shared tests" do end end end - - context 'when chunk store failed to get chunk' do - let(:length) { nil } - - before do - set_smaller_buffer_size_than(size) - fill_trace_to_chunks(sample_trace_raw) - - stub_chunk_store_get_failed - end - - it 'reads a trace' do - expect { subject }.to raise_error(described_class::FailedToGetChunkError) - end - end end describe '#readline' do @@ -244,23 +293,9 @@ shared_examples "ChunkedIO shared tests" do end end - context 'when chunk store failed to get chunk' do - let(:length) { nil } - - before do - set_smaller_buffer_size_than(size) - fill_trace_to_chunks(sample_trace_raw) - stub_chunk_store_get_failed - end - - it 'reads a trace' do - expect { subject }.to raise_error(described_class::FailedToGetChunkError) - end - end - context 'when buffer size is smaller than file size' do before do - set_smaller_buffer_size_than(size) + set_smaller_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -269,7 +304,7 @@ shared_examples "ChunkedIO shared tests" do context 'when buffer size is larger than file size', :partial_support do before do - set_larger_buffer_size_than(size) + set_larger_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) end @@ -278,11 +313,11 @@ shared_examples "ChunkedIO shared tests" do context 'when pos is at middle of the file' do before do - set_smaller_buffer_size_than(size) + set_smaller_buffer_size_than(sample_trace_raw.length) fill_trace_to_chunks(sample_trace_raw) - chunked_io.seek(size / 2) - string_io.seek(size / 2) + chunked_io.seek(chunked_io.size / 2) + string_io.seek(string_io.size / 2) end it 'reads from pos' do @@ -296,171 +331,91 @@ shared_examples "ChunkedIO shared tests" do let(:data) { sample_trace_raw } - context 'when write mode' do - let(:mode) { 'wb' } - - context 'when buffer size is smaller than file size' do - before do - set_smaller_buffer_size_than(size) - end - - it 'writes a trace' do - is_expected.to eq(data.length) + context 'when append mode', :partial_support do + let(:mode) { 'a+b' } - described_class.open(job_id, size, 'rb') do |stream| - expect(stream.read).to eq(data) - expect(chunk_store.chunks_count(job_id)).to eq(stream.send(:chunks_count)) - expect(chunk_store.chunks_size(job_id)).to eq(data.length) + context 'when data does not exist' do + context 'when buffer size is smaller than file size' do + before do + set_smaller_buffer_size_than(sample_trace_raw.length) end - end - end - - context 'when buffer size is larger than file size', :partial_support do - before do - set_larger_buffer_size_than(size) - end - it 'writes a trace' do - is_expected.to eq(data.length) + it 'writes a trace' do + is_expected.to eq(data.length) - described_class.open(job_id, size, 'rb') do |stream| - expect(stream.read).to eq(data) - expect(chunk_store.chunks_count(job_id)).to eq(stream.send(:chunks_count)) - expect(chunk_store.chunks_size(job_id)).to eq(data.length) + described_class.new(job_id, nil, 'rb') do |stream| + expect(stream.read).to eq(data) + expect(chunk_store.chunks_count(job_id)).to eq(stream.send(:chunks_count)) + expect(chunk_store.chunks_size(job_id)).to eq(data.length) + end end end - end - context 'when data is nil' do - let(:data) { nil } - - it 'writes a trace' do - expect { subject } .to raise_error('Could not write empty data') - end - end - end - - context 'when append mode', :partial_support do - let(:original_data) { 'original data' } - let(:total_size) { original_data.length + data.length } - - context 'when buffer size is smaller than file size' do - before do - set_smaller_buffer_size_than(size) - fill_trace_to_chunks(original_data) - end - - it 'appends a trace' do - described_class.open(job_id, original_data.length, 'a+b') do |stream| - expect(stream.write(data)).to eq(data.length) + context 'when buffer size is larger than file size', :partial_support do + before do + set_larger_buffer_size_than(data.length) end - described_class.open(job_id, total_size, 'rb') do |stream| - expect(stream.read).to eq(original_data + data) - expect(chunk_store.chunks_count(job_id)).to eq(stream.send(:chunks_count)) - expect(chunk_store.chunks_size(job_id)).to eq(total_size) - end - end - end + it 'writes a trace' do + is_expected.to eq(data.length) - context 'when buffer size is larger than file size' do - before do - set_larger_buffer_size_than(size) - fill_trace_to_chunks(original_data) + described_class.new(job_id, nil, 'rb') do |stream| + expect(stream.read).to eq(data) + expect(chunk_store.chunks_count(job_id)).to eq(stream.send(:chunks_count)) + expect(chunk_store.chunks_size(job_id)).to eq(data.length) + end + end end - it 'appends a trace' do - described_class.open(job_id, original_data.length, 'a+b') do |stream| - expect(stream.write(data)).to eq(data.length) - end + context 'when data is nil' do + let(:data) { nil } - described_class.open(job_id, total_size, 'rb') do |stream| - expect(stream.read).to eq(original_data + data) - expect(chunk_store.chunks_count(job_id)).to eq(stream.send(:chunks_count)) - expect(chunk_store.chunks_size(job_id)).to eq(total_size) + it 'writes a trace' do + expect { subject } .to raise_error('Could not write empty data') end end end - end - end - - describe '#truncate' do - context 'when data exists' do - context 'when buffer size is smaller than file size' do - before do - set_smaller_buffer_size_than(size) - fill_trace_to_chunks(sample_trace_raw) - end - it 'truncates a trace' do - described_class.open(job_id, size, 'rb') do |stream| - expect(stream.read).to eq(sample_trace_raw) - end + context 'when data already exists' do + let(:exist_data) { 'exist data' } + let(:total_size) { exist_data.length + data.length } - described_class.open(job_id, size, 'wb') do |stream| - stream.truncate(0) + context 'when buffer size is smaller than file size' do + before do + set_smaller_buffer_size_than(data.length) + fill_trace_to_chunks(exist_data) end - described_class.open(job_id, 0, 'rb') do |stream| - expect(stream.read).to be_empty - end - - expect(chunk_store.chunks_count(job_id)).to eq(0) - expect(chunk_store.chunks_size(job_id)).to eq(0) - end - - context 'when offset is negative' do - it 'raises an error' do - described_class.open(job_id, size, 'wb') do |stream| - expect { stream.truncate(-1) }.to raise_error('Offset is out of bound') + it 'appends a trace' do + described_class.new(job_id, nil, 'a+b') do |stream| + expect(stream.write(data)).to eq(data.length) end - end - end - context 'when offset is larger than file size' do - it 'raises an error' do - described_class.open(job_id, size, 'wb') do |stream| - expect { stream.truncate(size + 1) }.to raise_error('Offset is out of bound') + described_class.new(job_id, nil, 'rb') do |stream| + expect(stream.read).to eq(exist_data + data) + expect(chunk_store.chunks_count(job_id)).to eq(stream.send(:chunks_count)) + expect(chunk_store.chunks_size(job_id)).to eq(total_size) end end end - end - - context 'when buffer size is larger than file size', :partial_support do - before do - set_larger_buffer_size_than(size) - fill_trace_to_chunks(sample_trace_raw) - end - it 'truncates a trace' do - described_class.open(job_id, size, 'rb') do |stream| - expect(stream.read).to eq(sample_trace_raw) + context 'when buffer size is larger than file size' do + before do + set_larger_buffer_size_than(data.length) + fill_trace_to_chunks(exist_data) end - described_class.open(job_id, size, 'wb') do |stream| - stream.truncate(0) - end + it 'appends a trace' do + described_class.new(job_id, nil, 'a+b') do |stream| + expect(stream.write(data)).to eq(data.length) + end - described_class.open(job_id, 0, 'rb') do |stream| - expect(stream.read).to be_empty + described_class.new(job_id, nil, 'rb') do |stream| + expect(stream.read).to eq(exist_data + data) + expect(chunk_store.chunks_count(job_id)).to eq(stream.send(:chunks_count)) + expect(chunk_store.chunks_size(job_id)).to eq(total_size) + end end - - expect(chunk_store.chunks_count(job_id)).to eq(0) - expect(chunk_store.chunks_size(job_id)).to eq(0) - end - end - end - - context 'when data does not exist' do - before do - set_smaller_buffer_size_than(size) - end - - it 'truncates a trace' do - described_class.open(job_id, size, 'wb') do |stream| - stream.truncate(0) - expect(stream.send(:tell)).to eq(0) - expect(stream.send(:size)).to eq(0) end end end |