Improve the way we check if a MR is on "merging" state (#31207 follow-up)
Further discussed on https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13207/diffs#ef51b30fcc35462117070f3fe498d3949c279cb9_395_389.
!13207 (merged) introduces a method (MergeRequest#merge_ongoing?
) which checks if there's a running MergeWorker
job for a given MR.
This method could be improved in a way that:
- We wouldn't need to read from Redis to check it
- We would cover the scenario where the job was enqueued but have not started processing yet
In order to make this happen we'd need:
- A consistent way to update
MergeRequest#merge_jid
tonil
if the MR was not successfully merged - Instead persisting
merge_jid
on theMergeWorker
itself - persist afterMergeWorker.perform_async
call (it returns the jid right away) - Change
MergeRequest#merge_ongoing?
to something close to:
def merge_ongoing?
merge_jid && !merged?
end
Other point(s) to address with this issue:
- Remove the
if locked?
on https://gitlab.com/gitlab-org/gitlab-ee/blob/master/app/models/merge_request.rb#L847 (state machine should be able to take care of this)
Edited by Oswaldo Ferreir