From c72ba43c55708e3878cf741738068468a88eec55 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 5 Apr 2022 15:01:00 +0200 Subject: [PATCH 1/2] feat(ArchiveUploader.upload_with_chunking_wrapper): expect to retry call to wrapper once --- spec/services/archive_uploader_spec.rb | 27 ++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/spec/services/archive_uploader_spec.rb b/spec/services/archive_uploader_spec.rb index c4b4bcfac..c529129ee 100644 --- a/spec/services/archive_uploader_spec.rb +++ b/spec/services/archive_uploader_spec.rb @@ -57,14 +57,33 @@ describe ProcedureArchiveService do let(:fake_blob_bytesize) { 100.gigabytes } before do - expect(uploader).to receive(:syscall_to_custom_uploader).and_return(true) expect(File).to receive(:size).with(file.path).and_return(fake_blob_bytesize) expect(Digest::SHA256).to receive(:file).with(file.path).and_return(double(hexdigest: fake_blob_checksum.hexdigest)) end - it 'creates a blob' do - expect { uploader.send(:upload_with_chunking_wrapper) } - .to change { ActiveStorage::Blob.where(checksum: fake_blob_checksum.hexdigest, byte_size: fake_blob_bytesize).count }.by(1) + context 'when it just works' do + it 'creates a blob' do + expect(uploader).to receive(:syscall_to_custom_uploader).and_return(true) + expect { uploader.send(:upload_with_chunking_wrapper) } + .to change { ActiveStorage::Blob.where(checksum: fake_blob_checksum.hexdigest, byte_size: fake_blob_bytesize).count }.by(1) + end + end + + context 'when it fails once (DS proxy a bit flacky with archive ±>20Go, fails once, accept other call' do + it 'retries' do + expect(uploader).to receive(:syscall_to_custom_uploader).with(anything).once.and_raise(StandardError, "BOOM") + expect(uploader).to receive(:syscall_to_custom_uploader).with(anything).once.and_return(true) + expect { uploader.send(:upload_with_chunking_wrapper) } + .to change { ActiveStorage::Blob.where(checksum: fake_blob_checksum.hexdigest, byte_size: fake_blob_bytesize).count }.by(1) + end + end + + context 'when it fails twice' do + it 'does not retry more than once' do + expect(uploader).to receive(:syscall_to_custom_uploader).with(anything).twice.and_raise(StandardError, "BOOM") + expect { uploader.send(:upload_with_chunking_wrapper) } + .to raise_error(StandardError, "BOOM") + end end end end From 9e8807d12ae44ceafb4a4a958c8aac128aa7793d Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 5 Apr 2022 15:05:22 +0200 Subject: [PATCH 2/2] feat(ArchiveUploader.upload_with_chunking_wrapper): retry once on error --- app/services/archive_uploader.rb | 16 ++++++++++++++-- spec/services/archive_uploader_spec.rb | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/services/archive_uploader.rb b/app/services/archive_uploader.rb index 1fb039600..c3dd7581f 100644 --- a/app/services/archive_uploader.rb +++ b/app/services/archive_uploader.rb @@ -48,11 +48,11 @@ class ArchiveUploader params = blob_default_params(filepath).merge(byte_size: File.size(filepath), checksum: Digest::SHA256.file(filepath).hexdigest) blob = ActiveStorage::Blob.create_before_direct_upload!(**params) - if syscall_to_custom_uploader(blob) + if retryable_syscall_to_custom_uploader(blob) return blob else blob.purge - fail "custom archive attachment failed, should it be retried ?" + fail "custom archive attachment failed twice, retry later" end end @@ -73,6 +73,18 @@ class ArchiveUploader @namespaced_object_key ||= "archives/#{Date.today.strftime("%Y-%m-%d")}/#{SecureRandom.uuid}" end + def retryable_syscall_to_custom_uploader(blob) + limit_to_retry = 1 + begin + syscall_to_custom_uploader(blob) + rescue + if limit_to_retry > 0 + limit_to_retry = limit_to_retry - 1 + retry + end + end + end + def syscall_to_custom_uploader(blob) system(ENV.fetch('ACTIVE_STORAGE_BIG_FILE_UPLOADER_WITH_ENCRYPTION_PATH').to_s, filepath, blob.key, exception: true) end diff --git a/spec/services/archive_uploader_spec.rb b/spec/services/archive_uploader_spec.rb index c529129ee..eb6a41f0b 100644 --- a/spec/services/archive_uploader_spec.rb +++ b/spec/services/archive_uploader_spec.rb @@ -82,7 +82,7 @@ describe ProcedureArchiveService do it 'does not retry more than once' do expect(uploader).to receive(:syscall_to_custom_uploader).with(anything).twice.and_raise(StandardError, "BOOM") expect { uploader.send(:upload_with_chunking_wrapper) } - .to raise_error(StandardError, "BOOM") + .to raise_error(RuntimeError, "custom archive attachment failed twice, retry later") end end end