Merge branch...

Merge branch '10999-geo-implement-selective-sync-support-for-the-attachments-fdw-queries' into 'master'

Geo - Implement selective sync support for the FDW queries to count attachments

See merge request gitlab-org/gitlab-ee!11518

ee/app/finders/geo/attachment_registry_finder.rb

@@ -6,42 +6,30 @@ module Geo
       syncable.count
     end
 
-    def syncable
-      if use_legacy_queries_for_selective_sync?
-        legacy_finder.syncable
-      elsif selective_sync?
-        fdw_all.geo_syncable
-      else
-        Upload.geo_syncable
-      end
-    end
-
     def count_synced
-      if aggregate_pushdown_supported?
-        find_synced.count
-      else
-        legacy_find_synced.count
-      end
+      attachments_synced.count
     end
 
     def count_failed
-      if aggregate_pushdown_supported?
-        find_failed.count
-      else
-        legacy_find_failed.count
-      end
+      attachments_failed.count
     end
 
     def count_synced_missing_on_primary
-      if aggregate_pushdown_supported? && !use_legacy_queries?
-        fdw_find_synced_missing_on_primary.count
-      else
-        legacy_find_synced_missing_on_primary.count
-      end
+      attachments_synced_missing_on_primary.count
     end
 
     def count_registry
-      Geo::FileRegistry.attachments.count
+      registries_for_attachments.count
+    end
+
+    def syncable
+      if use_legacy_queries_for_selective_sync?
+        legacy_finder.syncable
+      elsif selective_sync?
+        attachments.syncable
+      else
+        Upload.syncable
+      end
     end
 
     # Find limited amount of non replicated attachments.
@@ -82,7 +70,9 @@ module Geo
 
     # rubocop: disable CodeReuse/ActiveRecord
     def find_retryable_failed_registries(batch_size:, except_file_ids: [])
-      find_failed_registries
+      Geo::FileRegistry
+        .attachments
+        .failed
         .retry_due
         .file_id_not_in(except_file_ids)
         .limit(batch_size)
@@ -91,7 +81,10 @@ module Geo
 
     # rubocop: disable CodeReuse/ActiveRecord
     def find_retryable_synced_missing_on_primary_registries(batch_size:, except_file_ids: [])
-      find_synced_missing_on_primary_registries
+      Geo::FileRegistry
+        .attachments
+        .synced
+        .missing_on_primary
         .retry_due
         .file_id_not_in(except_file_ids)
         .limit(batch_size)
@@ -110,10 +103,51 @@ module Geo
       @fdw_geo_node ||= Geo::Fdw::GeoNode.find(current_node.id)
     end
 
+    def registries_for_attachments
+      if use_legacy_queries_for_selective_sync?
+        legacy_finder.registries_for_attachments
+      else
+        attachments
+          .inner_join_file_registry
+          .merge(Geo::FileRegistry.attachments)
+      end
+    end
+
+    def attachments_synced
+      if use_legacy_queries_for_selective_sync?
+        legacy_finder.attachments_synced
+      else
+        registries_for_attachments
+          .syncable
+          .merge(Geo::FileRegistry.synced)
+      end
+    end
+
+    def attachments_failed
+      if use_legacy_queries_for_selective_sync?
+        legacy_finder.attachments_failed
+      else
+        registries_for_attachments
+          .syncable
+          .merge(Geo::FileRegistry.failed)
+      end
+    end
+
+    def attachments_synced_missing_on_primary
+      if use_legacy_queries_for_selective_sync?
+        legacy_finder.attachments_synced_missing_on_primary
+      else
+        registries_for_attachments
+          .syncable
+          .merge(Geo::FileRegistry.synced)
+          .merge(Geo::FileRegistry.missing_on_primary)
+      end
+    end
+
     # rubocop: disable CodeReuse/ActiveRecord
-    def fdw_all
+    def attachments
       if selective_sync?
-        Geo::Fdw::Upload.where(fdw_group_uploads.or(fdw_project_uploads).or(fdw_other_uploads))
+        Geo::Fdw::Upload.where(group_uploads.or(project_uploads).or(other_uploads))
       else
         Geo::Fdw::Upload.all
       end
@@ -121,7 +155,7 @@ module Geo
     # rubocop: enable CodeReuse/ActiveRecord
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def fdw_group_uploads
+    def group_uploads
       namespace_ids =
         if current_node.selective_sync_by_namespaces?
           Gitlab::ObjectHierarchy.new(fdw_geo_node.namespaces).base_and_descendants.select(:id)
@@ -141,7 +175,7 @@ module Geo
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
-    def fdw_project_uploads
+    def project_uploads
       project_ids = fdw_geo_node.projects.select(:id)
 
       # This query was intentionally converted to a raw one to get it work in Rails 5.0.
@@ -152,7 +186,7 @@ module Geo
       fdw_upload_table[:model_type].eq('Project').and(project_ids_in_sql)
     end
 
-    def fdw_other_uploads
+    def other_uploads
       fdw_upload_table[:model_type].not_in(%w[Namespace Project])
     end
 
@@ -160,84 +194,21 @@ module Geo
       Geo::Fdw::Upload.arel_table
     end
 
-    def find_synced
-      if use_legacy_queries?
-        legacy_find_synced
-      else
-        fdw_find_synced
-      end
-    end
-
-    def find_failed
-      if use_legacy_queries?
-        legacy_find_failed
-      else
-        fdw_find_failed
-      end
-    end
-
-    def find_synced_registries
-      Geo::FileRegistry.attachments.synced
-    end
-
-    def find_failed_registries
-      Geo::FileRegistry.attachments.failed
-    end
-
-    def find_synced_missing_on_primary_registries
-      find_synced_registries.missing_on_primary
-    end
-
-    def fdw_find_synced
-      fdw_find_syncable.merge(Geo::FileRegistry.synced)
-    end
-
-    def fdw_find_failed
-      fdw_find_syncable.merge(Geo::FileRegistry.failed)
-    end
-
-    def fdw_find_syncable
-      fdw_all
-        .inner_join_file_registry
-        .geo_syncable
-        .merge(Geo::FileRegistry.attachments)
-    end
-
     def fdw_find_unsynced(except_file_ids:)
-      fdw_all
+      attachments
         .missing_file_registry
-        .geo_syncable
+        .syncable
         .id_not_in(except_file_ids)
     end
 
-    def fdw_find_synced_missing_on_primary
-      fdw_find_synced.merge(Geo::FileRegistry.missing_on_primary)
-    end
-
     def fdw_find_migrated_local(except_file_ids:)
-      fdw_all
+      attachments
         .inner_join_file_registry
         .with_files_stored_remotely
         .merge(Geo::FileRegistry.attachments)
         .id_not_in(except_file_ids)
     end
 
-    def legacy_find_synced
-      legacy_inner_join_registry_ids(
-        syncable,
-        find_synced_registries.pluck_file_key,
-        Upload
-      )
-    end
-
-    def legacy_find_failed
-      legacy_inner_join_registry_ids(
-        syncable,
-        find_failed_registries.pluck_file_key,
-        Upload
-      )
-    end
-
     def legacy_find_unsynced(except_file_ids:)
       registry_file_ids = Geo::FileRegistry.attachments.pluck_file_key | except_file_ids
 
@@ -257,13 +228,5 @@ module Geo
         Upload
       )
     end
-
-    def legacy_find_synced_missing_on_primary
-      legacy_inner_join_registry_ids(
-        syncable,
-        find_synced_missing_on_primary_registries.pluck_file_key,
-        Upload
-      )
-    end
   end
 end

ee/app/finders/geo/legacy_attachment_registry_finder.rb

@@ -3,7 +3,7 @@
 module Geo
   class LegacyAttachmentRegistryFinder < RegistryFinder
     def syncable
-      attachments.geo_syncable
+      attachments.syncable
     end
 
     # rubocop:disable CodeReuse/ActiveRecord
@@ -16,6 +16,41 @@ module Geo
     end
     # rubocop:enable CodeReuse/ActiveRecord
 
+    def attachments_synced
+      legacy_inner_join_registry_ids(
+        syncable,
+        Geo::FileRegistry.attachments.synced.pluck_file_key,
+        Upload
+      )
+    end
+
+    def attachments_failed
+      legacy_inner_join_registry_ids(
+        syncable,
+        Geo::FileRegistry.attachments.failed.pluck_file_key,
+        Upload
+      )
+    end
+
+    def attachments_synced_missing_on_primary
+      legacy_inner_join_registry_ids(
+        syncable,
+        Geo::FileRegistry.attachments.synced.missing_on_primary.pluck_file_key,
+        Upload
+      )
+    end
+
+    def registries_for_attachments
+      return Geo::FileRegistry.attachments unless selective_sync?
+
+      legacy_inner_join_registry_ids(
+        Geo::FileRegistry.attachments,
+        attachments.pluck_primary_key,
+        Geo::FileRegistry,
+        foreign_key: :file_id
+      )
+    end
+
     private
 
     # rubocop:disable CodeReuse/ActiveRecord

ee/app/models/ee/upload.rb

@@ -12,7 +12,7 @@ module EE
       after_destroy :log_geo_deleted_event
 
       scope :for_model, ->(model) { where(model_id: model.id, model_type: model.class.name) }
-      scope :geo_syncable, -> { with_files_stored_locally }
+      scope :syncable, -> { with_files_stored_locally }
     end
 
     def log_geo_deleted_event

ee/app/models/geo/fdw/upload.rb

@@ -10,7 +10,7 @@ module Geo
 
       self.table_name = Gitlab::Geo::Fdw.foreign_table_name('uploads')
 
-      scope :geo_syncable, -> { with_files_stored_locally }
+      scope :syncable, -> { with_files_stored_locally }
 
       class << self
         def for_model_with_type(model, type)

ee/changelogs/unreleased/10999-geo-implement-selective-sync-support-for-the-fdw-queries-to-count-attachments.yml

+---
+title: Geo - Implement selective sync support for the FDW queries to count attachments
+merge_request: 11518
+author:
+type: changed

ee/spec/finders/geo/attachment_registry_finder_spec.rb

@@ -21,11 +21,6 @@ describe Geo::AttachmentRegistryFinder, :geo do
   let(:upload_3) { create(:upload, :issuable_upload, model: synced_project) }
   let(:upload_4) { create(:upload, model: unsynced_project) }
   let(:upload_5) { create(:upload, model: synced_project) }
-  let(:upload_6) { create(:upload, :personal_snippet_upload) }
-  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(:upload_remote_1) { create(:upload, :object_storage, model: synced_project) }
 
   subject { described_class.new(current_node: secondary) }
 
@@ -33,30 +28,156 @@ describe Geo::AttachmentRegistryFinder, :geo do
     stub_current_geo_node(secondary)
   end
 
+  it 'responds to file registry finder methods' do
+    file_registry_finder_methods = %i{
+      syncable
+      count_syncable
+      count_synced
+      count_failed
+      count_synced_missing_on_primary
+      count_registry
+      find_unsynced
+      find_migrated_local
+      find_retryable_failed_registries
+      find_retryable_synced_missing_on_primary_registries
+    }
+
+    file_registry_finder_methods.each do |method|
+      expect(subject).to respond_to(method)
+    end
+  end
+
+  shared_examples 'finds all the things' do
+    describe '#find_unsynced' do
+      it 'returns uploads without an entry on the tracking database' do
+        create(:geo_file_registry, :avatar, file_id: upload_1.id)
+
+        uploads = subject.find_unsynced(batch_size: 10)
+
+        expect(uploads).to match_ids(upload_2, upload_3, upload_4)
+      end
+
+      it 'excludes uploads in the except_file_ids option' do
+        uploads = subject.find_unsynced(batch_size: 10, except_file_ids: [upload_2.id])
+
+        expect(uploads).to match_ids(upload_1, upload_3, upload_4)
+      end
+
+      it 'excludes remote uploads' do
+        upload_1.update!(store: ObjectStorage::Store::REMOTE)
+
+        uploads = subject.find_unsynced(batch_size: 10)
+
+        expect(uploads).to match_ids(upload_2, upload_3, upload_4)
+      end
+    end
+
+    describe '#find_migrated_local' do
+      it 'returns uploads stored remotely and successfully synced locally' do
+        upload = create(:upload, :object_storage, model: synced_group)
+        create(:geo_file_registry, :avatar, file_id: upload.id)
+
+        uploads = subject.find_migrated_local(batch_size: 100)
+
+        expect(uploads).to match_ids(upload)
+      end
+
+      it 'excludes uploads stored remotely, but not synced yet' do
+        create(:upload, :object_storage, model: synced_group)
+
+        uploads = subject.find_migrated_local(batch_size: 100)
+
+        expect(uploads).to be_empty
+      end
+
+      it 'excludes synced uploads that are stored locally' do
+        create(:geo_file_registry, :avatar, file_id: upload_5.id)
+
+        uploads = subject.find_migrated_local(batch_size: 100)
+
+        expect(uploads).to be_empty
+      end
+
+      it 'excludes except_file_ids' do
+        upload_a = create(:upload, :object_storage, model: synced_group)
+        upload_b = create(:upload, :object_storage, model: unsynced_group)
+        create(:geo_file_registry, :avatar, file_id: upload_a.id)
+        create(:geo_file_registry, :avatar, file_id: upload_b.id)
+
+        uploads = subject.find_migrated_local(batch_size: 10, except_file_ids: [upload_a.id])
+
+        expect(uploads).to match_ids(upload_b)
+      end
+    end
+  end
+
   shared_examples 'counts all the things' do
-    describe '#count_synced' do
-      it 'delegates to #legacy_find_synced' do
-        allow(subject).to receive(:aggregate_pushdown_supported?).and_return(false)
+    describe '#count_syncable' do
+      let!(:upload_1) { create(:upload, model: synced_group) }
+      let!(:upload_2) { create(:upload, model: unsynced_group) }
+      let!(:upload_3) { create(:upload, :issuable_upload, model: synced_project_in_nested_group) }
+      let!(:upload_4) { create(:upload, model: unsynced_project) }
+      let!(:upload_5) { create(:upload, :personal_snippet_upload) }
+
+      it 'counts attachments' do
+        expect(subject.count_syncable).to eq 5
+      end
+
+      it 'ignores remote attachments' do
+        upload_1.update!(store: ObjectStorage::Store::REMOTE)
 
-        expect(subject).to receive(:legacy_find_synced).and_call_original
+        expect(subject.count_syncable).to eq 4
+      end
 
-        subject.count_synced
+      context 'with selective sync by namespace' do
+        before do
+          secondary.update!(selective_sync_type: 'namespaces', namespaces: [synced_group])
         end
 
-      it 'delegates to #find_synced for PostgreSQL 10' do
-        allow(subject).to receive(:aggregate_pushdown_supported?).and_return(true)
+        it 'counts attachments' do
+          expect(subject.count_syncable).to eq 3
+        end
 
-        expect(subject).to receive(:find_synced).and_call_original
+        it 'ignores remote attachments' do
+          upload_1.update!(store: ObjectStorage::Store::REMOTE)
 
-        subject.count_synced
+          expect(subject.count_syncable).to eq 2
+        end
+      end
+
+      context 'with selective sync by shard' do
+        before do
+          secondary.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
         end
 
+        it 'counts attachments' do
+          expect(subject.count_syncable).to eq 3
+        end
+
+        it 'ignores remote attachments' do
+          upload_4.update!(store: ObjectStorage::Store::REMOTE)
+
+          expect(subject.count_syncable).to eq 2
+        end
+      end
+    end
+
+    describe '#count_synced' do
+      let!(:upload_1) { create(:upload, model: synced_group) }
+      let!(:upload_2) { create(:upload, model: unsynced_group) }
+      let!(:upload_3) { create(:upload, :issuable_upload, model: synced_project_in_nested_group) }
+      let!(:upload_4) { create(:upload, model: unsynced_project) }
+      let!(:upload_5) { create(:upload, :personal_snippet_upload) }
+      let(:upload_remote_1) { create(:upload, :object_storage, model: synced_project) }
+
       it 'counts attachments that have been synced' do
         create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
         create(:geo_file_registry, :attachment, file_id: upload_2.id)
         create(:geo_file_registry, :attachment, file_id: upload_3.id)
+        create(:geo_file_registry, :attachment, file_id: upload_4.id)
+        create(:geo_file_registry, :attachment, file_id: upload_5.id)
 
-        expect(subject.count_synced).to eq 2
+        expect(subject.count_synced).to eq 4
       end
 
       it 'ignores remote attachments' do
@@ -72,18 +193,14 @@ describe Geo::AttachmentRegistryFinder, :geo do
           secondary.update!(selective_sync_type: 'namespaces', namespaces: [synced_group])
         end
 
-        it 'delegates to #legacy_find_synced' do
-          expect(subject).to receive(:legacy_find_synced).and_call_original
-
-          subject.count_synced
-        end
-
         it 'counts attachments that has been synced' do
           create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
           create(:geo_file_registry, :attachment, file_id: upload_2.id)
           create(:geo_file_registry, :attachment, file_id: upload_3.id)
+          create(:geo_file_registry, :attachment, file_id: upload_4.id)
+          create(:geo_file_registry, :attachment, file_id: upload_5.id)
 
-          expect(subject.count_synced).to eq 1
+          expect(subject.count_synced).to eq 2
         end
 
         it 'ignores remote attachments' do
@@ -97,21 +214,17 @@ describe Geo::AttachmentRegistryFinder, :geo do
 
       context 'with selective sync by shard' do
         before do
-          secondary.update!(selective_sync_type: 'shards', selective_sync_shards: ['default'])
-        end
-
-        it 'delegates to #legacy_find_synced' do
-          expect(subject).to receive(:legacy_find_synced).and_call_original
-
-          subject.count_synced
+          secondary.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
         end
 
         it 'counts attachments that has been synced' do
           create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
           create(:geo_file_registry, :attachment, file_id: upload_2.id)
           create(:geo_file_registry, :attachment, file_id: upload_3.id)
+          create(:geo_file_registry, :attachment, file_id: upload_4.id)
+          create(:geo_file_registry, :attachment, file_id: upload_5.id)
 
-          expect(subject.count_synced).to eq 1
+          expect(subject.count_synced).to eq 3
         end
 
         it 'ignores remote attachments' do
@@ -125,28 +238,21 @@ describe Geo::AttachmentRegistryFinder, :geo do
     end
 
     describe '#count_failed' do
-      it 'delegates to #legacy_find_failed' do
-        allow(subject).to receive(:aggregate_pushdown_supported?).and_return(false)
-
-        expect(subject).to receive(:legacy_find_failed).and_call_original
-
-        subject.count_failed
-      end
-
-      it 'delegates to #find_failed' do
-        allow(subject).to receive(:aggregate_pushdown_supported?).and_return(true)
-
-        expect(subject).to receive(:find_failed).and_call_original
-
-        subject.count_failed
-      end
+      let!(:upload_1) { create(:upload, model: synced_group) }
+      let!(:upload_2) { create(:upload, model: unsynced_group) }
+      let!(:upload_3) { create(:upload, :issuable_upload, model: synced_project_in_nested_group) }
+      let!(:upload_4) { create(:upload, model: unsynced_project) }
+      let!(:upload_5) { create(:upload, :personal_snippet_upload) }
+      let(:upload_remote_1) { create(:upload, :object_storage, model: synced_project) }
 
       it 'counts attachments that sync has failed' do
         create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
         create(:geo_file_registry, :attachment, file_id: upload_2.id)
         create(:geo_file_registry, :attachment, :failed, file_id: upload_3.id)
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_4.id)
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_5.id)
 
-        expect(subject.count_failed).to eq 2
+        expect(subject.count_failed).to eq 4
       end
 
       it 'ignores remote attachments' do
@@ -162,30 +268,20 @@ describe Geo::AttachmentRegistryFinder, :geo do
           secondary.update!(selective_sync_type: 'namespaces', namespaces: [synced_group])
         end
 
-        it 'delegates to #legacy_find_failed' do
-          expect(subject).to receive(:legacy_find_failed).and_call_original
-
-          subject.count_failed
-        end
-
         it 'counts attachments that sync has failed' do
-          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
-          create(:geo_file_registry, :attachment, file_id: upload_3.id)
-
-          expect(subject.count_failed).to eq 1
-        end
-
-        it 'does not count attachments of unsynced projects' do
+          create(:geo_file_registry, :attachment, file_id: upload_1.id)
           create(:geo_file_registry, :attachment, :failed, file_id: upload_2.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_3.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_4.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_5.id)
 
-          expect(subject.count_failed).to eq 0
+          expect(subject.count_failed).to eq 2
         end
 
         it 'ignores remote attachments' do
-          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_remote_1.id)
           create(:geo_file_registry, :attachment, :failed, file_id: upload_2.id)
           create(:geo_file_registry, :attachment, :failed, file_id: upload_3.id)
-          upload_1.update!(store: ObjectStorage::Store::REMOTE)
 
           expect(subject.count_failed).to eq 1
         end
@@ -193,33 +289,23 @@ describe Geo::AttachmentRegistryFinder, :geo do
 
       context 'with selective sync by shard' do
         before do
-          secondary.update!(selective_sync_type: 'shards', selective_sync_shards: ['default'])
-        end
-
-        it 'delegates to #legacy_find_failed' do
-          expect(subject).to receive(:legacy_find_failed).and_call_original
-
-          subject.count_failed
+          secondary.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
         end
 
         it 'counts attachments that sync has failed' do
-          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
-          create(:geo_file_registry, :attachment, file_id: upload_3.id)
-
-          expect(subject.count_failed).to eq 1
-        end
-
-        it 'does not count attachments of unsynced projects' do
+          create(:geo_file_registry, :attachment, file_id: upload_1.id)
           create(:geo_file_registry, :attachment, :failed, file_id: upload_2.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_3.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_4.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_5.id)
 
-          expect(subject.count_failed).to eq 0
+          expect(subject.count_failed).to eq 3
         end
 
         it 'ignores remote attachments' do
-          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_remote_1.id)
           create(:geo_file_registry, :attachment, :failed, file_id: upload_2.id)
           create(:geo_file_registry, :attachment, :failed, file_id: upload_3.id)
-          upload_1.update!(store: ObjectStorage::Store::REMOTE)
 
           expect(subject.count_failed).to eq 1
         end
@@ -227,169 +313,96 @@ describe Geo::AttachmentRegistryFinder, :geo do
     end
 
     describe '#count_synced_missing_on_primary' do
-      it 'delegates to #legacy_find_synced_missing_on_primary' do
-        allow(subject).to receive(:aggregate_pushdown_supported?).and_return(false)
-
-        expect(subject).to receive(:legacy_find_synced_missing_on_primary).and_call_original
-
-        subject.count_synced_missing_on_primary
-      end
-
-      it 'delegates to #fdw_find_synced_missing_on_primary for PostgreSQL 10' do
-        allow(subject).to receive(:aggregate_pushdown_supported?).and_return(true)
-        allow(subject).to receive(:use_legacy_queries?).and_return(false)
-
-        expect(subject).to receive(:fdw_find_synced_missing_on_primary).and_return(double(count: 1))
-
-        subject.count_synced_missing_on_primary
-      end
+      let!(:upload_1) { create(:upload, model: synced_group) }
+      let!(:upload_2) { create(:upload, model: unsynced_group) }
+      let!(:upload_3) { create(:upload, :issuable_upload, model: synced_project_in_nested_group) }
+      let!(:upload_4) { create(:upload, model: unsynced_project) }
+      let!(:upload_5) { create(:upload, :personal_snippet_upload) }
+      let(:upload_remote_1) { create(:upload, :object_storage, model: synced_project) }
 
       it 'counts attachments that have been synced and are missing on the primary' do
-        create(:geo_file_registry, :attachment, file_id: upload_1.id, missing_on_primary: true)
-
-        expect(subject.count_synced_missing_on_primary).to eq 1
-      end
-
-      it 'excludes attachments that are not missing on the primary' do
-        create(:geo_file_registry, :attachment, file_id: upload_1.id)
-
-        expect(subject.count_synced_missing_on_primary).to eq 0
-      end
-
-      it 'excludes attachments that are not synced' do
         create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id, missing_on_primary: true)
+        create(:geo_file_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
+        create(:geo_file_registry, :attachment, file_id: upload_3.id, missing_on_primary: true)
+        create(:geo_file_registry, :attachment, file_id: upload_4.id, missing_on_primary: false)
+        create(:geo_file_registry, :attachment, file_id: upload_5.id, missing_on_primary: true)
 
-        expect(subject.count_synced_missing_on_primary).to eq 0
+        expect(subject.count_synced_missing_on_primary).to eq 3
       end
 
       it 'ignores remote attachments' do
         create(:geo_file_registry, :attachment, file_id: upload_remote_1.id, missing_on_primary: true)
+        create(:geo_file_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
+        create(:geo_file_registry, :attachment, file_id: upload_3.id, missing_on_primary: true)
 
-        expect(subject.count_synced_missing_on_primary).to eq 0
+        expect(subject.count_synced_missing_on_primary).to eq 2
       end
 
-      context 'with selective sync' do
+      context 'with selective sync by namespace' do
         before do
           secondary.update!(selective_sync_type: 'namespaces', namespaces: [synced_group])
         end
 
-        it 'delegates to #legacy_find_synced_missing_on_primary' do
-          expect(subject).to receive(:legacy_find_synced_missing_on_primary).and_call_original
-
-          subject.count_synced_missing_on_primary
-        end
-
-        it 'counts attachments that has been synced' do
-          create(:geo_file_registry, :attachment, file_id: upload_1.id, missing_on_primary: true)
+        it 'counts attachments that have been synced and are missing on the primary' do
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id, missing_on_primary: true)
           create(:geo_file_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
           create(:geo_file_registry, :attachment, file_id: upload_3.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_4.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_5.id, missing_on_primary: true)
 
           expect(subject.count_synced_missing_on_primary).to eq 2
         end
 
         it 'ignores remote attachments' do
           create(:geo_file_registry, :attachment, file_id: upload_remote_1.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_3.id, missing_on_primary: true)
 
-          expect(subject.count_synced_missing_on_primary).to eq 0
-        end
-      end
-    end
-  end
-
-  shared_examples 'finds all the things' do
-    describe '#find_unsynced' do
-      it 'delegates to the correct method' do
-        expect(subject).to receive("#{method_prefix}_find_unsynced".to_sym).and_call_original
-
-        subject.find_unsynced(batch_size: 10)
-      end
-
-      it 'returns uploads without an entry on the tracking database' do
-        create(:geo_file_registry, :avatar, file_id: upload_1.id)
-
-        uploads = subject.find_unsynced(batch_size: 10)
-
-        expect(uploads).to match_ids(upload_2, upload_3, upload_4)
-      end
-
-      it 'excludes uploads in the except_file_ids option' do
-        uploads = subject.find_unsynced(batch_size: 10, except_file_ids: [upload_2.id])
-
-        expect(uploads).to match_ids(upload_1, upload_3, upload_4)
-      end
-
-      it 'excludes remote uploads' do
-        upload_1.update!(store: ObjectStorage::Store::REMOTE)
-
-        uploads = subject.find_unsynced(batch_size: 10)
-
-        expect(uploads).to match_ids(upload_2, upload_3, upload_4)
-      end
+          expect(subject.count_synced_missing_on_primary).to eq 1
         end
-
-    describe '#find_migrated_local' do
-      it 'delegates to the correct method' do
-        expect(subject).to receive("#{method_prefix}_find_migrated_local".to_sym).and_call_original
-
-        subject.find_migrated_local(batch_size: 100)
       end
 
-      it 'returns uploads stored remotely and successfully synced locally' do
-        upload = create(:upload, :object_storage, model: synced_group)
-        create(:geo_file_registry, :avatar, file_id: upload.id)
-
-        uploads = subject.find_migrated_local(batch_size: 100)
-
-        expect(uploads).to match_ids(upload)
-      end
-
-      it 'excludes uploads stored remotely, but not synced yet' do
-        create(:upload, :object_storage, model: synced_group)
-
-        uploads = subject.find_migrated_local(batch_size: 100)
-
-        expect(uploads).to be_empty
+      context 'with selective sync by shard' do
+        before do
+          secondary.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
         end
 
-      it 'excludes synced uploads that are stored locally' do
-        create(:geo_file_registry, :avatar, file_id: upload_5.id)
-
-        uploads = subject.find_migrated_local(batch_size: 100)
+        it 'counts attachments that have been synced and are missing on the primary' do
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_3.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_4.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_5.id, missing_on_primary: true)
 
-        expect(uploads).to be_empty
+          expect(subject.count_synced_missing_on_primary).to eq 3
         end
 
-      it 'excludes except_file_ids' do
-        upload_a = create(:upload, :object_storage, model: synced_group)
-        upload_b = create(:upload, :object_storage, model: unsynced_group)
-        create(:geo_file_registry, :avatar, file_id: upload_a.id)
-        create(:geo_file_registry, :avatar, file_id: upload_b.id)
-
-        uploads = subject.find_migrated_local(batch_size: 10, except_file_ids: [upload_a.id])
+        it 'ignores remote attachments' do
+          create(:geo_file_registry, :attachment, file_id: upload_remote_1.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_3.id, missing_on_primary: true)
 
-        expect(uploads).to match_ids(upload_b)
+          expect(subject.count_synced_missing_on_primary).to eq 1
         end
       end
     end
 
-  it_behaves_like 'a file registry finder'
-
-  shared_examples 'counts all the things with selective sync coverage' do
-    describe '#count_syncable' do
+    describe '#count_registry' do
       let!(:upload_1) { create(:upload, model: synced_group) }
       let!(:upload_2) { create(:upload, model: unsynced_group) }
       let!(:upload_3) { create(:upload, :issuable_upload, model: synced_project_in_nested_group) }
       let!(:upload_4) { create(:upload, model: unsynced_project) }
       let!(:upload_5) { create(:upload, :personal_snippet_upload) }
+      let(:upload_remote_1) { create(:upload, :object_storage, model: synced_project) }
 
-      it 'counts attachments' do
-        expect(subject.count_syncable).to eq 5
-      end
-
-      it 'ignores remote attachments' do
-        upload_1.update!(store: ObjectStorage::Store::REMOTE)
+      it 'counts file registries for attachments' do
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
+        create(:geo_file_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
+        create(:geo_file_registry, :attachment, file_id: upload_3.id)
+        create(:geo_file_registry, :attachment, file_id: upload_4.id, missing_on_primary: false)
+        create(:geo_file_registry, :attachment, file_id: upload_5.id)
 
-        expect(subject.count_syncable).to eq 4
+        expect(subject.count_registry).to eq 5
       end
 
       context 'with selective sync by namespace' do
@@ -397,14 +410,14 @@ describe Geo::AttachmentRegistryFinder, :geo do
           secondary.update!(selective_sync_type: 'namespaces', namespaces: [synced_group])
         end
 
-        it 'counts attachments' do
-          expect(subject.count_syncable).to eq 3
-        end
-
-        it 'ignores remote attachments' do
-          upload_1.update!(store: ObjectStorage::Store::REMOTE)
+        it 'counts file registries for attachments' do
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_2.id)
+          create(:geo_file_registry, :attachment, file_id: upload_3.id)
+          create(:geo_file_registry, :attachment, file_id: upload_4.id)
+          create(:geo_file_registry, :attachment, file_id: upload_5.id, missing_on_primary: true)
 
-          expect(subject.count_syncable).to eq 2
+          expect(subject.count_registry).to eq 3
         end
       end
 
@@ -413,39 +426,27 @@ describe Geo::AttachmentRegistryFinder, :geo do
           secondary.update!(selective_sync_type: 'shards', selective_sync_shards: ['broken'])
         end
 
-        it 'counts attachments' do
-          expect(subject.count_syncable).to eq 3
-        end
-
-        it 'ignores remote attachments' do
-          upload_4.update!(store: ObjectStorage::Store::REMOTE)
+        it 'counts file registries for attachments' do
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
+          create(:geo_file_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
+          create(:geo_file_registry, :attachment, file_id: upload_3.id)
+          create(:geo_file_registry, :attachment, file_id: upload_4.id)
+          create(:geo_file_registry, :attachment, file_id: upload_5.id)
 
-          expect(subject.count_syncable).to eq 2
+          expect(subject.count_registry).to eq 3
         end
       end
     end
   end
 
   context 'FDW', :geo_fdw do
-    context 'with a PostgreSQL version that does not support aggregate pushdown' do
-      before do
-        allow(Gitlab::Database).to receive(:version).and_return(9.6)
-      end
-
-      include_examples 'counts all the things with selective sync coverage'
-    end
-
-    context 'with a PostgreSQL version that supports aggregate pushdown' do
-      before do
-        allow(Gitlab::Database).to receive(:version).and_return(10.0)
-      end
-
     context 'with use_fdw_queries_for_selective_sync disabled' do
       before do
         stub_feature_flags(use_fdw_queries_for_selective_sync: false)
       end
 
-        include_examples 'counts all the things with selective sync coverage'
+      include_examples 'counts all the things'
+      include_examples 'finds all the things'
     end
 
     context 'with use_fdw_queries_for_selective_sync enabled' do
@@ -453,8 +454,8 @@ describe Geo::AttachmentRegistryFinder, :geo do
         stub_feature_flags(use_fdw_queries_for_selective_sync: true)
       end
 
-        include_examples 'counts all the things with selective sync coverage'
-      end
+      include_examples 'counts all the things'
+      include_examples 'finds all the things'
     end
   end
 
@@ -463,6 +464,7 @@ describe Geo::AttachmentRegistryFinder, :geo do
       stub_fdw_disabled
     end
 
-    include_examples 'counts all the things with selective sync coverage'
+    include_examples 'counts all the things'
+    include_examples 'finds all the things'
   end
 end