Skip to content

LinkMergeRequestsService: Remove fast-forward MR checking

Pam Artiaga requested to merge 454523-skip-checking-for-ff-merge into master

What does this MR do and why?

Issue: Follow-up from "Insert MRs by head_commit_sha s... (#454523 - closed)

As part of Insert MRs by head_commit_sha separately (!147297 - merged), we check if a project is a fast-forward merge before inserting merge requests queried by head_commit_sha, see: https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/deployments/link_merge_requests_service.rb#L93

It was pointed out that this could introduce a race condition:

  1. FF merge is made
  2. User disables ff-only merges
  3. This service is invoked by Sidekiq and the check returns false
  4. The deployment ends up without a link to the FF merge made on step 1

If what is described above is possible, would it be safe to remove this check?

This check was introduced because it was very difficult to come up with a performant query for "merge requests by merge commit sha". However, we have now come up with performant query (see comment thread), so it is better if we remove this check to avoid the possibility of the described race condition above.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

See: !145211 (merged)

How to set up and validate locally

See: !145211 (merged)

Related to #454523 (closed)

Merge request reports