Skip to content

Confirm pipeline existence when accepting a MR

Kerri Miller requested to merge 55127-add-delay-after-mr-creation-for-async-tasks-to-complete into master

What does this MR do?

If we create a MR then quickly (immediately) accept it via the API, the async task that creates and attaches a pipeline record to the MR may not have completed. The code uses the pipeline's existence and active? status as part of the tests to decide between immediately merging or to merge when the pipeline successfully completes; without an existing pipeline, it will attempt to merge immediately, regardless of the project's settings.

The reported race condition disappears when a delay is introduced between the creation and the acceptance API calls, so originally this MR replicated this delay, by introducing a check for the existence of of a merge_request.head_pipeline object. If one is not found, it raises a (new) PipelineNotFound exception, which provokes a 1-second sleep for the async task to catch up. It will attempt this 3 times, introducing at maximum a 3-second delay on this endpoint.

This isn't a great approach, but when we're dealing with an asynchronous task, we really can't be sure when it will execute. A better approach was where I landed -- if either the MR or the request required a successful pipeline run, then we call/return not_allowed! if a pipeline is not found. This allows us to write deterministic tests, and make behavior mirror what is the user's declared expectations for behavior.

Does this MR meet the acceptance criteria?

Conformity

Performance and testing

We have no available information as to the incident rate this error occurs, so it is difficult to estimate the impact, other than "Well, we only have one user reporting this, and the code has been out there for years.." so I'd guess the incident rate is fairly low.

Closes #55127 (closed)

Edited by Kerri Miller

Merge request reports