Improve performance of Geo verification queries

Adapt Geo verification queries to conditionally
use replicable model table or a separate
verification details table.

With the use of separate tables for verification
information, we want to adapt verification queries
such that they don't use extra filters that were
already used to backfill the separate tables.

This is done by splitting the original scope on
replicable models, `available_verifiables` into
two new scopes, `verifiables` and
`with_verification_state_record`.

EE: true

ee/app/models/ee/merge_request_diff.rb

@@ -29,6 +29,7 @@ module EE
       scope :has_external_diffs, -> { with_files.where(stored_externally: true) }
       scope :project_id_in, ->(ids) { where(merge_request_id: ::MergeRequest.where(target_project_id: ids)) }
       scope :available_replicables, -> { has_external_diffs }
+      scope :available_verifiables, -> { joins(:merge_request_diff_detail) }
       scope :with_verification_state, ->(state) { joins(:merge_request_diff_detail).where(merge_request_diff_details: { verification_state: verification_state_value(state) }) }
       scope :checksummed, -> { joins(:merge_request_diff_detail).where.not(merge_request_diff_details: { verification_checksum: nil } ) }
       scope :not_checksummed, -> { joins(:merge_request_diff_detail).where(merge_request_diff_details: { verification_checksum: nil } ) }

ee/app/services/geo/verification_state_backfill_service.rb

@@ -40,13 +40,17 @@ module Geo
     end
 
     # This method creates or deletes verification details records.
-    # It creates for available verifiables where records don't exist yet.
+    #
+    # It looks for replicable records that are eligible for verification (scoped
+    # as `verifiables`) but whose corresponding verification details record doesn't
+    # exist yet.
     # These would be replicable records that have recently become scoped as
-    # available_verifiables.
+    # `verifiables`, but were not so at the time of creation.
     # New replicable records will automatically create child records in the
     # verification details table, hence not created in this method.
+    #
     # When a replicable record is no longer a part of the scope
-    # available_veriables, it is deleted.
+    # `verifiables`, the corresponding verification state record needs to be deleted.
     # When a replicable record is deleted, the child record in the verification
     # details table is automatically removed, hence not deleted in this method.
     #

ee/lib/gitlab/geo/replicable_model.rb

@@ -21,7 +21,28 @@ module Gitlab
 
         # These scopes are intended to be overridden as needed
         scope :available_replicables, -> { all }
-        scope :available_verifiables, -> { self.respond_to?(:with_files_stored_locally) ? available_replicables.with_files_stored_locally : available_replicables }
+
+        # On primary, `verifiables` are records that can be checksummed and/or are replicable.
+
+        # On secondary, `verifiables` are records that have already been replicated
+        # and (ideally) have been checksummed on the primary
+
+        scope :verifiables, -> { self.respond_to?(:with_files_stored_locally) ? available_replicables.with_files_stored_locally : available_replicables }
+
+        # When storing verification details in the same table as the model,
+        # the scope `available_verifiables` returns only those records
+        # that are eligible for verification, i.e. the same as the scope
+        # `verifiables`.
+
+        # When using a separate table to store verification details,
+        # the scope `available_verifiables` should return all records
+        # from the separate table because the separate table will
+        # always only have records corresponding to replicables that are verifiable.
+        # For this, override the scope in the replicable model, e.g. like so in
+        # `MergeRequestDiff`,
+        # `scope :available_verifiables, -> { joins(:merge_request_diff_detail) }`
+
+        scope :available_verifiables, -> { verifiables }
       end
 
       class_methods do

ee/lib/gitlab/geo/verification_state.rb

@@ -95,7 +95,7 @@ module Gitlab
         def save_verification_details
           return unless self.class.separate_verification_state_table?
 
-          return unless self.class.available_verifiables.primary_key_in(self).exists?
+          return unless self.class.verifiables.primary_key_in(self).exists?
 
           # During a transaction, `verification_state_object` could be built before
           # a value for `verification_state_model_key` exists. So we check for that
@@ -306,7 +306,7 @@ module Gitlab
 
         def pluck_verifiable_ids_in_range(range)
           self
-            .available_verifiables
+            .verifiables
             .primary_key_in(range)
             .pluck_primary_key
         end

ee/spec/workers/geo/verification_state_backfill_service_spec.rb

@@ -3,15 +3,14 @@
 require 'spec_helper'
 
 RSpec.describe Geo::VerificationStateBackfillService, :geo do
-  let(:replicable_1) { FactoryBot.create(:merge_request_diff, :external) }
-  let(:verifiable_1) { FactoryBot.create(:merge_request_diff_detail, merge_request_diff: replicable_1) }
+  let_it_be(:replicable) { create(:merge_request_diff, :external) }
 
   subject(:job) { described_class.new(MergeRequestDiff, batch_size: 1000) }
 
   describe '#execute' do
     context 'when a replicable is missing a corresponding verifiable' do
       before do
-        replicable_1.merge_request_diff_detail.destroy!
+        replicable.merge_request_diff_detail.destroy!
       end
 
       it 'adds a new verifiable' do
@@ -20,8 +19,10 @@ RSpec.describe Geo::VerificationStateBackfillService, :geo do
     end
 
     context 'when some replicables were removed from scope' do
+      let(:verifiable) { create(:merge_request_diff_detail, merge_request_diff: replicable) }
+
       before do
-        replicable_1.update_attribute(:stored_externally, false)
+        replicable.update_attribute(:stored_externally, false)
       end
 
       it 'deletes the verifiable' do