Commit 7743828d authored by Shinya Maeda's avatar Shinya Maeda Committed by Kamil Trzciński

Validate chunk size when persist

parent 0f800a5c
...@@ -15,6 +15,8 @@ module Ci ...@@ -15,6 +15,8 @@ module Ci
WRITE_LOCK_SLEEP = 0.01.seconds WRITE_LOCK_SLEEP = 0.01.seconds
WRITE_LOCK_TTL = 1.minute WRITE_LOCK_TTL = 1.minute
FailedToPersistDataError = Class.new(StandardError)
# Note: The ordering of this enum is related to the precedence of persist store. # 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. # The bottom item takes the higest precedence, and the top item takes the lowest precedence.
enum data_store: { enum data_store: {
...@@ -109,15 +111,18 @@ module Ci ...@@ -109,15 +111,18 @@ module Ci
def unsafe_persist_to!(new_store) def unsafe_persist_to!(new_store)
return if data_store == new_store.to_s return if data_store == new_store.to_s
raise ArgumentError, 'Can not persist empty data' unless size > 0
current_data = get_data
unless current_data&.bytesize.to_i == CHUNK_SIZE
raise FailedToPersistDataError, 'Data is not fullfilled in a bucket'
end
old_store_class = self.class.get_store_class(data_store) old_store_class = self.class.get_store_class(data_store)
get_data.tap do |the_data|
self.raw_data = nil self.raw_data = nil
self.data_store = new_store self.data_store = new_store
unsafe_set_data!(the_data) unsafe_set_data!(current_data)
end
old_store_class.delete_data(self) old_store_class.delete_data(self)
end end
......
---
title: Validate chunk size when persist
merge_request: 23341
author:
type: fixed
...@@ -436,12 +436,13 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do ...@@ -436,12 +436,13 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
let(:data_store) { :redis } let(:data_store) { :redis }
context 'when data exists' do context 'when data exists' do
let(:data) { 'Sample data in redis' }
before do before do
build_trace_chunk.send(:unsafe_set_data!, data) build_trace_chunk.send(:unsafe_set_data!, data)
end end
context 'when data size reached CHUNK_SIZE' do
let(:data) { 'a' * described_class::CHUNK_SIZE }
it 'persists the data' do it 'persists the data' do
expect(build_trace_chunk.redis?).to be_truthy expect(build_trace_chunk.redis?).to be_truthy
expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to eq(data) expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to eq(data)
...@@ -459,9 +460,23 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do ...@@ -459,9 +460,23 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
it_behaves_like 'Atomic operation' it_behaves_like 'Atomic operation'
end end
context 'when data size has not reached CHUNK_SIZE' do
let(:data) { 'Sample data in redis' }
it 'does not persist the data and the orignal data is intact' do
expect { subject }.to raise_error(described_class::FailedToPersistDataError)
expect(build_trace_chunk.redis?).to be_truthy
expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to eq(data)
expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil
expect { Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk) }.to raise_error(Excon::Error::NotFound)
end
end
end
context 'when data does not exist' do context 'when data does not exist' do
it 'does not persist' do it 'does not persist' do
expect { subject }.to raise_error('Can not persist empty data') expect { subject }.to raise_error(described_class::FailedToPersistDataError)
end end
end end
end end
...@@ -470,12 +485,13 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do ...@@ -470,12 +485,13 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
let(:data_store) { :database } let(:data_store) { :database }
context 'when data exists' do context 'when data exists' do
let(:data) { 'Sample data in database' }
before do before do
build_trace_chunk.send(:unsafe_set_data!, data) build_trace_chunk.send(:unsafe_set_data!, data)
end end
context 'when data size reached CHUNK_SIZE' do
let(:data) { 'a' * described_class::CHUNK_SIZE }
it 'persists the data' do it 'persists the data' do
expect(build_trace_chunk.database?).to be_truthy expect(build_trace_chunk.database?).to be_truthy
expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil
...@@ -493,9 +509,23 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do ...@@ -493,9 +509,23 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
it_behaves_like 'Atomic operation' it_behaves_like 'Atomic operation'
end end
context 'when data size has not reached CHUNK_SIZE' do
let(:data) { 'Sample data in database' }
it 'does not persist the data and the orignal data is intact' do
expect { subject }.to raise_error(described_class::FailedToPersistDataError)
expect(build_trace_chunk.database?).to be_truthy
expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil
expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to eq(data)
expect { Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk) }.to raise_error(Excon::Error::NotFound)
end
end
end
context 'when data does not exist' do context 'when data does not exist' do
it 'does not persist' do it 'does not persist' do
expect { subject }.to raise_error('Can not persist empty data') expect { subject }.to raise_error(described_class::FailedToPersistDataError)
end end
end end
end end
...@@ -504,12 +534,13 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do ...@@ -504,12 +534,13 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
let(:data_store) { :fog } let(:data_store) { :fog }
context 'when data exists' do context 'when data exists' do
let(:data) { 'Sample data in fog' }
before do before do
build_trace_chunk.send(:unsafe_set_data!, data) build_trace_chunk.send(:unsafe_set_data!, data)
end end
context 'when data size reached CHUNK_SIZE' do
let(:data) { 'a' * described_class::CHUNK_SIZE }
it 'does not change data store' do it 'does not change data store' do
expect(build_trace_chunk.fog?).to be_truthy expect(build_trace_chunk.fog?).to be_truthy
expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil expect(Ci::BuildTraceChunks::Redis.new.data(build_trace_chunk)).to be_nil
...@@ -526,6 +557,15 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do ...@@ -526,6 +557,15 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
it_behaves_like 'Atomic operation' it_behaves_like 'Atomic operation'
end end
context 'when data size has not reached CHUNK_SIZE' do
let(:data) { 'Sample data in fog' }
it 'does not raise error' do
expect { subject }.not_to raise_error
end
end
end
end end
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