Add verification state machine

This will help to ensure we encapsulate verification state in the model
(or later, in the registry, when we implement verification on
secondaries).

The verification_started_at field will make it much easier to properly
manage concurrent verification jobs.

changelogs/unreleased/mk-add-verification-state-machine.yml

+---
+title: 'Geo: Add verification state machine fields to package files table'
+merge_request: 47260
+author:
+type: added

db/migrate/20201112173532_add_verification_state_to_package_files.rb

+# frozen_string_literal: true
+
+class AddVerificationStateToPackageFiles < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :packages_package_files, :verification_state, :integer, default: 0, limit: 2, null: false
+    add_column :packages_package_files, :verification_started_at, :datetime_with_timezone
+  end
+end

db/migrate/20201112173911_add_index_on_verification_state_on_package_files.rb

+# frozen_string_literal: true
+
+class AddIndexOnVerificationStateOnPackageFiles < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_packages_package_files_on_verification_state'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :packages_package_files, :verification_state, name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :packages_package_files, INDEX_NAME
+  end
+end

db/schema_migrations/20201112173532

+d88a47333a4cc2b6c4aafa817c766822728d14b947a195c7c40b39e0c8b41610
\ No newline at end of file

db/schema_migrations/20201112173911

+dde78a32d53a695e82b44574458b3670dce4803ffc6f34a1216f3671cca470ed
\ No newline at end of file

db/structure.sql

@@ -14483,6 +14483,8 @@ CREATE TABLE packages_package_files (
     verification_failure character varying(255),
     verification_retry_count integer,
     verification_checksum bytea,
+    verification_state smallint DEFAULT 0 NOT NULL,
+    verification_started_at timestamp with time zone,
     CONSTRAINT check_4c5e6bb0b3 CHECK ((file_store IS NOT NULL))
 );
 
@@ -21451,6 +21453,8 @@ CREATE INDEX index_packages_package_files_on_file_store ON packages_package_file
 
 CREATE INDEX index_packages_package_files_on_package_id_and_file_name ON packages_package_files USING btree (package_id, file_name);
 
+CREATE INDEX index_packages_package_files_on_verification_state ON packages_package_files USING btree (verification_state);
+
 CREATE INDEX index_packages_packages_on_creator_id ON packages_packages USING btree (creator_id);
 
 CREATE INDEX index_packages_packages_on_id_and_created_at ON packages_packages USING btree (id, created_at);

doc/development/geo/framework.md

@@ -393,6 +393,8 @@ can track verification state.
 
      def change
        change_table(:widgets) do |t|
+         t.integer :verification_state, default: 0, limit: 2, null: false
+         t.column :verification_started_at, :datetime_with_timezone
          t.integer :verification_retry_count, limit: 2
          t.column :verification_retry_at, :datetime_with_timezone
          t.column :verified_at, :datetime_with_timezone
@@ -431,13 +433,12 @@ can track verification state.
    end
    ```
 
-1. Add a partial index on `verification_failure` and `verification_checksum` to ensure
-   re-verification can be performed efficiently:
+1. Add an index on `verification_state` to ensure verification can be performed efficiently:
 
    ```ruby
    # frozen_string_literal: true
 
-   class AddVerificationFailureIndexToWidgets < ActiveRecord::Migration[6.0]
+   class AddVerificationStateIndexToWidgets < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
      DOWNTIME = false
@@ -445,22 +446,28 @@ can track verification state.
      disable_ddl_transaction!
 
      def up
-       add_concurrent_index :widgets, :verification_failure, where: "(verification_failure IS NOT NULL)", name: "widgets_verification_failure_partial"
-       add_concurrent_index :widgets, :verification_checksum, where: "(verification_checksum IS NOT NULL)", name: "widgets_verification_checksum_partial"
+       add_concurrent_index :widgets, :verification_state, name: "index_widgets_on_verification_state"
      end
 
      def down
-       remove_concurrent_index :widgets, :verification_failure
-       remove_concurrent_index :widgets, :verification_checksum
+       remove_concurrent_index :widgets, :verification_state
      end
    end
    ```
 
+1. Add the `Gitlab::Geo::VerificationState` concern to the `widget` model if it is not already included in `Gitlab::Geo::ReplicableModel`:
+
+   ```ruby
+   class Widget < ApplicationRecord
+     ...
+     include ::Gitlab::Geo::VerificationState
+     ...
+   end
+   ```
+
 ##### Option 2: Create a separate `widget_states` table with verification state fields
 
-1. Create a `widget_states` table and add a partial index on `verification_failure` and
-   `verification_checksum` to ensure re-verification can be performed efficiently. Order
-   the columns according to [the guidelines](../ordering_table_columns.md):
+1. Create a `widget_states` table and add an index on `verification_state` to ensure verification can be performed efficiently. Order the columns according to [the guidelines](../ordering_table_columns.md):
 
    ```ruby
    # frozen_string_literal: true
@@ -477,14 +484,15 @@ can track verification state.
          with_lock_retries do
            create_table :widget_states, id: false do |t|
              t.references :widget, primary_key: true, null: false, foreign_key: { on_delete: :cascade }
+             t.integer :verification_state, default: 0, limit: 2, null: false
+             t.column :verification_started_at, :datetime_with_timezone
              t.datetime_with_timezone :verification_retry_at
              t.datetime_with_timezone :verified_at
              t.integer :verification_retry_count, limit: 2
              t.binary :verification_checksum, using: 'verification_checksum::bytea'
              t.text :verification_failure
 
-             t.index :verification_failure, where: "(verification_failure IS NOT NULL)", name: "widgets_verification_failure_partial"
-             t.index :verification_checksum, where: "(verification_checksum IS NOT NULL)", name: "widgets_verification_checksum_partial"
+             t.index :verification_state, name: "index_widget_states_on_verification_state"
            end
          end
        end
@@ -498,6 +506,20 @@ can track verification state.
    end
    ```
 
+1. Add the following lines to the `widget_state.rb` model:
+
+   ```ruby
+   class WidgetState < ApplicationRecord
+     ...
+     self.primary_key = :widget_id
+
+     include ::Gitlab::Geo::VerificationState
+
+     belongs_to :widget, inverse_of: :widget_state
+     ...
+   end
+   ```
+
 1. Add the following lines to the `widget` model:
 
    ```ruby

ee/app/models/ee/packages/package_file.rb

@@ -7,6 +7,8 @@ module EE
 
       prepended do
         include ::Gitlab::Geo::ReplicableModel
+        include ::Gitlab::Geo::VerificationState
+
         with_replicator Geo::PackageFileReplicator
       end
 

ee/lib/gitlab/geo/replicable_model.rb

@@ -5,7 +5,6 @@ module Gitlab
     module ReplicableModel
       extend ActiveSupport::Concern
       include Checksummable
-      include ::ShaAttribute
 
       included do
         # If this hook turns out not to apply to all Models, perhaps we should extract a `ReplicableBlobModel`
@@ -15,8 +14,6 @@ module Gitlab
         scope :checksummed, -> { where.not(verification_checksum: nil) }
         scope :checksum_failed, -> { where.not(verification_failure: nil) }
         scope :available_replicables, -> { all }
-
-        sha_attribute :verification_checksum
       end
 
       class_methods do

ee/lib/gitlab/geo/verification_state.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Geo
+    # This concern is included on ActiveRecord classes to manage their
+    # verification fields. This concern does not handle how verification is
+    # performed.
+    #
+    # This is a separate concern from Gitlab::Geo::ReplicableModel because e.g.
+    # MergeRequestDiff stores its verification state in a separate table with
+    # the association to MergeRequestDiffDetail.
+    module VerificationState
+      extend ActiveSupport::Concern
+      include ::ShaAttribute
+      include Delay
+
+      VERIFICATION_STATE_VALUES = {
+        verification_pending: 0,
+        verification_started: 1,
+        verification_succeeded: 2,
+        verification_failed: 3
+      }.freeze
+      VERIFICATION_TIMEOUT = 8.hours
+
+      included do
+        sha_attribute :verification_checksum
+
+        # rubocop:disable CodeReuse/ActiveRecord
+        scope :verification_pending, -> { with_verification_state(:verification_pending) }
+        scope :verification_started, -> { with_verification_state(:verification_started) }
+        scope :verification_succeeded, -> { with_verification_state(:verification_succeeded) }
+        scope :verification_failed, -> { with_verification_state(:verification_failed) }
+        scope :checksummed, -> { where.not(verification_checksum: nil) }
+        scope :not_checksummed, -> { where(verification_checksum: nil) }
+        scope :never_attempted_verification, -> { verification_pending.where(verification_started_at: nil) }
+        scope :needs_verification_again, -> { verification_pending.where.not(verification_started_at: nil).or(verification_failed) }
+        scope :verification_timed_out, -> { verification_started.where("verification_started_at < ?", VERIFICATION_TIMEOUT.ago) }
+        scope :needs_verification, -> { verification_pending.or(verification_failed) }
+        # rubocop:enable CodeReuse/ActiveRecord
+
+        state_machine :verification_state, initial: :verification_pending do
+          state :verification_pending, value: VERIFICATION_STATE_VALUES[:verification_pending]
+          state :verification_started, value: VERIFICATION_STATE_VALUES[:verification_started]
+          state :verification_succeeded, value: VERIFICATION_STATE_VALUES[:verification_succeeded]
+          state :verification_failed, value: VERIFICATION_STATE_VALUES[:verification_failed] do
+            validates :verification_failure, presence: true
+          end
+
+          before_transition any => :verification_started do |instance, _|
+            instance.verification_started_at = Time.current
+            instance.verification_failure = nil
+          end
+
+          before_transition any => :verification_pending do |instance, _|
+            instance.verification_retry_count = 0
+            instance.verification_retry_at = nil
+            instance.verification_failure = nil
+          end
+
+          before_transition any => :verification_failed do |instance, _|
+            instance.verification_retry_count += 1
+            instance.verification_retry_at = instance.next_retry_time(instance.verification_retry_count)
+          end
+
+          before_transition any => :verification_succeeded do |instance, _|
+            instance.verification_retry_count = 0
+            instance.verification_retry_at = nil
+            instance.verification_failure = nil
+          end
+
+          event :verification_started do
+            transition [:verification_pending, :verification_succeeded, :verification_failed] => :verification_started
+          end
+
+          event :verification_succeeded do
+            transition verification_started: :verification_succeeded
+          end
+
+          event :verification_failed do
+            transition verification_started: :verification_failed
+          end
+
+          event :verification_pending do
+            transition [:verification_started, :verification_succeeded, :verification_failed] => :verification_pending
+          end
+        end
+      end
+
+      # @param [String] message error information
+      # @param [StandardError] error exception
+      def set_verification_failure(message, error = nil)
+        self.verification_failure = message
+        self.verification_failure += ": #{error.message}" if error.respond_to?(:message)
+      end
+    end
+  end
+end