Remove FF to make registry table SSOT for uploads

The feature flag has been enabled by default
and will be removed and is working as expected.
parent 57d0e600
......@@ -14,10 +14,6 @@ class Geo::UploadRegistry < Geo::BaseRegistry
scope :fresh, -> { order(created_at: :desc) }
scope :never, -> { where(success: false, retry_count: nil) }
def self.registry_consistency_worker_enabled?
Feature.enabled?(:geo_file_registry_ssot_sync, default_enabled: true)
end
def self.finder_class
::Geo::AttachmentRegistryFinder
end
......
......@@ -10,13 +10,9 @@ module Geo
end
def find_unsynced_jobs(batch_size:)
if Geo::UploadRegistry.registry_consistency_worker_enabled?
convert_registry_relation_to_job_args(
registry_finder.find_never_synced_registries(find_batch_params(batch_size))
)
else
super
end
end
private
......
......@@ -125,7 +125,7 @@ RSpec.describe GeoNodeStatus, :geo, :geo_fdw do
describe '#attachments_synced_count' do
it 'only counts successful syncs' do
create_list(:user, 3, avatar: fixture_file_upload('spec/fixtures/dk.png', 'image/png'))
uploads = Upload.all.pluck(:id)
uploads = Upload.pluck(:id)
create(:geo_upload_registry, :avatar, file_id: uploads[0])
create(:geo_upload_registry, :avatar, file_id: uploads[1])
......@@ -133,42 +133,12 @@ RSpec.describe GeoNodeStatus, :geo, :geo_fdw do
expect(subject.attachments_synced_count).to eq(2)
end
it 'does not count synced files that were replaced' do
user = create(:user, avatar: fixture_file_upload('spec/fixtures/dk.png', 'image/png'))
expect(subject.attachments_count).to eq(1)
expect(subject.attachments_synced_count).to eq(0)
upload = Upload.find_by(model: user, uploader: 'AvatarUploader')
create(:geo_upload_registry, :avatar, file_id: upload.id)
subject = described_class.current_node_status
expect(subject.attachments_count).to eq(1)
expect(subject.attachments_synced_count).to eq(1)
user.update(avatar: fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpeg'))
subject = described_class.current_node_status
expect(subject.attachments_count).to eq(1)
expect(subject.attachments_synced_count).to eq(0)
upload = Upload.find_by(model: user, uploader: 'AvatarUploader')
create(:geo_upload_registry, :avatar, file_id: upload.id)
subject = described_class.current_node_status
expect(subject.attachments_count).to eq(1)
expect(subject.attachments_synced_count).to eq(1)
end
end
describe '#attachments_synced_missing_on_primary_count' do
it 'only counts successful syncs' do
create_list(:user, 3, avatar: fixture_file_upload('spec/fixtures/dk.png', 'image/png'))
uploads = Upload.all.pluck(:id)
uploads = Upload.pluck(:id)
create(:geo_upload_registry, :avatar, file_id: uploads[0], missing_on_primary: true)
create(:geo_upload_registry, :avatar, file_id: uploads[1])
......@@ -194,32 +164,20 @@ RSpec.describe GeoNodeStatus, :geo, :geo_fdw do
end
describe '#attachments_synced_in_percentage' do
let(:avatar) { fixture_file_upload('spec/fixtures/dk.png') }
let(:upload_1) { create(:upload, model: group, path: avatar) }
let(:upload_2) { create(:upload, model: project_1, path: avatar) }
before do
create(:upload, model: create(:group), path: avatar)
create(:upload, model: project_3, path: avatar)
end
it 'returns 0 when no objects are available' do
it 'returns 0 when no registries are available' do
expect(subject.attachments_synced_in_percentage).to eq(0)
end
it 'returns the right percentage with no group restrictions' do
create(:geo_upload_registry, :avatar, file_id: upload_1.id)
create(:geo_upload_registry, :avatar, file_id: upload_2.id)
expect(subject.attachments_synced_in_percentage).to be_within(0.0001).of(50)
end
it 'returns the right percentage' do
create_list(:user, 4, avatar: fixture_file_upload('spec/fixtures/dk.png', 'image/png'))
uploads = Upload.pluck(:id)
it 'returns the right percentage with group restrictions' do
secondary.update!(selective_sync_type: 'namespaces', namespaces: [group])
create(:geo_upload_registry, :avatar, file_id: upload_1.id)
create(:geo_upload_registry, :avatar, file_id: upload_2.id)
create(:geo_upload_registry, :avatar, file_id: uploads[0])
create(:geo_upload_registry, :avatar, file_id: uploads[1])
create(:geo_upload_registry, :avatar, :failed, file_id: uploads[2])
create(:geo_upload_registry, :avatar, :never_synced, file_id: uploads[3])
expect(subject.attachments_synced_in_percentage).to be_within(0.0001).of(100)
expect(subject.attachments_synced_in_percentage).to be_within(0.0001).of(50)
end
end
......
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw, :use_sql_query_cache_for_tracking_db do
RSpec.describe Geo::FileDownloadDispatchWorker, :geo, :use_sql_query_cache_for_tracking_db do
include ::EE::GeoHelpers
include ExclusiveLeaseHelpers
......@@ -88,11 +88,6 @@ RSpec.describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw, :use_sql_query_c
context 'with attachments (Upload records)' do
let(:upload) { create(:upload) }
context 'with geo_file_registry_ssot_sync feature enabled' do
before do
stub_feature_flags(geo_file_registry_ssot_sync: true)
end
it 'performs Geo::FileDownloadWorker for unsynced attachments' do
create(:geo_upload_registry, :avatar, :never_synced, file_id: upload.id)
......@@ -197,114 +192,6 @@ RSpec.describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw, :use_sql_query_c
end
end
context 'with geo_file_registry_ssot_sync feature disabled' do
before do
stub_feature_flags(geo_file_registry_ssot_sync: false)
end
it 'performs Geo::FileDownloadWorker for unsynced attachments' do
expect(Geo::FileDownloadWorker).to receive(:perform_async).with('avatar', upload.id)
subject.perform
end
it 'performs Geo::FileDownloadWorker for failed-sync attachments' do
create(:geo_upload_registry, :avatar, :failed, file_id: upload.id, bytes: 0)
expect(Geo::FileDownloadWorker).to receive(:perform_async)
.with('avatar', upload.id).once.and_return(spy)
subject.perform
end
it 'does not perform Geo::FileDownloadWorker for synced attachments' do
create(:geo_upload_registry, :avatar, file_id: upload.id, bytes: 1234)
expect(Geo::FileDownloadWorker).not_to receive(:perform_async)
subject.perform
end
it 'does not perform Geo::FileDownloadWorker for synced attachments even with 0 bytes downloaded' do
create(:geo_upload_registry, :avatar, file_id: upload.id, bytes: 0)
expect(Geo::FileDownloadWorker).not_to receive(:perform_async)
subject.perform
end
context 'with a failed file' do
let(:failed_registry) { create(:geo_upload_registry, :avatar, :failed, file_id: non_existing_record_id) }
it 'does not stall backfill' do
unsynced = create(:upload)
stub_const('Geo::Scheduler::SchedulerWorker::DB_RETRIEVE_BATCH_SIZE', 1)
expect(Geo::FileDownloadWorker).not_to receive(:perform_async).with('avatar', failed_registry.file_id)
expect(Geo::FileDownloadWorker).to receive(:perform_async).with('avatar', unsynced.id)
subject.perform
end
it 'retries failed files' do
expect(Geo::FileDownloadWorker).to receive(:perform_async).with('avatar', failed_registry.file_id)
subject.perform
end
it 'does not retry failed files when retry_at is tomorrow' do
failed_registry = create(:geo_upload_registry, :avatar, :failed, file_id: non_existing_record_id, retry_at: Date.tomorrow)
expect(Geo::FileDownloadWorker).not_to receive(:perform_async).with('avatar', failed_registry.file_id)
subject.perform
end
it 'retries failed files when retry_at is in the past' do
failed_registry = create(:geo_upload_registry, :avatar, :failed, file_id: non_existing_record_id, retry_at: Date.yesterday)
expect(Geo::FileDownloadWorker).to receive(:perform_async).with('avatar', failed_registry.file_id)
subject.perform
end
end
context 'with Upload files missing on the primary that are marked as synced' do
let(:synced_upload_with_file_missing_on_primary) { create(:upload) }
before do
Geo::UploadRegistry.create!(file_type: :avatar, file_id: synced_upload_with_file_missing_on_primary.id, bytes: 1234, success: true, missing_on_primary: true)
end
it 'retries the files if there is spare capacity' do
expect(Geo::FileDownloadWorker).to receive(:perform_async).with('avatar', synced_upload_with_file_missing_on_primary.id)
subject.perform
end
it 'does not retry those files if there is no spare capacity' do
unsynced_upload = create(:upload)
expect(subject).to receive(:db_retrieve_batch_size).and_return(1).twice
expect(Geo::FileDownloadWorker).to receive(:perform_async).with('avatar', unsynced_upload.id)
subject.perform
end
it 'does not retry those files if they are already scheduled' do
unsynced_upload = create(:upload)
scheduled_jobs = [{ type: 'avatar', id: synced_upload_with_file_missing_on_primary.id, job_id: 'foo' }]
expect(subject).to receive(:scheduled_jobs).and_return(scheduled_jobs).at_least(1)
expect(Geo::FileDownloadWorker).to receive(:perform_async).with('avatar', unsynced_upload.id)
subject.perform
end
end
end
end
context 'with LFS objects' do
let!(:lfs_object_local_store) { create(:lfs_object, :with_file) }
let!(:lfs_object_remote_store) { create(:lfs_object, :with_file, :object_storage) }
......@@ -506,41 +393,4 @@ RSpec.describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw, :use_sql_query_c
subject.perform
end
end
context 'when node has namespace restrictions', :request_store do
let(:synced_group) { create(:group) }
let(:project_in_synced_group) { create(:project, group: synced_group) }
let(:unsynced_project) { create(:project) }
before do
secondary.update!(selective_sync_type: 'namespaces', namespaces: [synced_group])
allow(ProjectCacheWorker).to receive(:perform_async).and_return(true)
allow(::Gitlab::Geo).to receive(:current_node).and_call_original
Rails.cache.write(:current_node, secondary.to_json)
allow(::GeoNode).to receive(:current_node).and_return(secondary)
end
context 'with geo_file_registry_ssot_sync feature disabled' do
before do
stub_feature_flags(geo_file_registry_ssot_sync: false)
end
it 'does not perform Geo::FileDownloadWorker for upload objects that do not belong to selected namespaces to replicate' do
avatar = fixture_file_upload('spec/fixtures/dk.png')
avatar_in_synced_group = create(:upload, model: synced_group, path: avatar)
create(:upload, model: create(:group), path: avatar)
avatar_in_project_in_synced_group = create(:upload, model: project_in_synced_group, path: avatar)
create(:upload, model: unsynced_project, path: avatar)
expect(Geo::FileDownloadWorker).to receive(:perform_async)
.with('avatar', avatar_in_project_in_synced_group.id).once.and_return(spy)
expect(Geo::FileDownloadWorker).to receive(:perform_async)
.with('avatar', avatar_in_synced_group.id).once.and_return(spy)
subject.perform
end
end
end
end
......@@ -97,27 +97,6 @@ RSpec.describe Geo::Secondary::RegistryConsistencyWorker, :geo, :geo_fdw do
expect(Geo::PackageFileRegistry.where(package_file_id: package_file.id).count).to eq(1)
end
context 'when geo_file_registry_ssot_sync is disabled' do
before do
stub_feature_flags(geo_file_registry_ssot_sync: false)
end
it 'returns false' do
expect(subject.perform).to be_falsey
end
it 'does not execute RegistryConsistencyService for Uploads' do
allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::JobArtifactRegistry, batch_size: 1000).and_call_original
allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::LfsObjectRegistry, batch_size: 1000).and_call_original
allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::PackageFileRegistry, batch_size: 1000).and_call_original
allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::ProjectRegistry, batch_size: 1000).and_call_original
expect(Geo::RegistryConsistencyService).not_to receive(:new).with(Geo::UploadRegistry, batch_size: 1000)
subject.perform
end
end
context 'when geo_project_registry_ssot_sync is disabled' do
before do
stub_feature_flags(geo_project_registry_ssot_sync: false)
......
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