Schedule MergeRequestCleanupRefsWorker more efficiently
What does this MR do?
This converts the MergeRequestCleanupRefsWorker to be a limited capacity worker.
The ScheduleMergeRequestCleanupRefsWorker will still be enqueuing jobs but it'll be based on capacity. If there's no capacity then no new jobs will be enqueued.
For now, we are capping it at 4 jobs at a time. This will be later on be configurable.
This is still behind the merge_request_refs_cleanup feature flag so we can easily turn it off on production while testing.
Does this MR meet the acceptance criteria?
Migration
db:migrate
== 20210707095545 AddStatusToMergeRequestCleanupSchedules: migrating ==========
-- column_exists?(:merge_request_cleanup_schedules, :status)
-> 0.0028s
-- transaction_open?()
-> 0.0000s
-- index_exists?(:merge_request_cleanup_schedules, :status, {:name=>"index_merge_request_cleanup_schedules_on_status", :algorithm=>:concurrently})
-> 0.0023s
-- execute("SET statement_timeout TO 0")
-> 0.0005s
-- add_index(:merge_request_cleanup_schedules, :status, {:name=>"index_merge_request_cleanup_schedules_on_status", :algorithm=>:concurrently})
-> 0.0034s
-- execute("RESET ALL")
-> 0.0005s
== 20210707095545 AddStatusToMergeRequestCleanupSchedules: migrated (0.0103s) =
== 20210708063032 AddFailedCountToMergeRequestCleanupSchedules: migrating =====
-- add_column(:merge_request_cleanup_schedules, :failed_count, :integer, {:default=>0, :null=>false})
-> 0.0024s
== 20210708063032 AddFailedCountToMergeRequestCleanupSchedules: migrated (0.0025s)
== 20210713070842 UpdateMergeRequestCleanupSchedulesScheduledAtIndex: migrating
-- transaction_open?()
-> 0.0000s
-- index_exists?(:merge_request_cleanup_schedules, :scheduled_at, {:where=>"completed_at IS NULL AND status = 0", :name=>"index_mr_cleanup_schedules_timestamps_status", :algorithm=>:concurrently})
-> 0.0016s
-- add_index(:merge_request_cleanup_schedules, :scheduled_at, {:where=>"completed_at IS NULL AND status = 0", :name=>"index_mr_cleanup_schedules_timestamps_status", :algorithm=>:concurrently})
-> 0.0020s
-- transaction_open?()
-> 0.0000s
-- indexes(:merge_request_cleanup_schedules)
-> 0.0016s
-- remove_index(:merge_request_cleanup_schedules, {:algorithm=>:concurrently, :name=>"index_mr_cleanup_schedules_timestamps"})
-> 0.0020s
== 20210713070842 UpdateMergeRequestCleanupSchedulesScheduledAtIndex: migrated (0.0085s)
db:rollback
== 20210713070842 UpdateMergeRequestCleanupSchedulesScheduledAtIndex: reverting
-- transaction_open?()
-> 0.0000s
-- indexes(:merge_request_cleanup_schedules)
-> 0.0023s
-- execute("SET statement_timeout TO 0")
-> 0.0004s
-- remove_index(:merge_request_cleanup_schedules, {:algorithm=>:concurrently, :name=>"index_mr_cleanup_schedules_timestamps_status"})
-> 0.0035s
-- execute("RESET ALL")
-> 0.0004s
-- transaction_open?()
-> 0.0000s
-- index_exists?(:merge_request_cleanup_schedules, :scheduled_at, {:where=>"completed_at IS NULL", :name=>"index_mr_cleanup_schedules_timestamps", :algorithm=>:concurrently})
-> 0.0011s
-- add_index(:merge_request_cleanup_schedules, :scheduled_at, {:where=>"completed_at IS NULL", :name=>"index_mr_cleanup_schedules_timestamps", :algorithm=>:concurrently})
-> 0.0022s
== 20210713070842 UpdateMergeRequestCleanupSchedulesScheduledAtIndex: reverted (0.0112s)
== 20210708063032 AddFailedCountToMergeRequestCleanupSchedules: reverting =====
-- remove_column(:merge_request_cleanup_schedules, :failed_count, :integer, {:default=>0, :null=>false})
-> 0.0017s
== 20210708063032 AddFailedCountToMergeRequestCleanupSchedules: reverted (0.0030s)
== 20210707095545 AddStatusToMergeRequestCleanupSchedules: reverting ==========
-- transaction_open?()
-> 0.0000s
-- indexes(:merge_request_cleanup_schedules)
-> 0.0029s
-- execute("SET statement_timeout TO 0")
-> 0.0005s
-- remove_index(:merge_request_cleanup_schedules, {:algorithm=>:concurrently, :name=>"index_merge_request_cleanup_schedules_on_status"})
-> 0.0046s
-- execute("RESET ALL")
-> 0.0005s
-- column_exists?(:merge_request_cleanup_schedules, :status)
-> 0.0009s
-- remove_column(:merge_request_cleanup_schedules, :status)
-> 0.0010s
== 20210707095545 AddStatusToMergeRequestCleanupSchedules: reverted (0.0113s) =
Query plans
- For
MergeRequest::CleanupSchedule.next_scheduled
: https://explain.depesz.com/s/hHTe - For
MergeRequestCleanupRefsWorker#remaining_work_count
: https://explain.depesz.com/s/xnJU
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.)
Related to #296874 (closed)
Edited by Patrick Bajao