Create ref-based pipeline creation requests at PostReceive (FF)

Peter Gumesonrequested to merge
reinstate-232296-b-writer-ff into master

What does this MR do and why?

This is the writer MR in the stack that reinstates !232296 (merged). The scaffolding MR !256819 (merged) and the reader MR !258481 (merged) are both merged.

On push, a pipeline creation request acts as a synchronization record so a merge request is not merged before its pipeline exists. Before this change, that protection had two gaps: only merge-request pipelines got a request record, so branch pipelines had no protection against the same race, and auto-merge could be set before the request was created, letting a merge request merge immediately instead of waiting for the pipeline (push options were especially prone to this).

Behind the feature flag track_ref_pipeline_creation (default off), this MR:

  • Creates a ref-based pipeline creation request at the start of PostReceive and threads it through pipeline creation.
  • Adds start_for_ref to Ci::PipelineCreation::Requests (the ref key helpers ship in !258481 (merged)).
  • Moves merge-request request registration earlier, into MergeRequests::CreateService, so pipeline_creating_for_merge_request? becomes true sooner, shrinking the auto-merge race window.
  • With the flag on, a successful non-deferred request is completed by MergeRequests::UpdateHeadPipelineWorker after it updates the head pipeline; a deferred request stays with MergeRequests::CreatePipelineWorker, as on master.
  • Creates at most git_push_pipeline_limit ref requests per push, and a Redis error while creating them does not stop the push from being processed.
  • Stamps started_at on ref requests, like master does for project and merge request requests, so the create_pipeline_from_commit SLI keeps its start time on the push path. With the flag on, that start time is when the push recorded the request.

This also carries the fix that the revert demanded. !232296 (merged) was reverted in !239194 (merged) after incident gitlab-com/gl-infra/production#22243 (closed): it made NewMergeRequestWorker take a third Sidekiq argument unconditionally, so during a rolling deploy, canary nodes enqueued 3-argument jobs while the rest of the fleet still ran the 2-argument worker and raised ArgumentError. Here, the third argument is gated entirely behind the flag: when the flag is off, NewMergeRequestWorker is enqueued with two arguments, the third is omitted entirely, so a mixed-version Sidekiq fleet never receives an argument arity it cannot handle.

Behavior notes

Two effects worth calling out explicitly:

  1. Under the flag, merge-request pipeline creation request registration moves earlier. This is the intended race fix described above.
  2. On CreatePipelineWorker retries-exhausted, the project pipeline creation request is now marked failed. This is active immediately and is not gated by the flag.

This MR is safe to merge while the flag is off, with caveat 2 above in mind since it takes effect regardless of the flag.

Feature flag

The flag definition ships in !258481 (merged).

No UI changes.

Why this is one MR

The diff looks large (around 750 lines), but only about 150 lines of that are application logic. The rest is test coverage.

The parts that could be separated already are: the scaffolding MR (!256819 (merged), merged) carried the deploy-safety pieces (the Sidekiq argument shape and the event schema that had to ship first), and the reader MR (!258481 (merged)) carries the mergeability reader and the flag definition. What is left here is the writer itself, which is the bulk of the feature.

The writer does not split further cleanly:

  • It is one change behind one feature flag, and its ref-level and merge-request-level paths share the same model methods and the same pipeline-creation code.
  • GitLab's unused-code linter requires each new method to land in the same MR as its caller. Peeling pieces out would either orphan methods or spread one flag's behavior across MRs, so no single reviewer would see the race fix end to end.

Redis cost

Each ref request is one Redis hash field:

  • Key pipeline_creation:projects:{<project_id>}:ref:{<sha256 of ref>}: about 106 bytes (larger than the MR key because it embeds a 64-character sha256).
  • Field (request ID): 36 bytes.
  • Value: 68 bytes (in_progress) to 106 bytes (failed).
  • About 210-250 bytes of data per entry, roughly 350 bytes with Redis overhead.

Live entries = pushes per second x branch requests per push (at most git_push_pipeline_limit, default 4) x 300s TTL. The TTL refreshes on each write, and pushes to the same branch within 5 minutes share one hash.

Estimate from gitlab.com: at peak, about 19,000 Repositories::PostReceiveWorker starts per 5-minute bucket (this counts wiki and snippet pushes too, which create no requests, so it's an upper bound). With the 300s TTL, that's about 19,000 live entries at one branch per push, roughly 7 MB, and at most about 27 MB if every push hit the 4-branch cap.

Redis Cluster picks the slot from the first {...} hash tag, which is {project_id}, so ref keys spread across slots per project, the same way the existing merge request keys do.

Known limitations

Finding 7 (residual gap): In projects whose workflow rules only allow merge request pipelines, the branch pipeline can be filtered out and its ref request fail before UpdateMergeRequestsWorker registers the MR request. That leaves a short window where no request is in progress. The window is smaller than on master, where no ref request exists at all.

  • Retries-exhausted handlers in NewMergeRequestWorker and Repositories::PostReceiveWorker only fail requests that are still in progress. If the last attempt handed the request to a pipeline worker seconds earlier, the handler can mark it failed before that worker writes its own result. The window is seconds, after four failed attempts.
  • On a transient Gitaly read error, CreatePipelineWorker rewinds the request from failed to in_progress with the flag on. The two writes happen in the same job, milliseconds apart.
  • If MergeRequests::UpdateHeadPipelineWorker keeps failing, the ref request stays in progress until it expires (300s). The worker uses Sidekiq's default 25 retries, so a retries-exhausted handler would never see the request.

Before enabling the flag:

  • When git_push_pipeline_limit is 0, PostReceiveService writes one request per pushed branch with no cap. Add a cap or a pipelined write.
  • Optionally, move the retry decision in CreatePipelineWorker ahead of the service's terminal write.

References

Edited by Peter Gumeson

Merge request reports