Runway DLQ answers FAILED for a merge whose push already landed

Open
#820 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 2 days

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
55/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
go

Research direction

Start with runway/controller/dlq/dlq.go and the merger’s existing branch-containment check used for idempotence; the issue does not name that code’s file. Add a unit test for a successful push followed by failed answer publishing until dead-lettering. Done means the DLQ answers MERGED when the request’s changes are already on the target branch and never reports that landed request as FAILED.

Written by the indexing model from the issue text.

Description

bug

Problem

Runway can tell SubmitQueue that a merge failed after the merge has landed on the target branch. landsignal applies the FAILED, and the batch and its requests are reported failed for a change that is on main.

Cause

Runway's merge DLQ (runway/controller/dlq/dlq.go) answers FAILED unconditionally; Runway is stateless and never checks whether the push happened. The merge controller pushes first and publishes its answer afterwards:

runway merge:  push lands on the target branch
runway merge:  publishing SUCCEEDED keeps failing; retries re-run the merge (idempotent: no new commits)
               → retries exhaust → runway-merge DLQ
runway DLQ:    publish FAILED under a distinct message ID, so it is not deduped
landsignal:    batch landing → failed

The git merger's redelivery idempotence covers the merge controller's retries. Nothing covers the DLQ's answer. No orchestrator-side fix (#819) can repair this.

Direction

Check the branch before answering: answer MERGED when the request's changes are already contained in the target ref, FAILED otherwise. The merger already computes this for idempotence. Note the residual hazard recorded in #819: once the DLQ has honestly answered FAILED, a later re-send of the same request must not merge.

Found by model checking spec/submitqueue/landoutcome/LandOutcome.tla.

Acceptance criteria

  • Runway's DLQ never answers FAILED for a request whose changes are on the target branch.
  • Unit test: the push succeeds, the answer publish fails until dead-lettered, and the DLQ answers MERGED.
Dominant language
Go
Stars
247
Forks
16
Avg merge
2d 1h
Merged PRs (30d)
125

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from uber/submitqueue

All issues in uber/submitqueue

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.