Apply retry at time to verification_failed_batch

ee/lib/gitlab/geo/verification_state.rb

@@ -35,6 +35,7 @@ module Gitlab
         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 :retry_due, -> { where(arel_table[:verification_retry_at].eq(nil).or(arel_table[:verification_retry_at].lt(Time.current))) }
         scope :needs_verification, -> { with_verification_state(:verification_pending, :verification_failed) }
         # rubocop:enable CodeReuse/ActiveRecord
 
@@ -124,7 +125,7 @@ module Gitlab
 
         # Overridden by Geo::VerifiableRegistry
         def verification_failed_batch_relation(batch_size:)
-          verification_failed.order(Gitlab::Database.nulls_first_order(:verification_retry_at)).limit(batch_size) # rubocop:disable CodeReuse/ActiveRecord
+          verification_failed.retry_due.order(Gitlab::Database.nulls_first_order(:verification_retry_at)).limit(batch_size) # rubocop:disable CodeReuse/ActiveRecord
         end
 
         # @return [Integer] number of records that need verification

ee/spec/lib/gitlab/geo/verification_state_spec.rb

@@ -90,43 +90,57 @@ RSpec.describe Gitlab::Geo::VerificationState do
     let(:other_failed_ids) { other_failed_records.map { |result| result['id'] } }
 
     before do
-      subject.verification_started!
+      subject.verification_started
       subject.verification_failed_with_message!('foo')
     end
 
-    it 'returns IDs of rows pending verification' do
-      expect(subject.class.verification_failed_batch(batch_size: 3)).to include(subject.id)
-    end
+    context 'with a failed record with retry due' do
+      before do
+        subject.update!(verification_retry_at: 1.minute.ago)
+      end
 
-    it 'marks verification as started' do
-      subject.class.verification_failed_batch(batch_size: 3)
+      it 'returns IDs of rows pending verification' do
+        expect(subject.class.verification_failed_batch(batch_size: 3)).to include(subject.id)
+      end
 
-      expect(subject.reload.verification_started?).to be_truthy
-      expect(subject.verification_started_at).to be_present
-    end
+      it 'marks verification as started' do
+        subject.class.verification_failed_batch(batch_size: 3)
 
-    it 'limits with batch_size and orders records by verification_retry_at with NULLs first' do
-      expected = other_failed_ids
+        expect(subject.reload.verification_started?).to be_truthy
+        expect(subject.verification_started_at).to be_present
+      end
 
-      # `match_array` instead of `eq` because the UPDATE query does not
-      # guarantee that results are returned in the same order as the subquery
-      # used to SELECT the correct batch.
-      expect(subject.class.verification_failed_batch(batch_size: 2)).to match_array(expected)
-    end
+      it 'limits with batch_size and orders records by verification_retry_at with NULLs first' do
+        expected = other_failed_ids
 
-    context 'other verification states' do
-      it 'does not include them' do
-        subject.verification_started!
+        # `match_array` instead of `eq` because the UPDATE query does not
+        # guarantee that results are returned in the same order as the subquery
+        # used to SELECT the correct batch.
+        expect(subject.class.verification_failed_batch(batch_size: 2)).to match_array(expected)
+      end
 
-        expect(subject.class.verification_failed_batch(batch_size: 5)).not_to include(subject.id)
+      context 'other verification states' do
+        it 'does not include them' do
+          subject.verification_started!
 
-        subject.verification_succeeded_with_checksum!('foo', Time.current)
+          expect(subject.class.verification_failed_batch(batch_size: 5)).not_to include(subject.id)
 
-        expect(subject.class.verification_failed_batch(batch_size: 5)).not_to include(subject.id)
+          subject.verification_succeeded_with_checksum!('foo', Time.current)
+
+          expect(subject.class.verification_failed_batch(batch_size: 5)).not_to include(subject.id)
+
+          subject.verification_pending!
+
+          expect(subject.class.verification_failed_batch(batch_size: 5)).not_to include(subject.id)
+        end
+      end
+    end
 
-        subject.verification_pending!
+    context 'when verification_retry_at is in the future' do
+      it 'does not return the row' do
+        subject.update!(verification_retry_at: 1.minute.from_now)
 
-        expect(subject.class.verification_failed_batch(batch_size: 5)).not_to include(subject.id)
+        expect(subject.class.verification_failed_batch(batch_size: 3)).not_to include(subject.id)
       end
     end
   end

ee/spec/support/shared_examples/models/geo_verifiable_registry_shared_examples.rb

@@ -42,28 +42,42 @@ RSpec.shared_examples 'a Geo verifiable registry' do
       subject.verification_failed_with_message!('foo')
     end
 
-    it 'returns IDs of rows which are synced and failed verification' do
-      expect(described_class.verification_failed_batch(batch_size: 4)).to match_array([subject.model_record_id])
-    end
+    context 'with a failed record with retry due' do
+      before do
+        subject.update!(verification_retry_at: 1.minute.ago)
+      end
 
-    it 'excludes rows which are not synced or have not failed verification' do
-      # rubocop:disable Rails/SaveBang
-      create(registry_class_factory, verification_state: verification_state_value(:verification_failed), verification_failure: 'foo')
-      create(registry_class_factory, :started, verification_state: verification_state_value(:verification_failed), verification_failure: 'foo')
-      create(registry_class_factory, :failed, verification_state: verification_state_value(:verification_failed), verification_failure: 'foo')
-      create(registry_class_factory, :synced, verification_state: verification_state_value(:verification_pending))
-      create(registry_class_factory, :synced, verification_state: verification_state_value(:verification_started))
-      create(registry_class_factory, :synced, verification_state: verification_state_value(:verification_succeeded), verification_checksum: 'abc123')
-      # rubocop:enable Rails/SaveBang
+      it 'returns IDs of rows which are synced and have failed verification' do
+        expect(described_class.verification_failed_batch(batch_size: 4)).to match_array([subject.model_record_id])
+      end
 
-      expect(described_class.verification_failed_batch(batch_size: 4)).to match_array([subject.model_record_id])
+      it 'excludes rows which are not synced or have not failed verification' do
+        # rubocop:disable Rails/SaveBang
+        create(registry_class_factory, verification_state: verification_state_value(:verification_failed), verification_failure: 'foo')
+        create(registry_class_factory, :started, verification_state: verification_state_value(:verification_failed), verification_failure: 'foo')
+        create(registry_class_factory, :failed, verification_state: verification_state_value(:verification_failed), verification_failure: 'foo')
+        create(registry_class_factory, :synced, verification_state: verification_state_value(:verification_pending))
+        create(registry_class_factory, :synced, verification_state: verification_state_value(:verification_started))
+        create(registry_class_factory, :synced, verification_state: verification_state_value(:verification_succeeded), verification_checksum: 'abc123')
+        # rubocop:enable Rails/SaveBang
+
+        expect(described_class.verification_failed_batch(batch_size: 4)).to match_array([subject.model_record_id])
+      end
+
+      it 'marks verification as started' do
+        described_class.verification_failed_batch(batch_size: 4)
+
+        expect(subject.reload.verification_started?).to be_truthy
+        expect(subject.verification_started_at).to be_present
+      end
     end
 
-    it 'marks verification as started' do
-      described_class.verification_failed_batch(batch_size: 4)
+    context 'when verification_retry_at is in the future' do
+      it 'does not return the row which failed verification' do
+        subject.update!(verification_retry_at: 1.minute.from_now)
 
-      expect(subject.reload.verification_started?).to be_truthy
-      expect(subject.verification_started_at).to be_present
+        expect(subject.class.verification_failed_batch(batch_size: 4)).not_to include(subject.model_record_id)
+      end
     end
   end