Merge branch...

Merge branch '5196-geo-include-keep-around-and-other-references-in-the-checksum-calculation' into 'master'

Resolve "Geo: Include keep-around and other references in the checksum calculation"

Closes #5196

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

doc/administration/geo/disaster_recovery/background_verification.md

@@ -80,11 +80,11 @@ green, pending work in grey, and failures in red.
 # 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 theirs values. Right now the checksum only includes `heads`
-and `tags`. We should include all references ([issue #5196][ee-5196]), including
-GitLab-specific references to ensure true consistency. If two nodes have the
-same checksum, then they definitely hold the same data. We compute the checksum
-for every node after every update to make sure that they are all in sync.
+Git references and their values. The checksum includes `HEAD`, `heads`, `tags`,
+`notes`, and GitLab-specific references to ensure true consistency. If two nodes
+have the same checksum, then they definitely hold the same references. We compute
+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
 

ee/app/services/geo/repository_verification_primary_service.rb

@@ -9,6 +9,8 @@ module Geo
     def execute
       verify_checksum(:repository, project.repository)
       verify_checksum(:wiki, project.wiki.repository)
+
+      Geo::ResetChecksumEventStore.new(project).create!
     end
 
     private

ee/changelogs/unreleased/5196-geo-include-keep-around-and-other-references-in-the-checksum-calculation.yml

+---
+title: Geo - Include keep-around and other Gitlab-specific references in the checksum
+  calculation
+merge_request: 7367
+author:
+type: changed

ee/db/post_migrate/20180914195058_schedule_repository_checksum_cleanup.rb

+# frozen_string_literal: true
+
+class ScheduleRepositoryChecksumCleanup < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  MIGRATION = 'ResetChecksumFromProjectRepositoryStates'.freeze
+  BATCH_SIZE = 10_000
+  DELAY_INTERVAL = 5.minutes.to_i
+
+  class Project < ActiveRecord::Base
+    self.table_name = 'projects'
+
+    include ::EachBatch
+  end
+
+  class ProjectRepositoryState < ActiveRecord::Base
+    self.table_name = 'project_repository_states'
+  end
+
+  disable_ddl_transaction!
+
+  def up
+    # This background migration should only affect EE installations,
+    # which has entries in the project_repository_states table.
+    return unless ProjectRepositoryState.exists?
+
+    now = Time.now
+
+    projects_to_cleanup =
+      Project
+        .where(Project.arel_table[:last_repository_updated_at].lteq(now))
+
+    projects_to_cleanup.each_batch(of: BATCH_SIZE) do |relation, index|
+      range = relation.pluck('MIN(id)', 'MAX(id)').first
+      delay = index * DELAY_INTERVAL
+
+      BackgroundMigrationWorker.perform_in(delay, MIGRATION, range)
+    end
+  end
+
+  def down
+    # no-op
+  end
+end

ee/lib/gitlab/background_migration/reset_checksum_from_project_repository_states.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    class ResetChecksumFromProjectRepositoryStates
+      def perform(start_id, stop_id)
+        ProjectRepositoryState
+          .where(project_id: start_id..stop_id)
+          .update_all(
+            repository_verification_checksum: nil,
+            wiki_verification_checksum: nil,
+            last_repository_verification_failure: nil,
+            last_wiki_verification_failure: nil,
+            repository_retry_at: nil,
+            wiki_retry_at: nil,
+            repository_retry_count: nil,
+            wiki_retry_count: nil
+          )
+      end
+    end
+  end
+end

ee/spec/factories/project_repository_states.rb

@@ -5,6 +5,8 @@ FactoryBot.define do
     trait :repository_failed do
       repository_verification_checksum nil
       last_repository_verification_failure 'Could not calculate the checksum'
+      repository_retry_count 1
+      repository_retry_at { Time.now + 5.minutes }
     end
 
     trait :repository_outdated do
@@ -15,11 +17,15 @@ FactoryBot.define do
     trait :repository_verified do
       repository_verification_checksum 'f079a831cab27bcda7d81cd9b48296d0c3dd92ee'
       last_repository_verification_failure nil
+      repository_retry_count nil
+      repository_retry_at nil
     end
 
     trait :wiki_failed do
       wiki_verification_checksum nil
       last_wiki_verification_failure 'Could not calculate the checksum'
+      wiki_retry_count 1
+      wiki_retry_at { Time.now + 5.minutes }
     end
 
     trait :wiki_outdated do
@@ -30,6 +36,8 @@ FactoryBot.define do
     trait :wiki_verified do
       wiki_verification_checksum 'e079a831cab27bcda7d81cd9b48296d0c3dd92ef'
       last_wiki_verification_failure nil
+      wiki_retry_count nil
+      wiki_retry_at nil
     end
   end
 end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::ResetChecksumFromProjectRepositoryStates, :migration, schema: 20180914195058 do
+  describe '#perform' do
+    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)
+
+      subject.perform(repository_state_1.project_id, repository_state_2.project_id)
+
+      expect(repository_state_1.reload).to have_attributes(
+        repository_verification_checksum: be_nil,
+        wiki_verification_checksum: be_nil,
+        last_repository_verification_failure: be_nil,
+        last_wiki_verification_failure: be_nil,
+        repository_retry_at: be_nil,
+        wiki_retry_at: be_nil,
+        repository_retry_count: be_nil,
+        wiki_retry_count: be_nil
+      )
+
+      expect(repository_state_2.reload).to have_attributes(
+        repository_verification_checksum: be_nil,
+        wiki_verification_checksum: be_nil,
+        last_repository_verification_failure: be_nil,
+        last_wiki_verification_failure: be_nil,
+        repository_retry_at: be_nil,
+        wiki_retry_at: be_nil,
+        repository_retry_count: be_nil,
+        wiki_retry_count: be_nil
+      )
+
+      expect(repository_state_3.reload).to have_attributes(
+        repository_verification_checksum: be_present,
+        wiki_verification_checksum: be_present
+      )
+    end
+  end
+end

ee/spec/services/geo/repository_verification_primary_service_spec.rb

 require 'spec_helper'
 
 describe Geo::RepositoryVerificationPrimaryService do
+  include EE::GeoHelpers
+
   let(:project) { create(:project) }
   let(:repository) { double(checksum: 'f123') }
   let(:wiki) { double(checksum: 'e321') }
@@ -142,6 +144,24 @@ describe Geo::RepositoryVerificationPrimaryService do
       )
     end
 
+    context 'when running on a primary node' do
+      before do
+        stub_primary_node
+      end
+
+      it 'does not create a Geo::ResetChecksumEvent event if there are no secondary nodes' do
+        allow(Gitlab::Geo).to receive(:secondary_nodes) { [] }
+
+        expect { subject.execute }.not_to change(Geo::ResetChecksumEvent, :count)
+      end
+
+      it 'creates a Geo::ResetChecksumEvent event' do
+        allow(Gitlab::Geo).to receive(:secondary_nodes) { [build(:geo_node)] }
+
+        expect { subject.execute }.to change(Geo::ResetChecksumEvent, :count).by(1)
+      end
+    end
+
     context 'when checksum calculation fails' do
       before do
         stub_project_repository(project, repository)