Continue marking merge requests merged when one fails

What does this MR do and why?

Merged-manually detection (MergeRequests::RefreshService#post_merge_manually_merged, run in UpdateMergeRequestsWorker on every branch push) processes all candidate merge requests of a push in one unprotected loop. If handling one merge request raises, the whole worker fails and every remaining merge request in that push is skipped. A push is never re-inspected, so those merge requests stay open forever even though their commits are on the target branch.

Production evidence (see linked issue): one push had 0 of 10 merge requests detected across all retries. On another push, the first attempt died mid-merge-request, leaving it "half-merged" (state merged but null merged_at, null merged_by, and no merged state event), while a retry salvaged the other 10. The observed exception was ActiveRecord::QueryCanceled raised from the EE productivity metrics calculation, on a project with a very large notes table.

Changes made:

  • Continue marking merge requests merged when one fails:
    • In the post_merge_manually_merged loop, rescue per merge request, log the exception, and re-raise the first error after the loop finishes. Every merge request in the batch gets processed on each attempt, the worker still fails so Sidekiq retries, and the retry only re-attempts merge requests that are still open (merged ones are excluded by the query).
  • Save merge metrics when productivity data times out:
    • EE::MergeRequestMetricsService rescues ActiveRecord::QueryCanceled around the productivity data computation only, tracks the error the same way, and falls back to an empty hash, so merged_at, merged_by, and the merge event still persist. Productivity metrics columns are analytics so it should not stop the entire worker.

Note: EE::MergeRequestMetricsService change is only effective when the merge_request_metrics_outside_merge_transaction feature flag is enabled (that flag moves the metrics computation before the Event.transaction. With the flag disabled, the computation happens inside the transaction, so a statement timeout aborts the PostgreSQL transaction, the rescue cannot save the event write, and behavior degrades to the status quo (the raise propagates and commit 1 tracks and retries it).

References

Related to #624830 (closed)

Edited by Marc Shaw

Merge request reports

Loading
Loading