Automatically update MR merge-ref along merge status
What does this MR do?
Second try for https://gitlab.com/gitlab-org/gitlab-ce/issues/58495
- Handles race condition found at https://gitlab.com/gitlab-org/gitlab-ce/issues/62856
- Closes #58495 (closed)
Description of initial MR https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/28513
Stop recomputing unnecessarily the merge ref
We've introduced a MergeRequests::MergeabilityCheckService. This service knows if the merge-ref is outdated or not (looking at the merge_status. That means now we don't need to keep merging to the ref in a ad-hoc manner. This service should be the SSoT. Which leads to:
Coupling the MR merge_status and refs/merge-requests/:iid/merge ref update
With the introduced service we make sure the updates are made together in a single place. That is, we should not provide a way of updating one, without updating the other.
Make sure we're using the latest merge ref for creating pipelines
MergeRequest#merge_ref_head (which encapsulates MergeRequests::MergeabilityCheckService) behavior is enough as a interface for both our /ensure_merge_ref API and for ~Verify team needs of creating pipelines for this ref.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/58495
Performance tests
Since it'll be executed whenever the source/target branches get updated and the user accesses the MR page, I've put this into a reasonably heavy test in a MR with 114 file changes, 2882 additions and 12802 deletions. The repo target branch has 20k commits and 142MB in file size.
It's taking around 1 second to recalculate the merge-ref for it, which shows it's not a cheap operation. Important to bring up that we won't need to make this operation again if the source/target branches doesn't get updated.
That makes me wonder if we haven't considered recalculating the MR mergeability (and now the merge-ref altogether) at the RefreshService. Though, with the current approach we're recalculating it lazily, instead always iterating in every possible MR and recalculating it.
As an alternative to this potential performance issue, we might consider having a separate worker, which would be called at the end of RefreshService processing (to avoid introducing even more weight to this service). That worker would call MergeRequests::MergeabilityCheckService. As expected, when this worker gets processed, the MR will have both the merge-status and merge-ref updated, and when someone accesses the MR page, no extra computation will be needed.
Though, if for some reason the page is accessed before this worker gets finished, the update computation will be done upon access, as we're currently doing (I would expect that would be an exception, instead the rule).