From 79dc708cb6e4497709d4b1b0a762e214f3a72318 Mon Sep 17 00:00:00 2001 From: Gregorius Marco Date: Wed, 22 Oct 2025 16:12:08 +0800 Subject: [PATCH 1/3] Fix ResumeWorker not scheduling to the correct queue `ResumeWorker` execution with an worker name argument will try to reschedule another `ResumeWorker` job with 1s delay. However, using `.perform_in` would route the job to `ResumeWorker`'s queue which is the default/catchall queue. Instead, we want to run the `ResumeWorker` in the same queue that the resumed worker is running. The fix is by using the Sidekiq API `Sidekiq::Client.enqueue_to_in`. --- .rubocop.yml | 1 + .../concurrency_limit/resume_worker.rb | 20 +++++++++++-------- .../concurrency_limit/resume_worker_spec.rb | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 9322efd8591ef1..40cbb37da7a004 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1259,6 +1259,7 @@ Cop/SidekiqApiUsage: - 'lib/gitlab/redis/queues.rb' - 'app/workers/concerns/application_worker.rb' - 'config/initializers/active_job_shard_support.rb' + - 'app/workers/concurrency_limit/resume_worker.rb' Cop/FeatureFlagUsage: Include: diff --git a/app/workers/concurrency_limit/resume_worker.rb b/app/workers/concurrency_limit/resume_worker.rb index 37572e38b16240..72617dbaa3958e 100644 --- a/app/workers/concurrency_limit/resume_worker.rb +++ b/app/workers/concurrency_limit/resume_worker.rb @@ -40,16 +40,11 @@ def schedule_workers end def schedule_worker(worker) - _, pool = Gitlab::SidekiqSharding::Router.get_shard_instance(worker.get_sidekiq_options['store']) - Sidekiq::Client.via(pool) do - queue = ::Gitlab::SidekiqConfig::WorkerRouter.global.route(worker) + Gitlab::SidekiqSharding::Router.route(worker) do # Schedules ResumeWorker job to the respective queue of the `worker` we're resuming. # This is because `worker_limit` requires reading environment variables unique each sidekiq shard, # whereas ResumeWorker (cronjob) always runs in the default queue. - # - # rubocop: disable Cop/SidekiqApiUsage -- valid usage of scheduling to other queue - Sidekiq::Client.push('class' => self.class, 'args' => [worker.name], 'queue' => queue) - # rubocop: enable Cop/SidekiqApiUsage + Sidekiq::Client.push('class' => self.class, 'args' => [worker.name], 'queue' => queue_for_worker(worker)) end end @@ -73,7 +68,12 @@ def process_worker(worker_name) resumed_jobs_num = resume_processing!(worker) cleanup_stale_trackers(worker) - self.class.perform_in(RESCHEDULE_DELAY, worker_name) if queue_size(worker) > 0 + if queue_size(worker) > 0 + Gitlab::SidekiqSharding::Router.route(worker) do + Sidekiq::Client.enqueue_to_in(queue_for_worker(worker), RESCHEDULE_DELAY, worker, [worker.name]) + end + end + log_extra_metadata_on_done(:resumed_jobs, resumed_jobs_num) end @@ -104,5 +104,9 @@ def worker_limit(worker) def workers Gitlab::SidekiqConfig.workers_without_default.map(&:klass) end + + def queue_for_worker(worker) + ::Gitlab::SidekiqConfig::WorkerRouter.global.route(worker) + end end end diff --git a/ee/spec/workers/concurrency_limit/resume_worker_spec.rb b/ee/spec/workers/concurrency_limit/resume_worker_spec.rb index 728e4d4cf8b806..391e52775f648f 100644 --- a/ee/spec/workers/concurrency_limit/resume_worker_spec.rb +++ b/ee/spec/workers/concurrency_limit/resume_worker_spec.rb @@ -261,7 +261,7 @@ def with_shard_client(&block) expect(Gitlab::SidekiqMiddleware::ConcurrencyLimit::ConcurrencyLimitService) .to receive(:resume_processing!) .with(worker_with_concurrency_limit.name) - expect(described_class).to receive(:perform_in) + expect(Sidekiq::Client).to receive(:enqueue_to_in) perform end -- GitLab From 3974db99fe5afea176ce541be2c8faf6f98dbc4e Mon Sep 17 00:00:00 2001 From: Marco Gregorius Date: Wed, 22 Oct 2025 16:22:01 +0800 Subject: [PATCH 2/3] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: GitLab Duo --- app/workers/concurrency_limit/resume_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/concurrency_limit/resume_worker.rb b/app/workers/concurrency_limit/resume_worker.rb index 72617dbaa3958e..a721a283889716 100644 --- a/app/workers/concurrency_limit/resume_worker.rb +++ b/app/workers/concurrency_limit/resume_worker.rb @@ -70,7 +70,7 @@ def process_worker(worker_name) if queue_size(worker) > 0 Gitlab::SidekiqSharding::Router.route(worker) do - Sidekiq::Client.enqueue_to_in(queue_for_worker(worker), RESCHEDULE_DELAY, worker, [worker.name]) + Sidekiq::Client.enqueue_to_in(queue_for_worker(worker), RESCHEDULE_DELAY, self.class, [worker.name]) end end -- GitLab From 9e0db2faaf256bf77f70de4d94f0a22081c56c50 Mon Sep 17 00:00:00 2001 From: Gregorius Marco Date: Wed, 22 Oct 2025 16:26:52 +0800 Subject: [PATCH 3/3] update spec --- ee/spec/workers/concurrency_limit/resume_worker_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ee/spec/workers/concurrency_limit/resume_worker_spec.rb b/ee/spec/workers/concurrency_limit/resume_worker_spec.rb index 391e52775f648f..aa47c6376cc1d2 100644 --- a/ee/spec/workers/concurrency_limit/resume_worker_spec.rb +++ b/ee/spec/workers/concurrency_limit/resume_worker_spec.rb @@ -261,7 +261,8 @@ def with_shard_client(&block) expect(Gitlab::SidekiqMiddleware::ConcurrencyLimit::ConcurrencyLimitService) .to receive(:resume_processing!) .with(worker_with_concurrency_limit.name) - expect(Sidekiq::Client).to receive(:enqueue_to_in) + expect(Sidekiq::Client).to receive(:enqueue_to_in).with('default', described_class::RESCHEDULE_DELAY, + described_class, [worker_with_concurrency_limit.name]) perform end -- GitLab