Prevent concurrent retries of the same pipeline

What does this MR do and why?

Retrying a pipeline twice at the same time causes duplicate work: duplicate clones of each failed job, duplicate todos, and duplicate downstream pipeline triggers. This MR stops that.

This MR makes three changes:

  1. Ci::RetryPipelineWorker is now idempotent! with deduplicate :until_executed, if_deduplicated: :reschedule_once. Identical queued or running jobs for one pipeline now collapse into one job. A job that arrives while another one is running gets re-run once, after the first one finishes.
  2. Ci::RetryPipelineService now takes a non-waiting Redis exclusive lease per pipeline for the duration of the retry. This is behind the feature flag lock_concurrent_pipeline_retries, which is off by default. If a second call comes in while the lease is held, it gets a ServiceResponse error with reason :retry_in_progress and HTTP status 409.
  3. The REST endpoint POST /projects/:id/pipelines/:pipeline_id/retry now documents the new 409 Conflict response, in doc/api/pipelines.md and the OpenAPI spec.

The worker also logs error_message and error_reason when the service rejects a retry.

References

How to set up and validate locally

  1. In a Rails console, run Feature.enable(:lock_concurrent_pipeline_retries).

  2. Pick a project with a pipeline that has at least one failed job. Note the project ID and the pipeline ID.

  3. Send two retry calls at the same time. For example, run two curl calls in the background, then wait:

    curl --request POST --header "PRIVATE-TOKEN: <token>" "http://gdk.test:3000/api/v4/projects/<project_id>/pipelines/<pipeline_id>/retry" &
    curl --request POST --header "PRIVATE-TOKEN: <token>" "http://gdk.test:3000/api/v4/projects/<project_id>/pipelines/<pipeline_id>/retry" &
    wait
  4. Expect one 201 response and one 409 response with {"message":"Pipeline is already being retried"}.

  5. Disable the flag and repeat the test. Expect both calls to return 201.

Notes for reviewers

  • The rate limit check runs before the lock, on purpose. The per-user rate limit check happens before the lease is taken. A call rejected with 409 still counts toward the rate limit, the same way a call rejected for access does. This keeps the counter meaning "calls made", not "retries that did work".
  • Where the 409 shows up. The REST API returns it as the HTTP status. The GraphQL PipelineRetry mutation returns the message in errors. The pipelines controller behind the UI retry button only checks access and then queues the worker. So a 409 or 429 from the worker path only shows up in the Sidekiq logs, as error_message and error_reason.
  • A known idempotency limit. If a pipeline's persistent ref cannot be created, the second run of the worker raises StateMachines::InvalidTransition, because the first run already dropped the pipeline. This is pre-existing behaviour. This MR does not introduce it. The idempotency spec uses a failed build to keep the pipeline in a state the second run can transition from.
  • No changelog entry. The behaviour change is behind a flag that is off by default. The changelog entry will go in the MR that turns the flag on by default.
  • Needs a technical writing review for doc/api/pipelines.md and doc/api/openapi/openapi_v3.yaml.

MR acceptance checklist

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

Related to https://gitlab.com/gitlab-org/gitlab/-/work_items/631374 and #627233

Edited by Shabini Rajadas

Merge request reports

Loading
Loading