Resolves race condition for mergeability check service
From MR:
As pointed out at https://gitlab.slack.com/archives/CHLKE258E/p1559810531030900, the MergeabilityCheckService can return success even with an outdated merge ref. That's because the MergeRequests::RefreshService is called async and it has the responsibility of flipping the MRs merge_status to unchecked when they receive updates, or its target branches receives an update.
This, for instance, cause problems to create pipelines with the latest state:
- MR-1 merged
- MR-2 has an existing merge ref (but
recheck_state?(merge_status)returnsfalse- race condition) - MR-2 creates a pipeline for merged results on old target sha (given
MergeabilityCheckServicereturnedsuccesswith an outdated merge ref)
What does this MR do?
It was briefly discussed at https://gitlab.com/gitlab-org/gitlab-ce/issues/62856#note_178702235. Here we introduce a
recheck flag to MergeabilityCheckService#execute. From the method documentation:
recheck - When given, it'll enforce a merge-ref refresh if it's outdated, even if the current merge_status is can_be_merged or cannot_be_merged. Given MergeRequests::RefreshService is called async, it might happen that the target branch gets updated, but the MergeRequest#merge_status lags behind. So in scenarios where we need instance feedback of the current state of the repository, the
recheckargument is required.
We initially didn't consider having a recheck, though CI pipelines showed doing it for every caller can't make it. It'd introduce several N+1's, given MergeRequest#check_mergeability is a caller, and is used in iterations. So now, we use that with a recheck in the API, and will introduce that for EE, when creating pipelines, required mainly for the merge-train feature (where we can't afford the time to process the RefreshService async).