Skip to content

Schedule clean up of merge request refs efficiently

Patrick Bajao requested to merge 245263-mr-refs-cleanup-scheduler into master

What does this MR do?

In !41555 (merged), we implemented scheduling MergeRequestCleanupRefsWorker 14 days after a merge request gets closed/merged.

That means it'll stay in the scheduled queue for 14 days before it'll be worked on. That can result to huge number of jobs in the scheduled queue.

This MR changes the way we schedule things by utilizing the database and a cron worker. Every minute, a worker is scheduled to run which will look for 300 MRs scheduled to be cleaned up at that time. Chose 300 since based on initial data we get in this chart, a MergeRequestCleanupRefsWorker job gets executed in ~190ms average. That tells me, in a minute, 300 jobs can be performed. We also have multiple workers that can work on it. But since there are spikes from time to time, it's better to give it some allowance.

This needs to be done before we can cleanup the old MRs that were already closed/merged prior to this change: !46782 (merged).

Migration

== 20201023114628 CreateMergeRequestCleanupSchedules: migrating ===============
-- create_table(:merge_request_cleanup_schedules, {:if_not_exists=>true})
   -> 0.0174s
== 20201023114628 CreateMergeRequestCleanupSchedules: migrated (0.0174s) ======

Query Performance

With 1_000_000 records in merge_request_cleanup_schedules (tested on local machine):

gitlabhq_development=# EXPLAIN ANALYZE SELECT "merge_request_cleanup_schedules"."merge_request_id" FROM "merge_request_cleanup_schedules" WHERE (completed_at IS NULL AND scheduled_at <= NOW()) ORDER BY scheduled_at DESC LIMIT 300;
                                                                                       QUERY PLAN                                                                                        
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.28..6.37 rows=5 width=16) (actual time=0.038..0.273 rows=300 loops=1)
   ->  Index Scan Backward using index_mr_cleanup_schedules_timestamps on merge_request_cleanup_schedules  (cost=0.28..6.37 rows=5 width=16) (actual time=0.037..0.191 rows=300 loops=1)
         Index Cond: (scheduled_at <= now())
 Planning Time: 0.136 ms
 Execution Time: 0.348 ms
(5 rows)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Related to #245263 (closed)

Edited by Patrick Bajao

Merge request reports