Merge branch '359855-fixing-mirroring-inconsistency' into 'master'

Automatically fixing mirror tables inconsistencies

See merge request gitlab-org/gitlab!85930

app/services/database/consistency_fix_service.rb

+# frozen_string_literal: true
+
+module Database
+  class ConsistencyFixService
+    def initialize(source_model:, target_model:, sync_event_class:, source_sort_key:, target_sort_key:)
+      @source_model = source_model
+      @target_model = target_model
+      @sync_event_class = sync_event_class
+      @source_sort_key = source_sort_key
+      @target_sort_key = target_sort_key
+    end
+
+    attr_accessor :source_model, :target_model, :sync_event_class, :source_sort_key, :target_sort_key
+
+    def execute(ids:)
+      ids.each do |id|
+        if source_object(id) && target_object(id)
+          create_sync_event_for(id)
+        elsif target_object(id)
+          target_object(id).destroy!
+        end
+      end
+      sync_event_class.enqueue_worker
+    end
+
+    private
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def source_object(id)
+      source_model.find_by(source_sort_key => id)
+    end
+
+    def target_object(id)
+      target_model.find_by(target_sort_key => id)
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+
+    def create_sync_event_for(id)
+      if source_model == Namespace
+        sync_event_class.create!(namespace_id: id)
+      elsif source_model == Project
+        sync_event_class.create!(project_id: id)
+      else
+        raise("Unknown Source Model #{source_model.name}")
+      end
+    end
+  end
+end

app/workers/database/ci_namespace_mirrors_consistency_check_worker.rb

@@ -13,7 +13,7 @@ class CiNamespaceMirrorsConsistencyCheckWorker
     version 1
 
     def perform
-      return if Feature.disabled?(:ci_namespace_mirrors_consistency_check, default_enabled: :yaml)
+      return if Feature.disabled?(:ci_namespace_mirrors_consistency_check)
 
       results = ConsistencyCheckService.new(
         source_model: Namespace,
@@ -22,6 +22,16 @@ def perform
         target_columns: %w[namespace_id traversal_ids]
       ).execute
 
+      if results[:mismatches_details].any?
+        ConsistencyFixService.new(
+          source_model: Namespace,
+          target_model: Ci::NamespaceMirror,
+          sync_event_class: Namespaces::SyncEvent,
+          source_sort_key: :id,
+          target_sort_key: :namespace_id
+        ).execute(ids: results[:mismatches_details].map { |h| h[:id] })
+      end
+
       log_extra_metadata_on_done(:results, results)
     end
   end

app/workers/database/ci_project_mirrors_consistency_check_worker.rb

@@ -13,7 +13,7 @@ class CiProjectMirrorsConsistencyCheckWorker
     version 1
 
     def perform
-      return if Feature.disabled?(:ci_project_mirrors_consistency_check, default_enabled: :yaml)
+      return if Feature.disabled?(:ci_project_mirrors_consistency_check)
 
       results = ConsistencyCheckService.new(
         source_model: Project,
@@ -22,6 +22,16 @@ def perform
         target_columns: %w[project_id namespace_id]
       ).execute
 
+      if results[:mismatches_details].any?
+        ConsistencyFixService.new(
+          source_model: Project,
+          target_model: Ci::ProjectMirror,
+          sync_event_class: Projects::SyncEvent,
+          source_sort_key: :id,
+          target_sort_key: :project_id
+        ).execute(ids: results[:mismatches_details].map { |h| h[:id] })
+      end
+
       log_extra_metadata_on_done(:results, results)
     end
   end

config/feature_flags/development/ci_namespace_mirrors_consistency_check.yml

@@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/356577
 milestone: '14.10'
 type: development
 group: group::sharding
-default_enabled: false
+default_enabled: true

config/feature_flags/development/ci_project_mirrors_consistency_check.yml

@@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/356583
 milestone: '14.10'
 type: development
 group: group::sharding
-default_enabled: false
+default_enabled: true

spec/services/database/consistency_fix_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Database::ConsistencyFixService do
+  describe '#execute' do
+    context 'fixing namespaces inconsistencies' do
+      subject(:consistency_fix_service) do
+        described_class.new(
+          source_model: Namespace,
+          target_model: Ci::NamespaceMirror,
+          sync_event_class: Namespaces::SyncEvent,
+          source_sort_key: :id,
+          target_sort_key: :namespace_id
+        )
+      end
+
+      let(:table) { 'public.namespaces' }
+      let!(:namespace) { create(:namespace) }
+      let!(:namespace_mirror) { Ci::NamespaceMirror.find_by(namespace_id: namespace.id) }
+
+      context 'when both objects exist' do
+        it 'creates a Namespaces::SyncEvent to modify the target object' do
+          expect do
+            consistency_fix_service.execute(ids: [namespace.id])
+          end.to change {
+            Namespaces::SyncEvent.where(namespace_id: namespace.id).count
+          }.by(1)
+        end
+
+        it 'enqueues the worker to process the Namespaces::SyncEvents' do
+          expect(::Namespaces::ProcessSyncEventsWorker).to receive(:perform_async)
+          consistency_fix_service.execute(ids: [namespace.id])
+        end
+      end
+
+      context 'when the source object has been deleted, but not the target' do
+        before do
+          namespace.delete
+        end
+
+        it 'deletes the target object' do
+          expect do
+            consistency_fix_service.execute(ids: [namespace.id])
+          end.to change { Ci::NamespaceMirror.where(namespace_id: namespace.id).count }.by(-1)
+        end
+      end
+    end
+
+    context 'fixing projects inconsistencies' do
+      subject(:consistency_fix_service) do
+        described_class.new(
+          source_model: Project,
+          target_model: Ci::ProjectMirror,
+          sync_event_class: Projects::SyncEvent,
+          source_sort_key: :id,
+          target_sort_key: :project_id
+        )
+      end
+
+      let(:table) { 'public.projects' }
+      let!(:project) { create(:project) }
+      let!(:project_mirror) { Ci::ProjectMirror.find_by(project_id: project.id) }
+
+      context 'when both objects exist' do
+        it 'creates a Projects::SyncEvent to modify the target object' do
+          expect do
+            consistency_fix_service.execute(ids: [project.id])
+          end.to change {
+            Projects::SyncEvent.where(project_id: project.id).count
+          }.by(1)
+        end
+
+        it 'enqueues the worker to process the Projects::SyncEvents' do
+          expect(::Projects::ProcessSyncEventsWorker).to receive(:perform_async)
+          consistency_fix_service.execute(ids: [project.id])
+        end
+      end
+
+      context 'when the source object has been deleted, but not the target' do
+        before do
+          project.delete
+        end
+
+        it 'deletes the target object' do
+          expect do
+            consistency_fix_service.execute(ids: [project.id])
+          end.to change { Ci::ProjectMirror.where(project_id: project.id).count }.by(-1)
+        end
+      end
+    end
+  end
+
+  describe '#create_sync_event_for' do
+    context 'when the source model is Namespace' do
+      let(:namespace) { create(:namespace) }
+
+      let(:service) do
+        described_class.new(
+          source_model: Namespace,
+          target_model: Ci::NamespaceMirror,
+          sync_event_class: Namespaces::SyncEvent,
+          source_sort_key: :id,
+          target_sort_key: :namespace_id
+        )
+      end
+
+      it 'creates a Namespaces::SyncEvent object' do
+        expect do
+          service.send(:create_sync_event_for, namespace.id)
+        end.to change { Namespaces::SyncEvent.where(namespace_id: namespace.id).count }.by(1)
+      end
+    end
+
+    context 'when the source model is Project' do
+      let(:project) { create(:project) }
+
+      let(:service) do
+        described_class.new(
+          source_model: Project,
+          target_model: Ci::ProjectMirror,
+          sync_event_class: Projects::SyncEvent,
+          source_sort_key: :id,
+          target_sort_key: :project_id
+        )
+      end
+
+      it 'creates a Projects::SyncEvent object' do
+        expect do
+          service.send(:create_sync_event_for, project.id)
+        end.to change { Projects::SyncEvent.where(project_id: project.id).count }.by(1)
+      end
+    end
+  end
+
+  context 'when the source model is User' do
+    let(:service) do
+      described_class.new(
+        source_model: User,
+        target_model: Ci::ProjectMirror,
+        sync_event_class: Projects::SyncEvent,
+        source_sort_key: :id,
+        target_sort_key: :project_id
+      )
+    end
+
+    it 'raises an error' do
+      expect do
+        service.send(:create_sync_event_for, 1)
+      end.to raise_error("Unknown Source Model User")
+    end
+  end
+end

spec/workers/database/ci_namespace_mirrors_consistency_check_worker_spec.rb

@@ -62,6 +62,15 @@
         expect(worker).to receive(:log_extra_metadata_on_done).with(:results, expected_result)
         worker.perform
       end
+
+      it 'calls the consistency_fix_service to fix the inconsistencies' do
+        allow_next_instance_of(Database::ConsistencyFixService) do |instance|
+          expect(instance).to receive(:execute).with(
+            ids: [missing_namespace.id]
+          ).and_call_original
+        end
+        worker.perform
+      end
     end
   end
 end

spec/workers/database/ci_project_mirrors_consistency_check_worker_spec.rb

@@ -38,7 +38,7 @@
       before do
         redis_shared_state_cleanup!
         stub_feature_flags(ci_project_mirrors_consistency_check: true)
-        create_list(:project, 10) # This will also create Ci::NameSpaceMirror objects
+        create_list(:project, 10) # This will also create Ci::ProjectMirror objects
         missing_project.delete
 
         allow_next_instance_of(Database::ConsistencyCheckService) do |instance|
@@ -62,6 +62,15 @@
         expect(worker).to receive(:log_extra_metadata_on_done).with(:results, expected_result)
         worker.perform
       end
+
+      it 'calls the consistency_fix_service to fix the inconsistencies' do
+        expect_next_instance_of(Database::ConsistencyFixService) do |instance|
+          expect(instance).to receive(:execute).with(
+            ids: [missing_project.id]
+          ).and_call_original
+        end
+        worker.perform
+      end
     end
   end
 end