Skip to content

Preserve merge train history

Shinya Maeda requested to merge preserve-merge-train-rows-after-merge into master

What does this MR do?

Currently, we delete merge train rows after merge happens. However, this should be persisted in order to retain the history, because this history is quite useful to track the merge train status, especially to check if master's commit history aligns to merge train's history. => #13001 (closed)

We can also measure merge train stats that how long each MR took to get merged. This can be accomplished by joining tables with merge_request_metrics. => #36146 (closed)

We introduce status column to merge_trains table in order to track the status of trains. Any merge requests should start the status from created state, and will end up merged if nothing unexceptional case happened. In a certain case, the merge train could be marked as stale (e.g. when an immediate merge was performed), in this case, the system should re-create pipeline for staled trains. You can see the further improvement in !19434 (merged)

Related: #13001 (closed)

Query plan

gitlabhq_development=# EXPLAIN ANALYZE SELECT "merge_requests".* FROM "merge_requests"
gitlabhq_development-# INNER JOIN "merge_trains" ON "merge_trains"."merge_request_id" = "merge_requests"."id"
gitlabhq_development-# WHERE "merge_trains"."target_project_id" = 21 AND "merge_trains"."target_branch" = 'master' AND "merge_trains"."status" IN (0, 1)
gitlabhq_development-# ORDER BY merge_trains.id ASC;
                                                                               QUERY PLAN                                                                                
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=14.64..14.64 rows=1 width=853) (actual time=0.198..0.199 rows=6 loops=1)
   Sort Key: merge_trains.id
   Sort Method: quicksort  Memory: 26kB
   ->  Hash Join  (cost=8.18..14.63 rows=1 width=853) (actual time=0.111..0.153 rows=6 loops=1)
         Hash Cond: (merge_requests.id = merge_trains.merge_request_id)
         ->  Seq Scan on merge_requests  (cost=0.00..6.35 rows=35 width=845) (actual time=0.014..0.034 rows=42 loops=1)
         ->  Hash  (cost=8.17..8.17 rows=1 width=12) (actual time=0.051..0.052 rows=6 loops=1)
               Buckets: 1024  Batches: 1  Memory Usage: 9kB
               ->  Index Scan using index_for_status_per_branch_per_project on merge_trains  (cost=0.15..8.17 rows=1 width=12) (actual time=0.033..0.040 rows=6 loops=1)
                     Index Cond: ((target_project_id = 21) AND (target_branch = 'master'::text))
                     Filter: (status = ANY ('{0,1}'::integer[]))
 Planning time: 0.912 ms
 Execution time: 0.449 ms
(13 rows)

The index index_for_status_per_branch_per_project is working in the query plan.

TODO

  • Finish ee/spec/models/merge_train_spec.rb
  • ee/app/services/merge_trains/refresh_merge_request_service.rb

Screenshots

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
Edited by Shinya Maeda

Merge request reports