Add RepositoryCheck::DispatchWorker to start worker per shard

The RepositoryCheck::DispatchWorker will start a
RepositoryCheck::BatchWorker for each healthy shard.

Closes gitlab-org/gitlab-ce#48042

app/workers/all_queues.yml

@@ -11,7 +11,7 @@
 - cronjob:remove_old_web_hook_logs
 - cronjob:remove_unreferenced_lfs_objects
 - cronjob:repository_archive_cache
-- cronjob:repository_check_batch
+- cronjob:repository_check_dispatch
 - cronjob:requests_profiles
 - cronjob:schedule_update_user_activity
 - cronjob:stuck_ci_jobs
@@ -71,6 +71,7 @@
 - pipeline_processing:update_head_pipeline_for_merge_request
 
 - repository_check:repository_check_clear
+- repository_check:repository_check_batch
 - repository_check:repository_check_single_repository
 
 - default

app/workers/repository_check/batch_worker.rb

@@ -3,13 +3,18 @@ module RepositoryCheck
     prepend ::EE::RepositoryCheck::BatchWorker
 
     include ApplicationWorker
-    include CronjobQueue
+    include RepositoryCheckQueue
 
     RUN_TIME = 3600
     BATCH_SIZE = 10_000
 
-    def perform
+    attr_reader :shard_name
+
+    def perform(shard_name)
+      @shard_name = shard_name
+
       return unless Gitlab::CurrentSettings.repository_checks_enabled
+      return unless Gitlab::ShardHealthCache.healthy_shard?(shard_name)
 
       start = Time.now
 
@@ -39,18 +44,22 @@ module RepositoryCheck
     end
 
     def never_checked_project_ids(batch_size)
-      Project.where(last_repository_check_at: nil)
+      projects_on_shard.where(last_repository_check_at: nil)
         .where('created_at < ?', 24.hours.ago)
         .limit(batch_size).pluck(:id)
     end
 
     def old_checked_project_ids(batch_size)
-      Project.where.not(last_repository_check_at: nil)
+      projects_on_shard.where.not(last_repository_check_at: nil)
         .where('last_repository_check_at < ?', 1.month.ago)
         .reorder(last_repository_check_at: :asc)
         .limit(batch_size).pluck(:id)
     end
 
+    def projects_on_shard
+      Project.where(repository_storage: shard_name)
+    end
+
     def try_obtain_lease(id)
       # Use a 24-hour timeout because on servers/projects where 'git fsck' is
       # super slow we definitely do not want to run it twice in parallel.

app/workers/repository_check/dispatch_worker.rb

+module RepositoryCheck
+  class DispatchWorker
+    include ApplicationWorker
+    include CronjobQueue
+    include ::EachShardWorker
+
+    def perform
+      return unless Gitlab::CurrentSettings.repository_checks_enabled
+
+      each_shard do |shard_name|
+        RepositoryCheck::BatchWorker.perform_async(shard_name)
+      end
+    end
+  end
+end

changelogs/unreleased/tc-repo-check-per-shard.yml

+---
+title: Run repository checks in parallel for each shard
+merge_request: 20179
+author:
+type: added

config/initializers/1_settings.rb

@@ -301,7 +301,7 @@ Settings.cron_jobs['expire_build_artifacts_worker']['cron'] ||= '50 * * * *'
 Settings.cron_jobs['expire_build_artifacts_worker']['job_class'] = 'ExpireBuildArtifactsWorker'
 Settings.cron_jobs['repository_check_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['repository_check_worker']['cron'] ||= '20 * * * *'
-Settings.cron_jobs['repository_check_worker']['job_class'] = 'RepositoryCheck::BatchWorker'
+Settings.cron_jobs['repository_check_worker']['job_class'] = 'RepositoryCheck::DispatchWorker'
 Settings.cron_jobs['admin_email_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['admin_email_worker']['cron'] ||= '0 0 * * 0'
 Settings.cron_jobs['admin_email_worker']['job_class'] = 'AdminEmailWorker'

ee/spec/workers/ee/repository_check/batch_worker_spec.rb

@@ -3,8 +3,13 @@ require 'spec_helper'
 describe EE::RepositoryCheck::BatchWorker do
   include ::EE::GeoHelpers
 
+  let(:shard_name) { 'default' }
   subject(:worker) { RepositoryCheck::BatchWorker.new }
 
+  before do
+    Gitlab::ShardHealthCache.update([shard_name])
+  end
+
   context 'Geo primary' do
     set(:primary) { create(:geo_node, :primary) }
 
@@ -13,9 +18,9 @@ describe EE::RepositoryCheck::BatchWorker do
     end
 
     it 'loads project ids from main database' do
-      projects = create_list(:project, 3, created_at: 1.week.ago)
+      projects = create_list(:project, 3, created_at: 1.week.ago, repository_storage: shard_name)
 
-      expect(worker.perform).to eq(projects.map(&:id))
+      expect(worker.perform(shard_name)).to match_array(projects.map(&:id))
     end
   end
 
@@ -28,16 +33,24 @@ describe EE::RepositoryCheck::BatchWorker do
 
     it 'loads project ids from tracking database' do
       project_registries = create_list(:geo_project_registry, 3, :synced)
+      update_project_registry_shard(project_registries, shard_name)
 
-      expect(worker.perform).to eq(project_registries.map(&:project_id))
+      expect(worker.perform(shard_name)).to match_array(project_registries.map(&:project_id))
     end
 
     it 'loads project ids that were checked more than a month ago from tracking database' do
       project_registries = create_list(:geo_project_registry, 3, :synced,
                                        last_repository_check_failed: false,
                                        last_repository_check_at: 42.days.ago)
+      update_project_registry_shard(project_registries, shard_name)
+
+      expect(worker.perform(shard_name)).to match_array(project_registries.map(&:project_id))
+    end
+  end
 
-      expect(worker.perform).to eq(project_registries.map(&:project_id))
+  def update_project_registry_shard(project_registries, shard_name)
+    project_registries.each do |registry|
+      Project.find(registry.project_id).update_column(:repository_storage, shard_name)
     end
   end
 end

spec/workers/repository_check/batch_worker_spec.rb

 require 'spec_helper'
 
 describe RepositoryCheck::BatchWorker do
+  let(:shard_name) { 'default' }
   subject { described_class.new }
 
+  before do
+    Gitlab::ShardHealthCache.update([shard_name])
+  end
+
   it 'prefers projects that have never been checked' do
     projects = create_list(:project, 3, created_at: 1.week.ago)
     projects[0].update_column(:last_repository_check_at, 4.months.ago)
     projects[2].update_column(:last_repository_check_at, 3.months.ago)
 
-    expect(subject.perform).to eq(projects.values_at(1, 0, 2).map(&:id))
+    expect(subject.perform(shard_name)).to eq(projects.values_at(1, 0, 2).map(&:id))
   end
 
   it 'sorts projects by last_repository_check_at' do
@@ -17,7 +22,7 @@ describe RepositoryCheck::BatchWorker do
     projects[1].update_column(:last_repository_check_at, 4.months.ago)
     projects[2].update_column(:last_repository_check_at, 3.months.ago)
 
-    expect(subject.perform).to eq(projects.values_at(1, 2, 0).map(&:id))
+    expect(subject.perform(shard_name)).to eq(projects.values_at(1, 2, 0).map(&:id))
   end
 
   it 'excludes projects that were checked recently' do
@@ -26,7 +31,14 @@ describe RepositoryCheck::BatchWorker do
     projects[1].update_column(:last_repository_check_at, 2.months.ago)
     projects[2].update_column(:last_repository_check_at, 3.days.ago)
 
-    expect(subject.perform).to eq([projects[1].id])
+    expect(subject.perform(shard_name)).to eq([projects[1].id])
+  end
+
+  it 'excludes projects on another shard' do
+    projects = create_list(:project, 2, created_at: 1.week.ago)
+    projects[0].update_column(:repository_storage, 'other')
+
+    expect(subject.perform(shard_name)).to eq([projects[1].id])
   end
 
   it 'does nothing when repository checks are disabled' do
@@ -34,13 +46,20 @@ describe RepositoryCheck::BatchWorker do
 
     stub_application_setting(repository_checks_enabled: false)
 
-    expect(subject.perform).to eq(nil)
+    expect(subject.perform(shard_name)).to eq(nil)
+  end
+
+  it 'does nothing when shard is unhealthy' do
+    shard_name = 'broken'
+    create(:project, created_at: 1.week.ago, repository_storage: shard_name)
+
+    expect(subject.perform(shard_name)).to eq(nil)
   end
 
   it 'skips projects created less than 24 hours ago' do
     project = create(:project)
     project.update_column(:created_at, 23.hours.ago)
 
-    expect(subject.perform).to eq([])
+    expect(subject.perform(shard_name)).to eq([])
   end
 end

spec/workers/repository_check/dispatch_worker_spec.rb

+require 'spec_helper'
+
+describe RepositoryCheck::DispatchWorker do
+  subject { described_class.new }
+
+  it 'does nothing when repository checks are disabled' do
+    stub_application_setting(repository_checks_enabled: false)
+
+    expect(RepositoryCheck::BatchWorker).not_to receive(:perform_async)
+
+    subject.perform
+  end
+
+  it 'dispatches work to RepositoryCheck::BatchWorker' do
+    expect(RepositoryCheck::BatchWorker).to receive(:perform_async).at_least(:once)
+
+    subject.perform
+  end
+
+  context 'with unhealthy shard' do
+    let(:default_shard_name) { 'default' }
+    let(:unhealthy_shard_name) { 'unhealthy' }
+    let(:default_shard) { Gitlab::HealthChecks::Result.new(true, nil, shard: default_shard_name) }
+    let(:unhealthy_shard) { Gitlab::HealthChecks::Result.new(false, '14:Connect Failed', shard: unhealthy_shard_name) }
+
+    before do
+      allow(Gitlab::HealthChecks::GitalyCheck).to receive(:readiness).and_return([default_shard, unhealthy_shard])
+    end
+
+    it 'only triggers RepositoryCheck::BatchWorker for healthy shards' do
+      expect(RepositoryCheck::BatchWorker).to receive(:perform_async).with('default')
+
+      subject.perform
+    end
+  end
+end