Merge branch '7347-geo-constantly-reverify-repositories' into 'master'

Geo: Constantly reverify repositories

Closes #7347

See merge request gitlab-org/gitlab-ee!8550

db/schema.rb

@@ -1238,6 +1238,7 @@ ActiveRecord::Schema.define(version: 20181204135932) do
     t.string "selective_sync_type"
     t.text "selective_sync_shards"
     t.integer "verification_max_capacity", default: 100, null: false
+    t.integer "minimum_reverification_interval", default: 7, null: false
     t.index ["access_key"], name: "index_geo_nodes_on_access_key", using: :btree
     t.index ["primary"], name: "index_geo_nodes_on_primary", using: :btree
     t.index ["url"], name: "index_geo_nodes_on_url", unique: true, using: :btree
@@ -2175,8 +2176,12 @@ ActiveRecord::Schema.define(version: 20181204135932) do
     t.datetime_with_timezone "wiki_retry_at"
     t.integer "repository_retry_count"
     t.integer "wiki_retry_count"
+    t.datetime_with_timezone "last_repository_verification_ran_at"
+    t.datetime_with_timezone "last_wiki_verification_ran_at"
     t.index ["last_repository_verification_failure"], name: "idx_repository_states_on_repository_failure_partial", where: "(last_repository_verification_failure IS NOT NULL)", using: :btree
     t.index ["last_wiki_verification_failure"], name: "idx_repository_states_on_wiki_failure_partial", where: "(last_wiki_verification_failure IS NOT NULL)", using: :btree
+    t.index ["project_id", "last_repository_verification_ran_at"], name: "idx_repository_states_on_last_repository_verification_ran_at", where: "((repository_verification_checksum IS NOT NULL) AND (last_repository_verification_failure IS NULL))", using: :btree
+    t.index ["project_id", "last_wiki_verification_ran_at"], name: "idx_repository_states_on_last_wiki_verification_ran_at", where: "((wiki_verification_checksum IS NOT NULL) AND (last_wiki_verification_failure IS NULL))", using: :btree
     t.index ["project_id"], name: "idx_repository_states_outdated_checksums", where: "(((repository_verification_checksum IS NULL) AND (last_repository_verification_failure IS NULL)) OR ((wiki_verification_checksum IS NULL) AND (last_wiki_verification_failure IS NULL)))", using: :btree
     t.index ["project_id"], name: "index_project_repository_states_on_project_id", unique: true, using: :btree
   end

doc/administration/geo/disaster_recovery/background_verification.md

@@ -24,10 +24,9 @@ these failures, so you should follow [these instructions][reset-verification].
 If verification is lagging significantly behind replication, consider giving
 the node more time before scheduling a planned failover.
 
-### Disabling or enabling the automatic background verification
+## Disabling or enabling the automatic background verification
 
-The following commands are to be issues in a Rails console on
-the **primary**:
+Run the following commands in a Rails console on the **primary** node:
 
 ```sh
 # Omnibus GitLab
@@ -38,25 +37,25 @@ cd /home/git/gitlab
 sudo -u git -H bin/rails console RAILS_ENV=production
 ```
 
-**To check if automatic background verification is enabled:**
+To check if automatic background verification is enabled:
 
 ```ruby
 Gitlab::Geo.repository_verification_enabled?
 ```
 
-**To disable automatic background verification:**
+To disable automatic background verification:
 
 ```ruby
 Feature.disable('geo_repository_verification')
 ```
 
-**To enable automatic background verification:**
+To enable automatic background verification:
 
 ```ruby
 Feature.enable('geo_repository_verification')
 ```
 
-# Repository verification
+## Repository verification
 
 Navigate to the **Admin Area > Geo** dashboard on the **primary** node and expand
 the **Verification information** tab for that node to view automatic checksumming
@@ -72,7 +71,7 @@ green, pending work in grey, and failures in red.
 
 ![Verification status](img/verification-status-secondary.png)
 
-# Using checksums to compare Geo nodes
+## Using checksums to compare Geo nodes
 
 To check the health of Geo secondary nodes, we use a checksum over the list of
 Git references and their values. The checksum includes `HEAD`, `heads`, `tags`,
@@ -81,7 +80,49 @@ have the same checksum, then they definitely hold the same references. We comput
 the checksum for every node after every update to make sure that they are all
 in sync.
 
-# Reset verification for projects where verification has failed
+## Repository re-verification
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/8550) in GitLab Enterprise Edition 11.6. Available in [GitLab Premium](https://about.gitlab.com/pricing/).
+
+Due to bugs or transient infrastructure failures, it is possible for Git
+repositories to change unexpectedly without being marked for verification.
+Geo constantly reverifies the repositories to ensure the integrity of the
+data. The default and recommended re-verification interval is 7 days, though
+an interval as short as 1 day can be set. Shorter intervals reduce risk but
+increase load and vice versa.
+
+Navigate to the **Admin Area > Geo** dashboard on the **primary** node, and
+click the **Edit** button for the **primary** node to customize the minimum
+re-verification interval:
+
+![Re-verification interval](img/reverification-interval.png)
+
+The automatic background re-verification is enabled by default, but you can
+disable if you need. Run the following commands in a Rails console on the
+**primary** node:
+
+```sh
+# Omnibus GitLab
+gitlab-rails console
+
+# Installation from source
+cd /home/git/gitlab
+sudo -u git -H bin/rails console RAILS_ENV=production
+```
+
+To disable automatic background re-verification:
+
+```ruby
+Feature.disable('geo_repository_reverification')
+```
+
+To enable automatic background re-verification:
+
+```ruby
+Feature.enable('geo_repository_reverification')
+```
+
+## Reset verification for projects where verification has failed
 
 Geo actively try to correct verification failures marking the repository to
 be resynced with a backoff period. If you want to reset them manually, this
@@ -116,7 +157,7 @@ sudo gitlab-rake geo:verification:wiki:reset
 sudo -u git -H bundle exec rake geo:verification:wiki:reset RAILS_ENV=production
 ```
 
-# Current limitations
+## Current limitations
 
 Until [issue #5064][ee-5064] is completed, background verification doesn't cover
 CI job artifacts and traces, LFS objects, or user uploads in file storage.

ee/app/assets/javascripts/pages/admin/geo/nodes/shared/init_form.js

@@ -3,11 +3,12 @@ import { s__ } from '~/locale';
 import '~/flash';
 import Api from '~/api';
 
-const onPrimaryCheckboxChange = function onPrimaryCheckboxChange(e, $namespaces) {
+const onPrimaryCheckboxChange = function onPrimaryCheckboxChange(e, $namespaces, $reverification) {
   const $namespacesSelect = $('.select2', $namespaces);
 
   $namespacesSelect.select2('data', null);
   $namespaces.toggleClass('hidden', e.currentTarget.checked);
+  $reverification.toggleClass('hidden', !e.currentTarget.checked);
 };
 
 const onSelectiveSyncTypeChange = function onSelectiveSyncTypeChange(e, $byNamespaces, $byShards) {
@@ -18,13 +19,14 @@ const onSelectiveSyncTypeChange = function onSelectiveSyncTypeChange(e, $byNames
 export default function geoNodeForm() {
   const $container = $('.js-geo-node-form');
   const $namespaces = $('.js-hide-if-geo-primary', $container);
+  const $reverification = $('.js-hide-if-geo-secondary', $container);
   const $primaryCheckbox = $('input[type="checkbox"]', $container);
   const $selectiveSyncTypeSelect = $('.js-geo-node-selective-sync-type', $container);
   const $select2Dropdown = $('.js-geo-node-namespaces', $container);
   const $syncByNamespaces = $('.js-sync-by-namespace', $container);
   const $syncByShards = $('.js-sync-by-shard', $container);
 
-  $primaryCheckbox.on('change', e => onPrimaryCheckboxChange(e, $namespaces));
+  $primaryCheckbox.on('change', e => onPrimaryCheckboxChange(e, $namespaces, $reverification));
 
   $selectiveSyncTypeSelect.on('change', e =>
     onSelectiveSyncTypeChange(e, $syncByNamespaces, $syncByShards),

ee/app/controllers/admin/geo/nodes_controller.rb

@@ -18,7 +18,7 @@ class Admin::Geo::NodesController < Admin::ApplicationController
   # rubocop: enable CodeReuse/ActiveRecord
 
   def create
-    @node = Geo::NodeCreateService.new(geo_node_params).execute
+    @node = ::Geo::NodeCreateService.new(geo_node_params).execute
 
     if @node.persisted?
       redirect_to admin_geo_nodes_path, notice: 'Node was successfully created.'
@@ -34,7 +34,7 @@ class Admin::Geo::NodesController < Admin::ApplicationController
   end
 
   def update
-    if Geo::NodeUpdateService.new(@node, geo_node_params).execute
+    if ::Geo::NodeUpdateService.new(@node, geo_node_params).execute
       redirect_to admin_geo_nodes_path, notice: 'Node was successfully updated.'
     else
       render :edit
@@ -52,7 +52,8 @@ class Admin::Geo::NodesController < Admin::ApplicationController
       :namespace_ids,
       :repos_max_capacity,
       :files_max_capacity,
-      :verification_max_capacity
+      :verification_max_capacity,
+      :minimum_reverification_interval
     )
   end
 

ee/app/finders/geo/repository_verification_finder.rb

@@ -29,9 +29,9 @@ module Geo
     # rubocop: enable CodeReuse/ActiveRecord
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def find_outdated_projects(batch_size:)
-      query = build_query_to_find_outdated_projects(batch_size: batch_size)
-      cte   = Gitlab::SQL::CTE.new(:outdated_projects, query)
+    def find_recently_updated_projects(batch_size:)
+      query = build_query_to_find_recently_updated_projects(batch_size: batch_size)
+      cte   = Gitlab::SQL::CTE.new(:recently_updated_projects, query)
 
       Project.with(cte.to_arel)
              .from(cte.alias_to(projects_table))
@@ -40,19 +40,26 @@ module Geo
     # rubocop: enable CodeReuse/ActiveRecord
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def find_unverified_projects(batch_size:)
+    def find_never_verified_projects(batch_size:)
       relation =
         Project.select(:id)
          .with_route
          .joins(left_join_repository_state)
-         .where(repository_never_verified)
+         .where(repository_state_table[:project_id].eq(nil))
          .limit(batch_size)
 
-      relation = apply_shard_restriction(relation) if shard_name.present?
-      relation
+      apply_shard_restriction(relation)
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
+    def find_reverifiable_repositories(interval:, batch_size:)
+      build_query_to_find_reverifiable_projects(type: :repository, interval: interval, batch_size: batch_size)
+    end
+
+    def find_reverifiable_wikis(interval:, batch_size:)
+      build_query_to_find_reverifiable_projects(type: :wiki, interval: interval, batch_size: batch_size)
+    end
+
     def count_verified_repositories
       Project.verified_repos.count
     end
@@ -84,22 +91,50 @@ module Geo
               .and(repository_state_table["last_#{type}_verification_failure"].not_eq(nil))
           ).take(batch_size)
 
-      query = apply_shard_restriction(query) if shard_name.present?
-      query
+      apply_shard_restriction(query)
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def build_query_to_find_outdated_projects(batch_size:)
+    def build_query_to_find_recently_updated_projects(batch_size:)
+      repository_recently_updated =
+        repository_state_table[:repository_verification_checksum].eq(nil)
+          .and(repository_state_table[:last_repository_verification_failure].eq(nil))
+
+      wiki_recently_updated =
+        repository_state_table[:wiki_verification_checksum].eq(nil)
+          .and(repository_state_table[:last_wiki_verification_failure].eq(nil))
+
       query =
         projects_table
           .join(repository_state_table).on(project_id_matcher)
           .project(projects_table[:id], projects_table[:last_repository_updated_at])
-          .where(repository_outdated.or(wiki_outdated))
+          .where(repository_recently_updated.or(wiki_recently_updated))
           .take(batch_size)
 
-      query = apply_shard_restriction(query) if shard_name.present?
-      query
+      apply_shard_restriction(query)
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def build_query_to_find_reverifiable_projects(type:, interval:, batch_size:)
+      verification_succeded =
+        repository_state_table["#{type}_verification_checksum"].not_eq(nil)
+          .and(repository_state_table["last_#{type}_verification_failure"].eq(nil))
+
+      verified_before_interval =
+        repository_state_table["last_#{type}_verification_ran_at"].eq(nil).or(
+          repository_state_table["last_#{type}_verification_ran_at"].lteq(interval))
+
+      # We should prioritize less active projects first because high active
+      # projects have their repositories verified more frequently.
+      query =
+        Project.joins(:repository_state)
+          .where(verification_succeded.and(verified_before_interval))
+          .order(last_repository_updated_at_asc)
+          .limit(batch_size)
+
+      apply_shard_restriction(query)
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
@@ -122,27 +157,15 @@ module Geo
         .join_sources
     end
 
-    def repository_outdated
-      repository_state_table[:repository_verification_checksum].eq(nil)
-        .and(repository_state_table[:last_repository_verification_failure].eq(nil))
-    end
-
-    def wiki_outdated
-      repository_state_table[:wiki_verification_checksum].eq(nil)
-        .and(repository_state_table[:last_wiki_verification_failure].eq(nil))
-    end
-
-    def repository_never_verified
-      repository_state_table[:project_id].eq(nil)
-    end
-
     def last_repository_updated_at_asc
       Gitlab::Database.nulls_last_order('projects.last_repository_updated_at', 'ASC')
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def apply_shard_restriction(relation)
-      relation.where(projects_table[:repository_storage].eq(shard_name))
+    def apply_shard_restriction(query)
+      return query unless shard_name.present?
+
+      query.where(projects_table[:repository_storage].eq(shard_name))
     end
     # rubocop: enable CodeReuse/ActiveRecord
   end

ee/app/models/geo_node.rb

@@ -34,13 +34,14 @@ class GeoNode < ActiveRecord::Base
   validates :repos_max_capacity, numericality: { greater_than_or_equal_to: 0 }
   validates :files_max_capacity, numericality: { greater_than_or_equal_to: 0 }
   validates :verification_max_capacity, numericality: { greater_than_or_equal_to: 0 }
+  validates :minimum_reverification_interval, numericality: { greater_than_or_equal_to: 1 }
 
   validate :check_not_adding_primary_as_secondary, if: :secondary?
 
   after_save :expire_cache!
   after_destroy :expire_cache!
-  before_validation :update_dependents_attributes
 
+  before_validation :update_dependents_attributes
   before_validation :ensure_access_keys!
 
   alias_method :repair, :save # the `update_dependents_attributes` hook will take care of it

ee/app/models/project_repository_state.rb

@@ -4,9 +4,7 @@ class ProjectRepositoryState < ActiveRecord::Base
   include IgnorableColumn
   include ShaAttribute
 
-  ignore_column :last_repository_verification_at
   ignore_column :last_repository_verification_failed
-  ignore_column :last_wiki_verification_at
   ignore_column :last_wiki_verification_failed
 
   sha_attribute :repository_verification_checksum

ee/app/services/geo/repository_verification_primary_service.rb

@@ -33,6 +33,7 @@ module Geo
 
       repository_state.update!(
         "#{type}_verification_checksum" => checksum,
+        "last_#{type}_verification_ran_at" => Time.now,
         "last_#{type}_verification_failure" => failure,
         "#{type}_retry_at" => retry_at,
         "#{type}_retry_count" => retry_count

ee/app/views/admin/geo/nodes/_form.html.haml

@@ -60,3 +60,11 @@
     = form.number_field :verification_max_capacity, class: 'form-control', min: 0
     .form-text.text-muted
       #{ s_('Control the maximum concurrency of verification operations for this Geo node') }
+
+.form-group.row.js-hide-if-geo-secondary{ class: ('hidden' unless geo_node.primary?) }
+  .col-sm-2
+    = form.label :minimum_reverification_interval, s_('Geo|Re-verification interval'), class: 'col-form-label'
+  .col-sm-10
+    = form.number_field :minimum_reverification_interval, class: 'form-control', min: 1
+    .form-text.text-muted
+      #{ s_('Control the minimum interval in days that a repository should be reverified for this primary node') }

ee/app/workers/geo/repository_verification/primary/shard_worker.rb

@@ -44,29 +44,36 @@ module Geo
         end
 
         def load_pending_resources
-          resources = find_unverified_project_ids(batch_size: db_retrieve_batch_size)
+          resources = find_never_verified_project_ids(batch_size: db_retrieve_batch_size)
           remaining_capacity = db_retrieve_batch_size - resources.size
           return resources if remaining_capacity.zero?
 
-          resources += find_outdated_project_ids(batch_size: remaining_capacity)
+          resources += find_recently_updated_project_ids(batch_size: remaining_capacity)
           remaining_capacity = db_retrieve_batch_size - resources.size
           return resources if remaining_capacity.zero?
 
-          resources + find_failed_project_ids(batch_size: remaining_capacity)
+          resources + find_project_ids_to_reverify(batch_size: remaining_capacity)
         end
 
         # rubocop: disable CodeReuse/ActiveRecord
-        def find_unverified_project_ids(batch_size:)
-          finder.find_unverified_projects(batch_size: batch_size).pluck(:id)
+        def find_never_verified_project_ids(batch_size:)
+          finder.find_never_verified_projects(batch_size: batch_size).pluck(:id)
         end
         # rubocop: enable CodeReuse/ActiveRecord
 
         # rubocop: disable CodeReuse/ActiveRecord
-        def find_outdated_project_ids(batch_size:)
-          finder.find_outdated_projects(batch_size: batch_size).pluck(:id)
+        def find_recently_updated_project_ids(batch_size:)
+          finder.find_recently_updated_projects(batch_size: batch_size).pluck(:id)
         end
         # rubocop: enable CodeReuse/ActiveRecord
 
+        def find_project_ids_to_reverify(batch_size:)
+          failed_project_ids = find_failed_project_ids(batch_size: batch_size)
+          reverifiable_project_ids = find_reverifiable_projects_ids(batch_size: batch_size)
+
+          take_batch(failed_project_ids, reverifiable_project_ids, batch_size: batch_size)
+        end
+
         def find_failed_project_ids(batch_size:)
           repositories_ids = find_failed_repositories_ids(batch_size: batch_size)
           wiki_ids = find_failed_wiki_ids(batch_size: batch_size)
@@ -85,6 +92,28 @@ module Geo
           finder.find_failed_wikis(batch_size: batch_size).pluck(:id)
         end
         # rubocop: enable CodeReuse/ActiveRecord
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        def find_reverifiable_projects_ids(batch_size:)
+          return [] unless reverification_enabled?
+
+          jitter   = (minimum_reverification_interval.seconds * rand(15)) / 100
+          interval = minimum_reverification_interval.ago + jitter.seconds
+
+          repository_ids = finder.find_reverifiable_repositories(interval: interval, batch_size: batch_size).pluck(:id)
+          wiki_ids = finder.find_reverifiable_wikis(interval: interval, batch_size: batch_size).pluck(:id)
+
+          take_batch(repository_ids, wiki_ids, batch_size: batch_size)
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
+        def minimum_reverification_interval
+          ::Gitlab::Geo.current_node.minimum_reverification_interval.days
+        end
+
+        def reverification_enabled?
+          ::Feature.enabled?(:geo_repository_reverification, default_enabled: true)
+        end
       end
     end
   end

ee/changelogs/unreleased/7347-geo-constantly-reverify-repositories.yml

+---
+title: 'Geo: Constantly reverify repositories'
+merge_request: 8550
+author:
+type: changed

ee/db/migrate/20181121174028_add_last_verification_columns_to_project_repository_states.rb

+# frozen_string_literal: true
+
+class AddLastVerificationColumnsToProjectRepositoryStates < ActiveRecord::Migration[4.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :project_repository_states, :last_repository_verification_ran_at, :datetime_with_timezone
+    add_column :project_repository_states, :last_wiki_verification_ran_at, :datetime_with_timezone
+  end
+end

ee/db/migrate/20181121175359_add_index_to_last_verification_columns_on_project_repository_states.rb

+# frozen_string_literal: true
+
+class AddIndexToLastVerificationColumnsOnProjectRepositoryStates < ActiveRecord::Migration[4.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  REPOSITORY_INDEX_NAME = 'idx_repository_states_on_last_repository_verification_ran_at'
+  WIKI_INDEX_NAME = 'idx_repository_states_on_last_wiki_verification_ran_at'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(:project_repository_states,
+      [:project_id, :last_repository_verification_ran_at],
+      name: REPOSITORY_INDEX_NAME,
+      where: 'repository_verification_checksum IS NOT NULL AND last_repository_verification_failure IS NULL')
+
+    add_concurrent_index(:project_repository_states,
+      [:project_id, :last_wiki_verification_ran_at],
+      name: WIKI_INDEX_NAME,
+      where: 'wiki_verification_checksum IS NOT NULL AND last_wiki_verification_failure IS NULL')
+  end
+
+  def down
+    remove_concurrent_index_by_name(:project_repository_states, REPOSITORY_INDEX_NAME)
+    remove_concurrent_index_by_name(:project_repository_states, WIKI_INDEX_NAME)
+  end
+end

ee/db/migrate/20181127203117_add_minimum_reverification_interval_to_geo_nodes.rb

+# frozen_string_literal: true
+
+class AddMinimumReverificationIntervalToGeoNodes < ActiveRecord::Migration[4.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default :geo_nodes, :minimum_reverification_interval, :integer, default: 7, allow_null: false
+  end
+
+  def down
+    remove_column :geo_nodes, :minimum_reverification_interval
+  end
+end

ee/spec/factories/geo_nodes.rb

@@ -12,6 +12,8 @@ FactoryBot.define do
 
     trait :primary do
       primary true
+      minimum_reverification_interval 7
+
       url do
         uri = URI.parse("http://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.relative_url_root}")
         uri.port = Gitlab.config.gitlab.port

ee/spec/factories/project_repository_states.rb

@@ -4,6 +4,7 @@ FactoryBot.define do
 
     trait :repository_failed do
       repository_verification_checksum nil
+      last_repository_verification_ran_at { Time.now }
       last_repository_verification_failure 'Could not calculate the checksum'
       repository_retry_count 1
       repository_retry_at { 5.minutes.ago }
@@ -11,11 +12,13 @@ FactoryBot.define do
 
     trait :repository_outdated do
       repository_verification_checksum nil
+      last_repository_verification_ran_at { 1.day.ago }
       last_repository_verification_failure nil
     end
 
     trait :repository_verified do
       repository_verification_checksum 'f079a831cab27bcda7d81cd9b48296d0c3dd92ee'
+      last_repository_verification_ran_at { 1.day.ago }
       last_repository_verification_failure nil
       repository_retry_count nil
       repository_retry_at nil
@@ -23,6 +26,7 @@ FactoryBot.define do
 
     trait :wiki_failed do
       wiki_verification_checksum nil
+      last_wiki_verification_ran_at { Time.now }
       last_wiki_verification_failure 'Could not calculate the checksum'
       wiki_retry_count 1
       wiki_retry_at { 5.minutes.ago }
@@ -30,11 +34,13 @@ FactoryBot.define do
 
     trait :wiki_outdated do
       wiki_verification_checksum nil
+      last_wiki_verification_ran_at { 1.day.ago }
       last_wiki_verification_failure nil
     end
 
     trait :wiki_verified do
       wiki_verification_checksum 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef'
+      last_wiki_verification_ran_at { 1.day.ago }
       last_wiki_verification_failure nil
       wiki_retry_count nil
       wiki_retry_at nil

ee/spec/features/admin/geo/admin_geo_nodes_spec.rb

@@ -38,6 +38,18 @@ describe 'admin Geo Nodes', :js do
       end
     end
 
+    it 'changes re-verification interval field visibility based on primary node checkbox' do
+      expect(page).not_to have_field('Re-verification interval')
+
+      check 'This is a primary node'
+
+      expect(page).to have_field('Re-verification interval')
+
+      uncheck 'This is a primary node'
+
+      expect(page).not_to have_field('Re-verification interval')
+    end
+
     it 'returns an error message when a duplicate primary is added' do
       create(:geo_node, :primary)
 

ee/spec/finders/geo/repository_verification_finder_spec.rb

@@ -17,7 +17,7 @@ describe Geo::RepositoryVerificationFinder, :postgresql do
       expect(subject.find_failed_repositories(batch_size: 10)).to be_empty
     end
 
-    it 'does not return projects where repository verification is outdated' do
+    it 'does not return projects where repository was recently updated' do
       create(:repository_state, :repository_outdated, project: project)
 
       expect(subject.find_failed_repositories(batch_size: 10)).to be_empty
@@ -66,7 +66,7 @@ describe Geo::RepositoryVerificationFinder, :postgresql do
       expect(subject.find_failed_wikis(batch_size: 10)).to be_empty
     end
 
-    it 'does not return projects where wiki verification is outdated' do
+    it 'does not return projects where wiki was recently updated' do
       create(:repository_state, :wiki_outdated, project: project)
 
       expect(subject.find_failed_wikis(batch_size: 10)).to be_empty
@@ -101,45 +101,45 @@ describe Geo::RepositoryVerificationFinder, :postgresql do
     end
   end
 
-  describe '#find_outdated_projects' do
-    it 'returns projects where repository verification is outdated' do
+  describe '#find_recently_updated_projects' do
+    it 'returns projects where repository was recently updated' do
       create(:repository_state, :repository_outdated, project: project)
 
-      expect(subject.find_outdated_projects(batch_size: 10))
+      expect(subject.find_recently_updated_projects(batch_size: 10))
         .to match_array(project)
     end
 
     it 'returns projects where repository verification is pending' do
       create(:repository_state, :wiki_verified, project: project)
 
-      expect(subject.find_outdated_projects(batch_size: 10))
+      expect(subject.find_recently_updated_projects(batch_size: 10))
         .to match_array(project)
     end
 
     it 'does not return projects where repository verification failed' do
       create(:repository_state, :repository_failed, :wiki_verified, project: project)
 
-      expect(subject.find_outdated_projects(batch_size: 10)).to be_empty
+      expect(subject.find_recently_updated_projects(batch_size: 10)).to be_empty
     end
 
-    it 'returns projects where wiki verification is outdated' do
+    it 'returns projects where wiki was recently updated' do
       create(:repository_state, :wiki_outdated, project: project)
 
-      expect(subject.find_outdated_projects(batch_size: 10))
+      expect(subject.find_recently_updated_projects(batch_size: 10))
         .to match_array(project)
     end
 
     it 'returns projects where wiki verification is pending' do
       create(:repository_state, :repository_verified, project: project)
 
-      expect(subject.find_outdated_projects(batch_size: 10))
+      expect(subject.find_recently_updated_projects(batch_size: 10))
         .to match_array(project)
     end
 
     it 'does not return projects where wiki verification failed' do
       create(:repository_state, :repository_verified, :wiki_failed, project: project)
 
-      expect(subject.find_outdated_projects(batch_size: 10)).to be_empty
+      expect(subject.find_recently_updated_projects(batch_size: 10)).to be_empty
     end
 
     it 'returns less active projects first' do
@@ -149,7 +149,7 @@ describe Geo::RepositoryVerificationFinder, :postgresql do
       project.update_column(:last_repository_updated_at, 30.minutes.ago)
       less_active_project.update_column(:last_repository_updated_at, 2.days.ago)
 
-      expect(subject.find_outdated_projects(batch_size: 10)).to eq [less_active_project, project]
+      expect(subject.find_recently_updated_projects(batch_size: 10)).to eq [less_active_project, project]
     end
 
     context 'with shard restriction' do
@@ -161,18 +161,18 @@ describe Geo::RepositoryVerificationFinder, :postgresql do
         create(:repository_state, :repository_outdated, project: project)
         create(:repository_state, :repository_outdated, project: project_other_shard)
 
-        expect(subject.find_outdated_projects(batch_size: 10))
+        expect(subject.find_recently_updated_projects(batch_size: 10))
           .to match_array(project)
       end
     end
   end
 
-  describe '#find_unverified_projects' do
+  describe '#find_never_verified_projects' do
     it 'returns projects that never have been verified' do
       create(:repository_state, :repository_outdated)
       create(:repository_state, :wiki_outdated)
 
-      expect(subject.find_unverified_projects(batch_size: 10))
+      expect(subject.find_never_verified_projects(batch_size: 10))
         .to match_array(project)
     end
 
@@ -183,9 +183,73 @@ describe Geo::RepositoryVerificationFinder, :postgresql do
         project_other_shard = create(:project)
         project_other_shard.update_column(:repository_storage, 'other')
 
-        expect(subject.find_unverified_projects(batch_size: 10))
+        expect(subject.find_never_verified_projects(batch_size: 10))
           .to match_array(project)
       end
     end
   end
+
+  shared_examples 'find reverifiable projects' do |type|
+    let(:failed) { "#{type}_failed".to_sym }
+    let(:verified) { "#{type}_verified".to_sym }
+    let(:outdated) { "#{type}_outdated".to_sym }
+    let(:last_verification_ran_at) { "last_#{type}_verification_ran_at" }
+    let(:finder_method) { "find_reverifiable_#{type.to_s.pluralize}" }
+
+    it "returns projects where #{type} was verified before the minimum re-verification interval" do
+      project_recently_verified = create(:project)
+      create(:repository_state, verified, project: project, last_verification_ran_at => 2.days.ago)
+      create(:repository_state, verified, project: project_recently_verified, last_verification_ran_at => Time.now)
+
+      expect(subject.public_send(finder_method, interval: 1.day.ago, batch_size: 10))
+        .to match_array(project)
+    end
+
+    it "does not return projects where #{type} was recently updated" do
+      create(:repository_state, outdated, project: project, last_verification_ran_at => 2.days.ago)
+
+      expect(subject.public_send(finder_method, interval: 1.day.ago, batch_size: 10))
+        .to be_empty
+    end
+
+    it "does not return projects where #{type} verification failed" do
+      create(:repository_state, failed, project: project, last_verification_ran_at => 2.days.ago)
+
+      expect(subject.public_send(finder_method, interval: 1.day.ago, batch_size: 10))
+        .to be_empty
+    end
+
+    it 'returns less active projects first' do
+      less_active_project = create(:project)
+      create(:repository_state, verified, project: project, last_verification_ran_at => 2.days.ago)
+      create(:repository_state, verified, project: less_active_project, last_verification_ran_at => 2.days.ago)
+      project.update_column(:last_repository_updated_at, 30.minutes.ago)
+      less_active_project.update_column(:last_repository_updated_at, 2.days.ago)
+
+      expect(subject.public_send(finder_method, interval: 1.day.ago, batch_size: 10))
+        .to eq [less_active_project, project]
+    end
+
+    context 'with shard restriction' do
+      subject { described_class.new(shard_name: project.repository_storage) }
+
+      it 'does not return projects on other shards' do
+        project_other_shard = create(:project)
+        project_other_shard.update_column(:repository_storage, 'other')
+        create(:repository_state, verified, project: project, last_verification_ran_at => 2.days.ago)
+        create(:repository_state, verified, project: project_other_shard, last_verification_ran_at => 2.days.ago)
+
+        expect(subject.public_send(finder_method, interval: 1.day.ago, batch_size: 10))
+          .to match_array(project)
+      end
+    end
+  end
+
+  describe '#find_reverifiable_repositories' do
+    it_behaves_like 'find reverifiable projects', :repository
+  end
+
+  describe '#find_reverifiable_wikis' do
+    it_behaves_like 'find reverifiable projects', :wiki
+  end
 end

ee/spec/lib/gitlab/background_migration/reset_checksum_from_project_repository_states_spec.rb

@@ -4,10 +4,42 @@ require 'spec_helper'
 
 describe Gitlab::BackgroundMigration::ResetChecksumFromProjectRepositoryStates, :migration, schema: 20180914195058 do
   describe '#perform' do
+    let(:users) { table(:users) }
+    let(:projects) { table(:projects) }
+    let(:repository_states) { table(:project_repository_states) }
+
+    def create_repository_state(params = {})
+      attrs = {
+        repository_verification_checksum: 'f079a831cab27bcda7d81cd9b48296d0c3dd92ee',
+        last_repository_verification_failure: nil,
+        repository_retry_count: nil,
+        repository_retry_at: nil,
+        wiki_verification_checksum: 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef',
+        last_wiki_verification_failure: nil,
+        wiki_retry_count: nil,
+        wiki_retry_at: nil
+      }.merge(params)
+
+      repository_states.create!(attrs)
+    end
+
     it 'processes all repository states in batch' do
-      repository_state_1 = create(:repository_state, :repository_verified, :wiki_verified)
-      repository_state_2 = create(:repository_state, :repository_failed, :wiki_failed)
-      repository_state_3 = create(:repository_state, :repository_verified, :wiki_verified)
+      users.create!(email: 'test@example.com', projects_limit: 100, username: 'test')
+      projects.create!(id: 1, name: 'project-1', path: 'project-1', visibility_level: 0, namespace_id: 1)
+      projects.create!(id: 2, name: 'project-2', path: 'project-2', visibility_level: 0, namespace_id: 1)
+      projects.create!(id: 3, name: 'project-3', path: 'project-3', visibility_level: 0, namespace_id: 1)
+
+      repository_state_1 = create_repository_state(project_id: 1)
+
+      repository_state_2 = create_repository_state(
+        project_id: 2,
+        wiki_verification_checksum: nil,
+        last_wiki_verification_failure: 'Could not calculate the checksum',
+        wiki_retry_count: 1,
+        wiki_retry_at: Time.now + 5.minutes
+      )
+
+      repository_state_3 = create_repository_state(project_id: 3)
 
       subject.perform(repository_state_1.project_id, repository_state_2.project_id)
 

ee/spec/models/geo_node_spec.rb

@@ -26,6 +26,7 @@ describe GeoNode, type: :model do
     it { is_expected.to validate_numericality_of(:repos_max_capacity).is_greater_than_or_equal_to(0) }
     it { is_expected.to validate_numericality_of(:files_max_capacity).is_greater_than_or_equal_to(0) }
     it { is_expected.to validate_numericality_of(:verification_max_capacity).is_greater_than_or_equal_to(0) }
+    it { is_expected.to validate_numericality_of(:minimum_reverification_interval).is_greater_than_or_equal_to(1) }
   end
 
   context 'default values' do

ee/spec/services/geo/repository_verification_primary_service_spec.rb

@@ -18,8 +18,10 @@ describe Geo::RepositoryVerificationPrimaryService do
 
       expect(project.repository_state).to have_attributes(
         repository_verification_checksum: 'f123',
+        last_repository_verification_ran_at: be_present,
         last_repository_verification_failure: nil,
         wiki_verification_checksum: 'e321',
+        last_wiki_verification_ran_at: be_present,
         last_wiki_verification_failure: nil,
         repository_retry_at: nil,
         repository_retry_count: nil,
@@ -42,8 +44,10 @@ describe Geo::RepositoryVerificationPrimaryService do
 
       expect(repository_state.reload).to have_attributes(
         repository_verification_checksum: 'f123',
+        last_repository_verification_ran_at: be_present,
         last_repository_verification_failure: nil,
         wiki_verification_checksum: 'e321',
+        last_wiki_verification_ran_at: be_present,
         last_wiki_verification_failure: nil,
         repository_retry_at: nil,
         repository_retry_count: nil,
@@ -66,8 +70,10 @@ describe Geo::RepositoryVerificationPrimaryService do
 
       expect(repository_state.reload).to have_attributes(
         repository_verification_checksum: 'f123',
+        last_repository_verification_ran_at: be_present,
         last_repository_verification_failure: nil,
         wiki_verification_checksum: 'e321',
+        last_wiki_verification_ran_at: be_present,
         last_wiki_verification_failure: nil,
         repository_retry_at: nil,
         repository_retry_count: nil,
@@ -83,12 +89,19 @@ describe Geo::RepositoryVerificationPrimaryService do
       create(:repository_state,
         project: project,
         repository_verification_checksum: 'f079a831cab27bcda7d81cd9b48296d0c3dd92ee',
-        wiki_verification_checksum: 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef')
+        last_repository_verification_ran_at: 1.day.ago,
+        wiki_verification_checksum: 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef',
+        last_wiki_verification_ran_at: 1.day.ago)
 
       expect(repository).to receive(:checksum)
       expect(wiki).to receive(:checksum)
 
       subject.execute
+
+      expect(project.repository_state).to have_attributes(
+        last_repository_verification_ran_at: be_within(100.seconds).of(Time.now),
+        last_wiki_verification_ran_at: be_within(100.seconds).of(Time.now)
+      )
     end
 
     it 'calculates the wiki checksum even when wiki is not enabled for project' do
@@ -101,8 +114,10 @@ describe Geo::RepositoryVerificationPrimaryService do
 
       expect(project.repository_state).to have_attributes(
         repository_verification_checksum: 'f123',
+        last_repository_verification_ran_at: be_present,
         last_repository_verification_failure: nil,
         wiki_verification_checksum: 'e321',
+        last_wiki_verification_ran_at: be_present,
         last_wiki_verification_failure: nil,
         repository_retry_at: nil,
         repository_retry_count: nil,
@@ -116,8 +131,10 @@ describe Geo::RepositoryVerificationPrimaryService do
 
       expect(project.repository_state).to have_attributes(
         repository_verification_checksum: '0000000000000000000000000000000000000000',
+        last_repository_verification_ran_at: be_present,
         last_repository_verification_failure: nil,
         wiki_verification_checksum: '0000000000000000000000000000000000000000',
+        last_wiki_verification_ran_at: be_present,
         last_wiki_verification_failure: nil,
         repository_retry_at: nil,
         repository_retry_count: nil,
@@ -134,8 +151,10 @@ describe Geo::RepositoryVerificationPrimaryService do
 
       expect(project_broken_repo.repository_state).to have_attributes(
         repository_verification_checksum: '0000000000000000000000000000000000000000',
+        last_repository_verification_ran_at: be_present,
         last_repository_verification_failure: nil,
         wiki_verification_checksum: '0000000000000000000000000000000000000000',
+        last_wiki_verification_ran_at: be_present,
         last_wiki_verification_failure: nil,
         repository_retry_at: nil,
         repository_retry_count: nil,
@@ -176,8 +195,10 @@ describe Geo::RepositoryVerificationPrimaryService do
 
         expect(project.repository_state).to have_attributes(
           repository_verification_checksum: nil,
+          last_repository_verification_ran_at: be_present,
           last_repository_verification_failure: 'Something went wrong with repository',
           wiki_verification_checksum: nil,
+          last_wiki_verification_ran_at: be_present,
           last_wiki_verification_failure: 'Something went wrong with wiki',
           repository_retry_at: be_present,
           repository_retry_count: 1,
@@ -197,8 +218,10 @@ describe Geo::RepositoryVerificationPrimaryService do
 
         expect(repository_state.reload).to have_attributes(
           repository_verification_checksum: nil,
+          last_repository_verification_ran_at: be_present,
           last_repository_verification_failure: 'Something went wrong with repository',
           wiki_verification_checksum: nil,
+          last_wiki_verification_ran_at: be_present,
           last_wiki_verification_failure: 'Something went wrong with wiki',
           repository_retry_at: be_within(100.seconds).of(Time.now + 7.days),
           repository_retry_count: 31,

ee/spec/workers/geo/repository_verification/primary/shard_worker_spec.rb

@@ -91,6 +91,60 @@ describe Geo::RepositoryVerification::Primary::ShardWorker, :postgresql, :clean_
       subject.perform(shard_name)
     end
 
+    context 'reverification' do
+      context 'feature geo_repository_reverification flag is enabled' do
+        before do
+          stub_feature_flags(geo_repository_reverification: true)
+        end
+
+        it 'performs Geo::RepositoryVerification::Primary::SingleWorker for projects where repository should be reverified' do
+          project_to_be_reverified = create(:project)
+
+          create(:repository_state, :repository_verified, :wiki_verified,
+            project: project_to_be_reverified, last_repository_verification_ran_at: 10.days.ago)
+
+          expect(primary_singleworker).to receive(:perform_async).with(project_to_be_reverified.id)
+
+          subject.perform(shard_name)
+        end
+
+        it 'performs Geo::RepositoryVerification::Primary::SingleWorker for projects where wiki should be reverified' do
+          project_to_be_reverified = create(:project)
+
+          create(:repository_state, :repository_verified, :wiki_verified,
+            project: project_to_be_reverified, last_wiki_verification_ran_at: 10.days.ago)
+
+          expect(primary_singleworker).to receive(:perform_async).with(project_to_be_reverified.id)
+
+          subject.perform(shard_name)
+        end
+      end
+
+      context 'feature geo_repository_reverification flag is disabled' do
+        before do
+          stub_feature_flags(geo_repository_reverification: false)
+        end
+
+        it 'does not perform Geo::RepositoryVerification::Primary::SingleWorker for projects where repository should be reverified' do
+          create(:repository_state, :repository_verified, :wiki_verified,
+            last_repository_verification_ran_at: 10.days.ago)
+
+          expect(primary_singleworker).not_to receive(:perform_async)
+
+          subject.perform(shard_name)
+        end
+
+        it 'does not Geo::RepositoryVerification::Primary::SingleWorker for projects where wiki should be reverified' do
+          create(:repository_state, :repository_verified, :wiki_verified,
+            last_wiki_verification_ran_at: 10.days.ago)
+
+          expect(primary_singleworker).not_to receive(:perform_async)
+
+          subject.perform(shard_name)
+        end
+      end
+    end
+
     it 'does not perform Geo::RepositoryVerification::Primary::SingleWorker when shard becomes unhealthy' do
       create(:project)
 
@@ -172,6 +226,8 @@ describe Geo::RepositoryVerification::Primary::ShardWorker, :postgresql, :clean_
       let(:project_wiki_unverified) { create(:repository_state).project }
       let(:project_repo_failed_wiki_verified) { create(:repository_state, :repository_failed, :wiki_verified).project }
       let(:project_repo_verified_wiki_failed) { create(:repository_state, :repository_verified, :wiki_failed).project }
+      let(:project_repo_reverify) { create(:repository_state, :repository_verified, :wiki_verified, last_repository_verification_ran_at: 10.days.ago).project }
+      let(:project_wiki_reverify) { create(:repository_state, :repository_verified, :wiki_verified, last_wiki_verification_ran_at: 10.days.ago).project }
 
       it 'handles multiple batches of projects needing verification' do
         expect(primary_singleworker).to receive(:perform_async).with(project_repo_unverified.id).once.and_call_original
@@ -182,7 +238,7 @@ describe Geo::RepositoryVerification::Primary::ShardWorker, :postgresql, :clean_
         end
       end
 
-      it 'handles multiple batches of projects needing verification, including failed repos' do
+      it 'handles multiple batches of projects needing verification' do
         expect(primary_singleworker).to receive(:perform_async).with(project_repo_unverified.id).once.and_call_original
         expect(primary_singleworker).to receive(:perform_async).with(project_wiki_unverified.id).once.and_call_original
         expect(primary_singleworker).to receive(:perform_async).with(project_repo_verified.id).once.and_call_original
@@ -190,8 +246,10 @@ describe Geo::RepositoryVerification::Primary::ShardWorker, :postgresql, :clean_
         expect(primary_singleworker).to receive(:perform_async).with(project_both_failed.id).once.and_call_original
         expect(primary_singleworker).to receive(:perform_async).with(project_repo_failed_wiki_verified.id).once.and_call_original
         expect(primary_singleworker).to receive(:perform_async).with(project_repo_verified_wiki_failed.id).once.and_call_original
+        expect(primary_singleworker).to receive(:perform_async).with(project_repo_reverify.id).once.and_call_original
+        expect(primary_singleworker).to receive(:perform_async).with(project_wiki_reverify.id).once.and_call_original
 
-        8.times do
+        10.times do
           Sidekiq::Testing.inline! { subject.perform(shard_name) }
         end
       end

locale/gitlab.pot

@@ -2417,6 +2417,9 @@ msgstr ""
 msgid "Control the maximum concurrency of verification operations for this Geo node"
 msgstr ""
 
+msgid "Control the minimum interval in days that a repository should be reverified for this primary node"
+msgstr ""
+
 msgid "ConvDev Index"
 msgstr ""
 
@@ -3994,6 +3997,9 @@ msgstr ""
 msgid "Geo|Projects in certain storage shards"
 msgstr ""
 
+msgid "Geo|Re-verification interval"
+msgstr ""
+
 msgid "Geo|Recheck"
 msgstr ""