Commit d0c5ddca authored by Valery Sizov's avatar Valery Sizov

[Geo] Skip attachments that is stored in the object storage

parent 9d4757c2
...@@ -9,6 +9,8 @@ class Upload < ActiveRecord::Base ...@@ -9,6 +9,8 @@ class Upload < ActiveRecord::Base
validates :model, presence: true validates :model, presence: true
validates :uploader, presence: true validates :uploader, presence: true
scope :with_files_stored_locally, -> { where(store: [nil, ObjectStorage::Store::LOCAL]) }
before_save :calculate_checksum!, if: :foreground_checksummable? before_save :calculate_checksum!, if: :foreground_checksummable?
after_commit :schedule_checksum, if: :checksummable? after_commit :schedule_checksum, if: :checksummable?
......
module Geo module Geo
class AttachmentRegistryFinder < FileRegistryFinder class AttachmentRegistryFinder < FileRegistryFinder
def attachments def attachments
relation =
if selective_sync? if selective_sync?
Upload.where(group_uploads.or(project_uploads).or(other_uploads)) Upload.where(group_uploads.or(project_uploads).or(other_uploads))
else else
Upload.all Upload.all
end end
relation.with_files_stored_locally
end end
def count_attachments def count_attachments
...@@ -105,6 +108,7 @@ module Geo ...@@ -105,6 +108,7 @@ module Geo
fdw_table = Geo::Fdw::Upload.table_name fdw_table = Geo::Fdw::Upload.table_name
Geo::Fdw::Upload.joins("INNER JOIN file_registry ON file_registry.file_id = #{fdw_table}.id") Geo::Fdw::Upload.joins("INNER JOIN file_registry ON file_registry.file_id = #{fdw_table}.id")
.with_files_stored_locally
.merge(Geo::FileRegistry.attachments) .merge(Geo::FileRegistry.attachments)
end end
...@@ -115,6 +119,7 @@ module Geo ...@@ -115,6 +119,7 @@ module Geo
Geo::Fdw::Upload.joins("LEFT OUTER JOIN file_registry Geo::Fdw::Upload.joins("LEFT OUTER JOIN file_registry
ON file_registry.file_id = #{fdw_table}.id ON file_registry.file_id = #{fdw_table}.id
AND file_registry.file_type IN (#{upload_types})") AND file_registry.file_type IN (#{upload_types})")
.with_files_stored_locally
.where(file_registry: { id: nil }) .where(file_registry: { id: nil })
.where.not(id: except_registry_ids) .where.not(id: except_registry_ids)
end end
......
...@@ -78,7 +78,7 @@ module Geo ...@@ -78,7 +78,7 @@ module Geo
Geo::Fdw::Ci::JobArtifact.joins("LEFT OUTER JOIN file_registry Geo::Fdw::Ci::JobArtifact.joins("LEFT OUTER JOIN file_registry
ON file_registry.file_id = #{fdw_table}.id ON file_registry.file_id = #{fdw_table}.id
AND file_registry.file_type = 'job_artifact'") AND file_registry.file_type = 'job_artifact'")
.merge(Geo::Fdw::Ci::JobArtifact.with_files_stored_locally) .with_files_stored_locally
.where(file_registry: { id: nil }) .where(file_registry: { id: nil })
.where.not(id: except_registry_ids) .where.not(id: except_registry_ids)
end end
......
...@@ -79,7 +79,7 @@ module Geo ...@@ -79,7 +79,7 @@ module Geo
Geo::Fdw::LfsObject.joins("LEFT OUTER JOIN file_registry Geo::Fdw::LfsObject.joins("LEFT OUTER JOIN file_registry
ON file_registry.file_id = #{fdw_table}.id ON file_registry.file_id = #{fdw_table}.id
AND file_registry.file_type = 'lfs'") AND file_registry.file_type = 'lfs'")
.merge(Geo::Fdw::LfsObject.with_files_stored_locally) .with_files_stored_locally
.where(file_registry: { id: nil }) .where(file_registry: { id: nil })
.where.not(id: except_registry_ids) .where.not(id: except_registry_ids)
end end
......
...@@ -2,6 +2,8 @@ module Geo ...@@ -2,6 +2,8 @@ module Geo
module Fdw module Fdw
class Upload < ::Geo::BaseFdw class Upload < ::Geo::BaseFdw
self.table_name = Gitlab::Geo.fdw_table('uploads') self.table_name = Gitlab::Geo.fdw_table('uploads')
scope :with_files_stored_locally, -> { where(store: [nil, ObjectStorage::Store::LOCAL]) }
end end
end end
end end
...@@ -18,6 +18,8 @@ describe Geo::AttachmentRegistryFinder, :geo do ...@@ -18,6 +18,8 @@ describe Geo::AttachmentRegistryFinder, :geo do
let(:upload_5) { create(:upload, model: synced_project) } let(:upload_5) { create(:upload, model: synced_project) }
let(:upload_6) { create(:upload, :personal_snippet_upload) } let(:upload_6) { create(:upload, :personal_snippet_upload) }
let(:upload_7) { create(:upload, model: synced_subgroup) } let(:upload_7) { create(:upload, model: synced_subgroup) }
let(:upload_8) { create(:upload, :object_storage, model: unsynced_project) }
let(:upload_9) { create(:upload, :object_storage, model: unsynced_group) }
let(:lfs_object) { create(:lfs_object) } let(:lfs_object) { create(:lfs_object) }
subject { described_class.new(current_node: secondary) } subject { described_class.new(current_node: secondary) }
...@@ -135,6 +137,14 @@ describe Geo::AttachmentRegistryFinder, :geo do ...@@ -135,6 +137,14 @@ describe Geo::AttachmentRegistryFinder, :geo do
expect(uploads.map(&:id)).to match_array([upload_3.id, upload_4.id]) expect(uploads.map(&:id)).to match_array([upload_3.id, upload_4.id])
end end
it 'excludes remote uploads without an entry on the tracking database' do
create(:geo_file_registry, :avatar, file_id: upload_1.id, success: true)
uploads = subject.find_unsynced_attachments(batch_size: 10)
expect(uploads).not_to include(upload_8, upload_9)
end
end end
end end
...@@ -230,7 +240,7 @@ describe Geo::AttachmentRegistryFinder, :geo do ...@@ -230,7 +240,7 @@ describe Geo::AttachmentRegistryFinder, :geo do
subject.find_unsynced_attachments(batch_size: 10) subject.find_unsynced_attachments(batch_size: 10)
end end
it 'returns LFS objects without an entry on the tracking database' do it 'returns uploads without an entry on the tracking database' do
create(:geo_file_registry, :avatar, file_id: upload_1.id, success: true) create(:geo_file_registry, :avatar, file_id: upload_1.id, success: true)
uploads = subject.find_unsynced_attachments(batch_size: 10) uploads = subject.find_unsynced_attachments(batch_size: 10)
...@@ -245,6 +255,14 @@ describe Geo::AttachmentRegistryFinder, :geo do ...@@ -245,6 +255,14 @@ describe Geo::AttachmentRegistryFinder, :geo do
expect(uploads).to match_array([upload_3, upload_4]) expect(uploads).to match_array([upload_3, upload_4])
end end
it 'excludes remote uploads without an entry on the tracking database' do
create(:geo_file_registry, :avatar, file_id: upload_1.id, success: true)
uploads = subject.find_unsynced_attachments(batch_size: 10)
expect(uploads).not_to include(upload_8, upload_9)
end
end end
end end
end end
...@@ -25,6 +25,10 @@ FactoryBot.define do ...@@ -25,6 +25,10 @@ FactoryBot.define do
uploader "FileUploader" uploader "FileUploader"
end end
trait :object_storage do
store ObjectStorage::Store::REMOTE
end
trait :namespace_upload do trait :namespace_upload do
model { build(:group) } model { build(:group) }
path { File.join(secret, 'myfile.jpg') } path { File.join(secret, 'myfile.jpg') }
......
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