Commit 14dab931 authored by Valery Sizov's avatar Valery Sizov

Geo: Handle orphaned Uploads records

parent 525cd053
---
title: 'Geo: Handle orphaned Uploads records'
merge_request: 8054
author:
type: fixed
...@@ -23,6 +23,7 @@ module Gitlab ...@@ -23,6 +23,7 @@ module Gitlab
def execute def execute
upload = Upload.find_by(id: object_db_id) upload = Upload.find_by(id: object_db_id)
return fail_before_transfer unless upload.present? return fail_before_transfer unless upload.present?
return missing_on_primary if upload.model.nil?
transfer = ::Gitlab::Geo::FileTransfer.new(object_type.to_sym, upload) transfer = ::Gitlab::Geo::FileTransfer.new(object_type.to_sym, upload)
Result.from_transfer_result(transfer.download_from_primary) Result.from_transfer_result(transfer.download_from_primary)
...@@ -51,6 +52,10 @@ module Gitlab ...@@ -51,6 +52,10 @@ module Gitlab
def fail_before_transfer def fail_before_transfer
Result.new(success: false, bytes_downloaded: 0, failed_before_transfer: true) Result.new(success: false, bytes_downloaded: 0, failed_before_transfer: true)
end end
def missing_on_primary
Result.new(success: true, bytes_downloaded: 0, primary_missing_file: true)
end
end end
end end
end end
...@@ -11,6 +11,28 @@ describe Geo::FileDownloadService do ...@@ -11,6 +11,28 @@ describe Geo::FileDownloadService do
stub_current_geo_node(secondary) stub_current_geo_node(secondary)
end end
shared_examples_for 'a service that handles orphaned uploads' do |file_type|
let(:download_service) { described_class.new(file_type, file.id) }
let(:registry) { Geo::FileRegistry }
before do
stub_exclusive_lease("file_download_service:#{file_type}:#{file.id}",
timeout: Geo::FileDownloadService::LEASE_TIMEOUT)
file.update_column(:model_id, 22222) # Not-existing record
end
it 'marks upload as successful and missing_on_primary' do
expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including(:message,
:download_time_s,
download_success: true,
bytes_downloaded: 0,
primary_missing_file: true)).and_call_original
expect { download_service.execute }.to change { registry.synced.missing_on_primary.count }.by(1)
end
end
shared_examples_for 'a service that downloads the file and registers the sync result' do |file_type| shared_examples_for 'a service that downloads the file and registers the sync result' do |file_type|
let(:download_service) { described_class.new(file_type, file.id) } let(:download_service) { described_class.new(file_type, file.id) }
let(:registry) { file_type == 'job_artifact' ? Geo::JobArtifactRegistry : Geo::FileRegistry } let(:registry) { file_type == 'job_artifact' ? Geo::JobArtifactRegistry : Geo::FileRegistry }
...@@ -276,51 +298,59 @@ describe Geo::FileDownloadService do ...@@ -276,51 +298,59 @@ describe Geo::FileDownloadService do
describe '#execute' do describe '#execute' do
context 'user avatar' do context 'user avatar' do
it_behaves_like "a service that downloads the file and registers the sync result", 'avatar' do let(:file) { create(:upload, model: build(:user)) }
let(:file) { create(:upload, model: build(:user)) }
end it_behaves_like "a service that downloads the file and registers the sync result", 'avatar'
it_behaves_like 'a service that handles orphaned uploads', 'avatar'
end end
context 'group avatar' do context 'group avatar' do
it_behaves_like "a service that downloads the file and registers the sync result", 'avatar' do let(:file) { create(:upload, model: build(:group)) }
let(:file) { create(:upload, model: build(:group)) }
end it_behaves_like "a service that downloads the file and registers the sync result", 'avatar'
it_behaves_like 'a service that handles orphaned uploads', 'avatar'
end end
context 'project avatar' do context 'project avatar' do
it_behaves_like "a service that downloads the file and registers the sync result", 'avatar' do let(:file) { create(:upload, model: build(:project)) }
let(:file) { create(:upload, model: build(:project)) }
end it_behaves_like "a service that downloads the file and registers the sync result", 'avatar'
it_behaves_like 'a service that handles orphaned uploads', 'avatar'
end end
context 'with an attachment' do context 'with an attachment' do
it_behaves_like "a service that downloads the file and registers the sync result", 'attachment' do let(:file) { create(:upload, :attachment_upload) }
let(:file) { create(:upload, :attachment_upload) }
end it_behaves_like "a service that downloads the file and registers the sync result", 'attachment'
it_behaves_like 'a service that handles orphaned uploads', 'attachment'
end end
context 'with a favicon' do context 'with a favicon' do
it_behaves_like "a service that downloads the file and registers the sync result", 'favicon' do let(:file) { create(:upload, :favicon_upload) }
let(:file) { create(:upload, :favicon_upload) }
end it_behaves_like "a service that downloads the file and registers the sync result", 'favicon'
it_behaves_like 'a service that handles orphaned uploads', 'favicon'
end end
context 'with a snippet' do context 'with a snippet' do
it_behaves_like "a service that downloads the file and registers the sync result", 'personal_file' do let(:file) { create(:upload, :personal_snippet_upload) }
let(:file) { create(:upload, :personal_snippet_upload) }
end it_behaves_like "a service that downloads the file and registers the sync result", 'personal_file'
it_behaves_like 'a service that handles orphaned uploads', 'personal_file'
end end
context 'with file upload' do context 'with file upload' do
it_behaves_like "a service that downloads the file and registers the sync result", 'file' do let(:file) { create(:upload, :issuable_upload) }
let(:file) { create(:upload, :issuable_upload) }
end it_behaves_like "a service that downloads the file and registers the sync result", 'file'
it_behaves_like 'a service that handles orphaned uploads', 'file'
end end
context 'with namespace file upload' do context 'with namespace file upload' do
it_behaves_like "a service that downloads the file and registers the sync result", 'namespace_file' do let(:file) { create(:upload, :namespace_upload) }
let(:file) { create(:upload, :namespace_upload) }
end it_behaves_like "a service that downloads the file and registers the sync result", 'namespace_file'
it_behaves_like 'a service that handles orphaned uploads', 'namespace_file'
end end
context 'LFS object' do context 'LFS object' do
...@@ -336,9 +366,10 @@ describe Geo::FileDownloadService do ...@@ -336,9 +366,10 @@ describe Geo::FileDownloadService do
end end
context 'Import/Export' do context 'Import/Export' do
it_behaves_like "a service that downloads the file and registers the sync result", 'import_export' do let(:file) { create(:upload, model: build(:import_export_upload)) }
let(:file) { create(:upload, model: build(:import_export_upload)) }
end it_behaves_like "a service that downloads the file and registers the sync result", 'import_export'
it_behaves_like 'a service that handles orphaned uploads', 'import_export'
end end
context 'bad object type' do context 'bad object type' do
......
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