Rename find_retryable_dirty_registries method

Refactor code to make the finder class
closer to a Geo replicator.

ee/app/finders/geo/container_repository_registry_finder.rb

@@ -2,18 +2,6 @@
 
 module Geo
   class ContainerRepositoryRegistryFinder < RegistryFinder
-    # rubocop:disable CodeReuse/ActiveRecord
-    def find_retryable_dirty_registries(batch_size:, except_ids: [])
-      registry_class
-        .retryable
-        .model_id_not_in(except_ids)
-        .order(Gitlab::Database.nulls_first_order(:last_synced_at))
-        .limit(batch_size)
-    end
-    # rubocop:enable CodeReuse/ActiveRecord
-
-    private
-
     def replicables
       current_node.container_repositories
     end

ee/app/finders/geo/design_registry_finder.rb

@@ -2,18 +2,6 @@
 
 module Geo
   class DesignRegistryFinder < RegistryFinder
-    # rubocop:disable CodeReuse/ActiveRecord
-    def find_retryable_dirty_registries(batch_size:, except_ids: [])
-      registry_class
-        .retryable
-        .model_id_not_in(except_ids)
-        .order(Gitlab::Database.nulls_first_order(:last_synced_at))
-        .limit(batch_size)
-    end
-    # rubocop:enable CodeReuse/ActiveRecord
-
-    private
-
     def replicables
       current_node.designs
     end

ee/app/finders/geo/file_registry_finder.rb

@@ -9,23 +9,6 @@ module Geo
       registry_class.synced.missing_on_primary.count
     end
 
-    # @!method find_retryable_failed_registries
-    #    Return an ActiveRecord::Relation of registry records marked as failed,
-    #    which are ready to be retried, excluding specified IDs, limited to
-    #    batch_size
-    #
-    # @param [Integer] batch_size used to limit the results returned
-    # @param [Array<Integer>] except_ids ids that will be ignored from the query
-    #
-    # rubocop:disable CodeReuse/ActiveRecord
-    def find_retryable_failed_registries(batch_size:, except_ids: [])
-      registry_class
-        .retryable
-        .model_id_not_in(except_ids)
-        .limit(batch_size)
-    end
-    # rubocop:enable CodeReuse/ActiveRecord
-
     # @!method find_retryable_synced_missing_on_primary_registries
     #    Return an ActiveRecord::Relation of registry records marked as synced
     #    and missing on the primary, which are ready to be retried, excluding

ee/app/finders/geo/project_registry_finder.rb

@@ -11,7 +11,7 @@ module Geo
     #
     # Any registries that have ever been synced that currently need to be
     # resynced will be handled by other find methods (like
-    # #find_retryable_dirty_registries)
+    # #find_failed_registries)
     #
     # You can pass a list with `except_ids:` so you can exclude items you
     # already scheduled but haven't finished and aren't persisted to the database yet
@@ -20,7 +20,7 @@ module Geo
     # @param [Array<Integer>] except_ids ids that will be ignored from the query
     # rubocop:disable CodeReuse/ActiveRecord
     def find_never_synced_registries(batch_size:, except_ids: [])
-      Geo::ProjectRegistry
+      registry_class
         .never_synced
         .model_id_not_in(except_ids)
         .limit(batch_size)
@@ -28,18 +28,15 @@ module Geo
     # rubocop:enable CodeReuse/ActiveRecord
 
     # rubocop:disable CodeReuse/ActiveRecord
-    def find_retryable_dirty_registries(batch_size:, except_ids: [])
-      Geo::ProjectRegistry
-        .retryable
-        .model_id_not_in(except_ids)
-        .order(Gitlab::Database.nulls_first_order(:last_repository_synced_at))
-        .limit(batch_size)
+    def find_failed_registries(batch_size:, except_ids: [])
+      registry_class
+        .find_failed_registries(batch_size: batch_size, except_ids: except_ids)
     end
     # rubocop:enable CodeReuse/ActiveRecord
 
     # rubocop:disable CodeReuse/ActiveRecord
     def find_project_ids_pending_verification(batch_size:, except_ids: [])
-      Geo::ProjectRegistry
+      registry_class
         .from_union([
           repositories_checksummed_pending_verification,
           wikis_checksummed_pending_verification
@@ -52,19 +49,23 @@ module Geo
 
     private
 
+    def registry_class
+      Geo::ProjectRegistry
+    end
+
     # rubocop:disable CodeReuse/ActiveRecord
     def repositories_checksummed_pending_verification
-      Geo::ProjectRegistry
+      registry_class
         .repositories_checksummed_pending_verification
-        .select(Geo::ProjectRegistry.arel_table[:project_id])
+        .select(registry_class.arel_table[:project_id])
     end
     # rubocop:enable CodeReuse/ActiveRecord
 
     # rubocop:disable CodeReuse/ActiveRecord
     def wikis_checksummed_pending_verification
-      Geo::ProjectRegistry
+      registry_class
         .wikis_checksummed_pending_verification
-        .select(Geo::ProjectRegistry.arel_table[:project_id])
+        .select(registry_class.arel_table[:project_id])
     end
     # rubocop:enable CodeReuse/ActiveRecord
   end

ee/app/finders/geo/registry_finder.rb

@@ -49,7 +49,7 @@ module Geo
     #
     #    Any registries that have ever been synced that currently need to be
     #    resynced will be handled by other find methods (like
-    #    #find_retryable_failed_registries)
+    #    #find_failed_registries)
     #
     #    You can pass a list with `except_ids:` so you can exclude items you
     #    already scheduled but haven't finished and aren't persisted to the database yet
@@ -66,6 +66,21 @@ module Geo
     end
     # rubocop:enable CodeReuse/ActiveRecord
 
+    # @!method find_failed_registries
+    #    Return an ActiveRecord::Relation of registry records marked as failed,
+    #    which are ready to be retried, excluding specified IDs, limited to
+    #    batch_size
+    #
+    # @param [Integer] batch_size used to limit the results returned
+    # @param [Array<Integer>] except_ids ids that will be ignored from the query
+    #
+    # rubocop:disable CodeReuse/ActiveRecord
+    def find_failed_registries(batch_size:, except_ids: [])
+      registry_class
+        .find_failed_registries(batch_size: batch_size, except_ids: except_ids)
+    end
+    # rubocop:enable CodeReuse/ActiveRecord
+
     # @!method registry_class
     #    Return an ActiveRecord::Base class for the tracked type
     def registry_class

ee/app/models/geo/container_repository_registry.rb

@@ -46,6 +46,11 @@ class Geo::ContainerRepositoryRegistry < Geo::BaseRegistry
     finder_class.new(current_node_id: Gitlab::Geo.current_node.id).find_registry_differences(range)
   end
 
+  def self.find_failed_registries(batch_size:, except_ids: [])
+    super
+      .order(Gitlab::Database.nulls_first_order(:last_synced_at))
+  end
+
   def self.delete_for_model_ids(container_repository_ids)
     where(container_repository_id: container_repository_ids).delete_all
 

ee/app/models/geo/design_registry.rb

@@ -59,6 +59,11 @@ class Geo::DesignRegistry < Geo::BaseRegistry
     finder_class.new(current_node_id: Gitlab::Geo.current_node.id).find_registry_differences(range)
   end
 
+  def self.find_failed_registries(batch_size:, except_ids: [])
+    super
+      .order(Gitlab::Database.nulls_first_order(:last_synced_at))
+  end
+
   # Search for a list of projects associated with registries,
   # based on the query given in `query`.
   #

ee/app/models/geo/project_registry.rb

@@ -54,6 +54,11 @@ class Geo::ProjectRegistry < Geo::BaseRegistry
     [untracked_ids, unused_tracked_ids]
   end
 
+  def self.find_failed_registries(batch_size:, except_ids: [])
+    super
+      .order(Gitlab::Database.nulls_first_order(:last_repository_synced_at))
+  end
+
   def self.delete_worker_class
     ::GeoRepositoryDestroyWorker
   end

ee/app/workers/geo/container_repository_sync_dispatch_worker.rb

@@ -54,7 +54,7 @@ module Geo
 
     def find_retryable_container_registry_ids(batch_size:)
       registry_finder
-        .find_retryable_dirty_registries(batch_size: batch_size, except_ids: scheduled_repository_ids)
+        .find_failed_registries(batch_size: batch_size, except_ids: scheduled_repository_ids)
         .pluck_model_foreign_key
     end
 

ee/app/workers/geo/design_repository_shard_sync_worker.rb

@@ -25,7 +25,7 @@ module Geo
     def find_project_ids_updated_recently(except_ids:, batch_size:)
       project_ids =
         registry_finder
-          .find_retryable_dirty_registries(batch_size: batch_size, except_ids: except_ids)
+          .find_failed_registries(batch_size: batch_size, except_ids: except_ids)
           .pluck_model_foreign_key
 
       find_project_ids_within_shard(project_ids, direction: :asc)

ee/app/workers/geo/file_download_dispatch_worker/job_finder.rb

@@ -29,7 +29,7 @@ module Geo
 
       def find_failed_jobs(batch_size:)
         convert_registry_relation_to_job_args(
-          registry_finder.find_retryable_failed_registries(find_batch_params(batch_size))
+          registry_finder.find_failed_registries(find_batch_params(batch_size))
         )
       end
 

ee/app/workers/geo/repository_shard_sync_worker.rb

@@ -89,7 +89,7 @@ module Geo
     def find_project_ids_updated_recently(except_ids:, batch_size:)
       project_ids =
         registry_finder
-          .find_retryable_dirty_registries(batch_size: batch_size, except_ids: except_ids)
+          .find_failed_registries(batch_size: batch_size, except_ids: except_ids)
           .pluck_model_foreign_key
 
       find_project_ids_within_shard(project_ids, direction: :asc)

ee/spec/finders/geo/attachment_registry_finder_spec.rb

@@ -153,7 +153,7 @@ RSpec.describe Geo::AttachmentRegistryFinder, :geo do
     end
   end
 
-  describe '#find_retryable_failed_registries' do
+  describe '#find_failed_registries' do
     it 'returns registries for job artifacts that have failed to sync' do
       registry_upload_1 = create(:geo_upload_registry, :attachment, :failed, file_id: upload_1.id)
       create(:geo_upload_registry, :attachment, file_id: upload_2.id, missing_on_primary: true)
@@ -164,7 +164,7 @@ RSpec.describe Geo::AttachmentRegistryFinder, :geo do
       create(:geo_upload_registry, :attachment, file_id: upload_7.id, missing_on_primary: true)
       create(:geo_upload_registry, :attachment, :never_synced, file_id: upload_8.id)
 
-      registries = subject.find_retryable_failed_registries(batch_size: 10)
+      registries = subject.find_failed_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_upload_1, registry_upload_4, registry_upload_6)
     end
@@ -179,7 +179,7 @@ RSpec.describe Geo::AttachmentRegistryFinder, :geo do
       create(:geo_upload_registry, :attachment, file_id: upload_7.id, missing_on_primary: true)
       create(:geo_upload_registry, :attachment, :never_synced, file_id: upload_8.id)
 
-      registries = subject.find_retryable_failed_registries(batch_size: 10, except_ids: [upload_4.id])
+      registries = subject.find_failed_registries(batch_size: 10, except_ids: [upload_4.id])
 
       expect(registries).to match_ids(registry_upload_1, registry_upload_6)
     end

ee/spec/finders/geo/container_repository_registry_finder_spec.rb

@@ -206,7 +206,7 @@ RSpec.describe Geo::ContainerRepositoryRegistryFinder, :geo do
     end
   end
 
-  describe '#find_retryable_dirty_registries' do
+  describe '#find_failed_registries' do
     let_it_be(:registry_container_registry_1) { create(:container_repository_registry, :synced, container_repository_id: container_repository_1.id) }
     let_it_be(:registry_container_registry_2) { create(:container_repository_registry, :sync_started, container_repository_id: container_repository_2.id) }
     let_it_be(:registry_container_registry_3) { create(:container_repository_registry, state: :failed, container_repository_id: container_repository_3.id, last_synced_at: nil) }
@@ -215,13 +215,13 @@ RSpec.describe Geo::ContainerRepositoryRegistryFinder, :geo do
     let_it_be(:registry_container_registry_6) { create(:container_repository_registry, state: :failed, container_repository_id: container_repository_6.id, last_synced_at: nil) }
 
     it 'returns registries for projects that have been recently updated' do
-      registries = subject.find_retryable_dirty_registries(batch_size: 10)
+      registries = subject.find_failed_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_container_registry_3, registry_container_registry_4, registry_container_registry_5, registry_container_registry_6)
     end
 
     it 'excludes except_ids' do
-      registries = subject.find_retryable_dirty_registries(batch_size: 10, except_ids: [container_repository_4.id, container_repository_5.id, container_repository_6.id])
+      registries = subject.find_failed_registries(batch_size: 10, except_ids: [container_repository_4.id, container_repository_5.id, container_repository_6.id])
 
       expect(registries).to match_ids(registry_container_registry_3)
     end

ee/spec/finders/geo/design_registry_finder_spec.rb

@@ -220,7 +220,7 @@ RSpec.describe Geo::DesignRegistryFinder, :geo do
     end
   end
 
-  describe '#find_retryable_dirty_registries' do
+  describe '#find_failed_registries' do
     let!(:registry_project_1) { create(:geo_design_registry, :synced, project_id: project_1.id) }
     let!(:registry_project_2) { create(:geo_design_registry, :sync_failed, project_id: project_2.id) }
     let!(:registry_project_3) { create(:geo_design_registry, project_id: project_3.id, last_synced_at: nil) }
@@ -229,13 +229,13 @@ RSpec.describe Geo::DesignRegistryFinder, :geo do
     let!(:registry_project_6) { create(:geo_design_registry, project_id: project_6.id, last_synced_at: nil) }
 
     it 'returns registries for projects that have been recently updated' do
-      registries = subject.find_retryable_dirty_registries(batch_size: 10)
+      registries = subject.find_failed_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_project_2, registry_project_3, registry_project_4, registry_project_5, registry_project_6)
     end
 
     it 'excludes except_ids' do
-      registries = subject.find_retryable_dirty_registries(batch_size: 10, except_ids: [project_4.id, project_5.id, project_6.id])
+      registries = subject.find_failed_registries(batch_size: 10, except_ids: [project_4.id, project_5.id, project_6.id])
 
       expect(registries).to match_ids(registry_project_2, registry_project_3)
     end

ee/spec/finders/geo/job_artifact_registry_finder_spec.rb

@@ -338,7 +338,7 @@ RSpec.describe Geo::JobArtifactRegistryFinder, :geo do
     end
   end
 
-  describe '#find_retryable_failed_registries' do
+  describe '#find_failed_registries' do
     it 'returns registries for job artifacts that have failed to sync' do
       registry_ci_job_artifact_1 = create(:geo_job_artifact_registry, :failed, artifact_id: ci_job_artifact_1.id)
       create(:geo_job_artifact_registry, artifact_id: ci_job_artifact_2.id, missing_on_primary: true)
@@ -349,7 +349,7 @@ RSpec.describe Geo::JobArtifactRegistryFinder, :geo do
       create(:geo_job_artifact_registry, artifact_id: ci_job_artifact_remote_2.id, missing_on_primary: true)
       create(:geo_job_artifact_registry, :never_synced, artifact_id: ci_job_artifact_remote_3.id)
 
-      registries = subject.find_retryable_failed_registries(batch_size: 10)
+      registries = subject.find_failed_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_ci_job_artifact_1, registry_ci_job_artifact_4, registry_ci_job_artifact_remote_1)
     end
@@ -364,7 +364,7 @@ RSpec.describe Geo::JobArtifactRegistryFinder, :geo do
       create(:geo_job_artifact_registry, artifact_id: ci_job_artifact_remote_2.id, missing_on_primary: true)
       create(:geo_job_artifact_registry, :never_synced, artifact_id: ci_job_artifact_remote_3.id)
 
-      registries = subject.find_retryable_failed_registries(batch_size: 10, except_ids: [ci_job_artifact_4.id])
+      registries = subject.find_failed_registries(batch_size: 10, except_ids: [ci_job_artifact_4.id])
 
       expect(registries).to match_ids(registry_ci_job_artifact_1, registry_ci_job_artifact_remote_1)
     end

ee/spec/finders/geo/lfs_object_registry_finder_spec.rb

@@ -285,7 +285,7 @@ RSpec.describe Geo::LfsObjectRegistryFinder, :geo do
     end
   end
 
-  describe '#find_retryable_failed_registries' do
+  describe '#find_failed_registries' do
     it 'returns registries for LFS objects that have failed to sync' do
       registry_lfs_object_1 = create(:geo_lfs_object_registry, :failed, lfs_object_id: lfs_object_1.id)
       create(:geo_lfs_object_registry, lfs_object_id: lfs_object_2.id, missing_on_primary: true)
@@ -296,7 +296,7 @@ RSpec.describe Geo::LfsObjectRegistryFinder, :geo do
       create(:geo_lfs_object_registry, lfs_object_id: lfs_object_remote_2.id, missing_on_primary: true)
       create(:geo_lfs_object_registry, :never_synced, lfs_object_id: lfs_object_remote_3.id)
 
-      registries = subject.find_retryable_failed_registries(batch_size: 10)
+      registries = subject.find_failed_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_lfs_object_1, registry_lfs_object_4, registry_lfs_object_remote_1)
     end
@@ -311,7 +311,7 @@ RSpec.describe Geo::LfsObjectRegistryFinder, :geo do
       create(:geo_lfs_object_registry, lfs_object_id: lfs_object_remote_2.id, missing_on_primary: true)
       create(:geo_lfs_object_registry, :never_synced, lfs_object_id: lfs_object_remote_3.id)
 
-      registries = subject.find_retryable_failed_registries(batch_size: 10, except_ids: [lfs_object_4.id])
+      registries = subject.find_failed_registries(batch_size: 10, except_ids: [lfs_object_4.id])
 
       expect(registries).to match_ids(registry_lfs_object_1, registry_lfs_object_remote_1)
     end

ee/spec/finders/geo/project_registry_finder_spec.rb

@@ -31,15 +31,15 @@ RSpec.describe Geo::ProjectRegistryFinder, :geo do
     end
   end
 
-  describe '#find_retryable_dirty_registries' do
+  describe '#find_failed_registries' do
     it 'returns registries for projects that have been recently updated or that have never been synced' do
-      registries = subject.find_retryable_dirty_registries(batch_size: 10)
+      registries = subject.find_failed_registries(batch_size: 10)
 
       expect(registries).to match_ids(registry_project_2, registry_project_3, registry_project_4, registry_project_5, registry_project_6)
     end
 
     it 'excludes except_ids' do
-      registries = subject.find_retryable_dirty_registries(batch_size: 10, except_ids: [project_4.id, project_5.id, project_6.id])
+      registries = subject.find_failed_registries(batch_size: 10, except_ids: [project_4.id, project_5.id, project_6.id])
 
       expect(registries).to match_ids(registry_project_2, registry_project_3)
     end

ee/spec/support/shared_examples/finders/geo/file_registry_finders_shared_examples.rb

@@ -8,7 +8,7 @@ RSpec.shared_examples 'a file registry finder' do
       synced_count
       failed_count
       synced_missing_on_primary_count
-      find_retryable_failed_registries
+      find_failed_registries
       find_retryable_synced_missing_on_primary_registries
     }