Merge branch 'ag-fix-verification-state-upload-bug' into 'master'

Geo: adapt verification timed out query to use state table

See merge request gitlab-org/gitlab!77364

ee/lib/gitlab/geo/verification_state.rb

@@ -35,7 +35,7 @@ module Gitlab
         scope :verification_failed, -> { available_verifiables.with_verification_state(:verification_failed) }
         scope :checksummed, -> { where.not(verification_checksum: nil) }
         scope :not_checksummed, -> { where(verification_checksum: nil) }
-        scope :verification_timed_out, -> { verification_started.where("verification_started_at < ?", VERIFICATION_TIMEOUT.ago) }
+        scope :verification_timed_out, -> { available_verifiables.where(verification_arel_table[:verification_state].eq(1)).where(verification_arel_table[:verification_started_at].lt(VERIFICATION_TIMEOUT.ago)) }
         scope :verification_retry_due, -> { where(verification_arel_table[:verification_retry_at].eq(nil).or(verification_arel_table[:verification_retry_at].lt(Time.current))) }
         scope :needs_verification, -> { available_verifiables.merge(with_verification_state(:verification_pending).or(with_verification_state(:verification_failed).verification_retry_due)) }
         scope :needs_reverification, -> { verification_succeeded.where("verified_at < ?", ::Gitlab::Geo.current_node.minimum_reverification_interval.days.ago) }
@@ -231,6 +231,14 @@ module Gitlab
           verification_state_table_class.arel_table
         end
 
+        # rubocop:disable CodeReuse/ActiveRecord
+        def verification_timed_out_batch_query
+          return verification_timed_out unless separate_verification_state_table?
+
+          verification_state_table_class.where(self.verification_state_model_key => verification_timed_out)
+        end
+        # rubocop:enable CodeReuse/ActiveRecord
+
         # Fail verification for records which started verification a long time ago
         def fail_verification_timeouts
           attrs = {
@@ -242,7 +250,7 @@ module Gitlab
             verified_at: Time.current
           }
 
-          verification_timed_out.each_batch do |relation|
+          verification_timed_out_batch_query.each_batch do |relation|
             relation.update_all(attrs)
           end
         end

ee/spec/support/helpers/ee/geo_helpers.rb

@@ -48,7 +48,7 @@ module EE
       allow(::Gitlab::Geo).to receive(:geo_database_configured?).and_call_original
     end
 
-    def stub_dummy_replicator_class
+    def stub_dummy_replicator_class(model_class: 'DummyModel')
       stub_const('Geo::DummyReplicator', Class.new(::Gitlab::Geo::Replicator))
 
       ::Geo::DummyReplicator.class_eval do
@@ -56,7 +56,7 @@ module EE
         event :another_test
 
         def self.model
-          ::DummyModel
+          model_class.constantize
         end
 
         def handle_after_create_commit
@@ -120,5 +120,112 @@ module EE
         drop_table :dummy_models, force: true
       end
     end
+
+    # Example:
+    #
+    # before(:all) do
+    #   create_dummy_model_with_separate_state_table
+    # end
+
+    # after(:all) do
+    #   drop_dummy_model_with_separate_state_table
+    # end
+
+    # before do
+    #   stub_dummy_model_with_separate_state_class
+    # end
+
+    def create_dummy_model_with_separate_state_table
+      ActiveRecord::Schema.define do
+        create_table :_test_dummy_model_with_separate_states, force: true
+      end
+
+      ActiveRecord::Schema.define do
+        create_table :_test_dummy_model_states, id: false, force: true do |t|
+          t.bigint :_test_dummy_model_with_separate_state_id
+          t.binary :verification_checksum
+          t.integer :verification_state
+          t.datetime_with_timezone :verification_started_at
+          t.datetime_with_timezone :verified_at
+          t.datetime_with_timezone :verification_retry_at
+          t.integer :verification_retry_count
+          t.text :verification_failure
+        end
+      end
+    end
+
+    def drop_dummy_model_with_separate_state_table
+      ActiveRecord::Schema.define do
+        drop_table :_test_dummy_model_with_separate_states, force: true
+      end
+
+      ActiveRecord::Schema.define do
+        drop_table :_test_dummy_model_states, force: true
+      end
+    end
+
+    def stub_dummy_model_with_separate_state_class
+      stub_const('TestDummyModelWithSeparateState', Class.new(ApplicationRecord))
+
+      TestDummyModelWithSeparateState.class_eval do
+        self.table_name = '_test_dummy_model_with_separate_states'
+
+        include ::Gitlab::Geo::ReplicableModel
+        include ::Gitlab::Geo::VerificationState
+
+        with_replicator Geo::DummyReplicator
+
+        has_one :_test_dummy_model_state,
+          autosave: false,
+          inverse_of: :_test_dummy_model_with_separate_state,
+          foreign_key: :_test_dummy_model_with_separate_state_id
+
+        after_save :save_verification_details
+
+        delegate :verification_retry_at, :verification_retry_at=,
+                 :verified_at, :verified_at=,
+                 :verification_checksum, :verification_checksum=,
+                 :verification_failure, :verification_failure=,
+                 :verification_retry_count, :verification_retry_count=,
+                 :verification_state=, :verification_state,
+                 :verification_started_at=, :verification_started_at,
+          to: :_test_dummy_model_state, allow_nil: true
+
+        scope :available_verifiables, -> { joins(:_test_dummy_model_state) }
+
+        def verification_state_object
+          _test_dummy_model_state
+        end
+
+        def self.replicables_for_current_secondary(primary_key_in)
+          self.primary_key_in(primary_key_in)
+        end
+
+        def self.verification_state_table_class
+          TestDummyModelState
+        end
+
+        private
+
+        def _test_dummy_model_state
+          super || build__test_dummy_model_state
+        end
+      end
+
+      TestDummyModelWithSeparateState.reset_column_information
+
+      stub_const('TestDummyModelState', Class.new(ApplicationRecord))
+
+      TestDummyModelState.class_eval do
+        include EachBatch
+
+        self.table_name = '_test_dummy_model_states'
+        self.primary_key = '_test_dummy_model_with_separate_state_id'
+
+        belongs_to :_test_dummy_model_with_separate_state, inverse_of: :_test_dummy_model_state
+      end
+
+      TestDummyModelState.reset_column_information
+    end
   end
 end