Run likely-to-fail mergeability checks first in auto-merge

Marc Shawrequested to merge
631600-auto-merge-likely-to-fail-first-checks into master
  • Auto-merge workers (AutoMergeProcessWorker, ProcessAutoMergeFromEventWorker) run the full mergeability suite on every run, but most runs cannot merge yet. The most common blockers (open threads, CI) are checked 11th and 12th, so those runs pay for about ten passing checks first.
  • Behind the auto_merge_process_early_exits flag, the worker now runs the checks most likely to fail first, and exits early for a known conflict or an already queued merge.
  • On GDK, round trips (SQL + Redis + Gitaly) across a full auto-merge lifecycle drop by about 31-32%. No state gets worse.
Detailed context for AI agents

What does this MR do and why?

Problem:

  • AutoMergeProcessWorker (and ProcessAutoMergeFromEventWorker, through AutoMergeService#process) runs the full mergeability state check suite on every run.
  • Most runs cannot merge yet: CI still running, open threads, unapproved, a known conflict, or a merge already queued.
  • The suite is ordered cheapest first (tiers trivial/light/heavy by production P99 latency) and stops at the first failure. The most common blockers, open threads and CI, are checked 11th and 12th in EE, so a run that stops on them still pays for about ten checks that pass.
  • Two states fail only at the very end:
    • A merge request already known to be cannot_be_merged: the conflict check runs last, in mergeable_git_state?.
    • A merge already queued: the run passes everything and enqueues a second MergeWorker, which MergeWorker deduplicates, and overwrites merge_jid.
  • Production context: on GitLab.com this worker's slow jobs are mostly these no-op runs, and the time is largely Ruby GVL wait on the 10-thread low-urgency-cpu-bound shard. With the enable_sidekiq_gvl_metrics sample, 30-70% of this worker's job time is GVL wait. Under that load every database or Redis round trip cost roughly 165-565 ms in a local reproduction with busy threads in the same process, so fewer round trips per run means shorter jobs. Discussion: !259376 (comment 3971487486)

Change (all behind the gitlab_com_derisk flag auto_merge_process_early_exits, actor: project, default off):

  • MergeRequest.mergeable_state_checks(order:) takes :cheapest_first (default, unchanged, from .mergeable_state_checks_by_tier) or :likely_to_fail_first (from the new .likely_to_fail_first_checks_by_group). An unknown order raises ArgumentError.
    • .likely_to_fail_first_checks_by_group sits next to .mergeable_state_checks_by_tier and lists every check, grouped the same way: likely_light (open, draft, threads; EE adds blocked by another MR and changes requested), likely_heavy (CI), then light and heavy for the rest.
    • EE adds its checks into each group in its override, the same way it extends the tiers.
    • Specs keep both lists to the same checks: combined, CE on its own, and the EE additions on their own, so a check cannot be added to one list without the other.
  • MergeRequest#mergeable? and #mergeable_state? accept check_order: (default :cheapest_first).
  • AutoMerge::MergeWhenChecksPassService#process with the flag on:
    • returns early if merge_request.cannot_be_merged? or merge_request.merge_ongoing?;
    • otherwise calls mergeable?(use_cache: false, check_order: :likely_to_fail_first).
    • Flag off: unchanged mergeable?(use_cache: false).
  • AutoMerge::AddToMergeTrainWhenChecksPassService#process (EE) with the flag on passes check_order: :likely_to_fail_first. It does not get the two early exits because it skips the conflict check on purpose (trains handle conflicts) and never calls merge_async.
  • Feature flag YAML: config/feature_flags/gitlab_com_derisk/auto_merge_process_early_exits.yml. Rollout issue: #632857.

Known small trade-off:

  • CheckSecurityPolicyViolationsService schedules an approval rules resync when policy rules are out of sync. It now runs after CI in the new order, so for an MR with out-of-sync policy rules that resync starts once CI passes instead of while CI runs. No effect when rules are in sync.

References

Screenshots or screen recordings

Not applicable, backend only.

How to set up and validate locally

  1. Enable the flag for a project: Feature.enable(:auto_merge_process_early_exits, Project.find(<id>)).
  2. Set auto-merge (merge when checks pass) on a merge request in that project while its pipeline is running, then let the pipeline succeed. It merges as before.
  3. Compare Sidekiq log lines for AutoMergeProcessWorker on that project with the flag off and on. db_count is lower for runs that cannot merge yet.

MR acceptance checklist

Evaluated against https://docs.gitlab.com/development/code_review/#acceptance-checklist

Rollout and revert

  • Enable for a test project, then a busy project with many auto-merge merge requests, then a percentage of actors.
  • Measure before and after with:
    • Sidekiq job logs (json.db_count, json.redis_calls, json.duration_s for AutoMergeProcessWorker);
    • Mimir mean DB seconds and GVL wait per job;
    • MergeWorker log lines with json.job_status "deduplicated" (should drop).
  • Revert by disabling the flag.

Alternatives considered

  • Running a few checks by hand before the suite and passing skip_* params (first prototype): worked, but bypassed the per-check mergeability logging and needed skip params kept in sync with the pre-run checks.
  • Sorting the tier order by a list of check identifiers (an earlier version of this MR): replaced by explicit lists next to the tiers after review.
  • Passing an explicit list of check classes into mergeable?: callers could pass a partial list and drop a check by mistake, and CE cannot reference EE-only check classes.
  • Exiting early when another process holds the mergeability lease: dropped, rare and small gain for an extra public method on MergeabilityCheckService.
  • Reordering the default (cheapest-first) order for everyone: would change which failing check the merge widget and API report first.

Verification performed

  • Specs, all passing locally:
    • spec/services/auto_merge/merge_when_checks_pass_service_spec.rb
    • ee/spec/services/auto_merge/add_to_merge_train_when_checks_pass_service_spec.rb
    • spec/models/merge_request_spec.rb (.mergeable_state_checks order invariant and #mergeable_state? check_order)
    • related auto-merge service and worker specs
    • New examples were confirmed to fail against master code.
  • GDK end-to-end traces of all worker paths and two full lifecycles, flag off and on, with the real code. Round trips = SQL + Redis + Gitaly, summed over every worker run in the lifecycle (flag off -> on):
    • CI finishes last (auto-merge set while CI runs, approval, thread resolved, 3 target pushes, 2 child pipelines, pipeline success and merge, the self re-enqueued run): 318 -> 218 (-31%).
    • People finish last (CI passes early, then 3 target pushes while waiting on a thread and an approval, thread resolved, approval and merge, self re-enqueued run): 256 -> 175 (-32%).
  • Single runs (flag off -> on):
    • CI running: 20 -> 14
    • thread open: 13 -> 4
    • known conflict: 32 -> 3
    • merge already queued: 35 -> 4, and no second MergeWorker enqueue
    • blocked by draft: 3 -> 3
    • blocked by another MR: 6 -> 5
    • changes requested: 7 -> 6
    • the run that merges and the unapproved run: unchanged
  • No state gets worse. Every path ends in the same state (merged or not, same merge_status) with the flag on and off.

Out of scope (follow-ups)

  • Not enqueueing the worker for child pipelines.
  • Suppressing the self re-enqueue.
  • Making the worker's deduplication effective.
  • The second full check pass in MergeTrainService#available_for?.
  • Moving the worker to high urgency: !259376
Edited by Marc Shaw

Merge request reports