Land and landsignal DLQs fail a `landing` batch that Runway goes on to merge

未关闭
  • 哪里不对？
#819 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 2 天内回复

还没有人认领这个 Issue。

评估

难度
5/5
预计耗时
一周以上
新手友好度
35/100
Issue 类型
缺陷
描述清晰度
基本清楚
活跃度
活跃
技术栈
go
领域
distributed-systems

调研方向

Start with submitqueue/orchestrator/controller/dlq/dlq.go and the interleaving described in the issue; then read spec/submitqueue/landoutcome/LandOutcome.tla and compare the design options. Add tests that interleave a DLQ with a late SUCCEEDED and confirm they fail before the fix. Done means the chosen design updates the model and meets all four acceptance criteria without allowing an incorrect terminal outcome.

由索引模型根据 Issue 内容生成。

描述

bug

Problem

A batch is reported failed, and its requests error, even though Runway merged the change onto the target branch.

Cause

failBatch (submitqueue/orchestrator/controller/dlq/dlq.go) moves any non-terminal batch to failed, landing included. A landing batch has already handed its outcome to Runway, so the reconciler cannot know whether the merge happened. Two DLQs reach failBatch while the batch is landing:

  • submitqueue-land DLQ. The Runway publish is land's last step, so a lost ack, or an ambiguous publish error after the row was written, leaves Runway holding the MergeRequest. If later attempts then fail on a storage error, the message dead-letters.
  • landsignal DLQ. landsignal holds Runway's SUCCEEDED, but the batch compare-and-swap keeps failing on a storage fault. The DLQ payload contains the verdict, and the reconciler discards it.

When Runway's answer arrives later, landsignal sees a terminal batch, re-fans out, and acks. Nothing corrects the outcome.

Interleaving (TLC's shortest counterexample)

speculate:   batch speculating → landing, publish to submitqueue-land
land:        publish MergeRequest to runway-merge, then the attempt fails; retries exhaust → land DLQ
runway:      merges, publishes SUCCEEDED
land DLQ:    failBatch: landing → failed, fan out requests → error
landsignal:  batch already terminal → ack, outcome unchanged

Why the obvious fixes are not enough

Found by model checking spec/submitqueue/landoutcome/LandOutcome.tla, which checks 24 design combinations:

  • Skip landing in the DLQ and wait for Runway. This trades the wrong answer for a batch that never finishes, when Runway's answer is itself the message that dead-lettered.
  • Re-send the MergeRequest from the DLQ. This is unsound against today's Runway, for two reasons:
    • A stale FAILED can win. Runway's first attempt dead-letters without pushing, so its DLQ honestly answers FAILED. The re-send then merges on a second attempt, and the earlier FAILED is applied.
    • The replay is swallowed. A replayed answer reuses the request ID as its message ID and is deduped by the queue (the #352 class).
  • The only design that passes every property has four parts: the DLQ re-sends; Runway's DLQ checks the branch; Runway records its verdict per request ID; and answers do not dedup on the request ID alone. That gives Runway state, so cheaper designs are worth modelling first. Two candidates: the landsignal DLQ applies the verdict it holds, and land records "dispatched" before it publishes.

Acceptance criteria

  • A dead letter on submitqueue-land or landsignal never moves a landing batch to failed while the change can still land or has landed.
  • Every landing batch still reaches a terminal state.
  • Unit tests that interleave the DLQ with a late SUCCEEDED, confirmed to fail before the fix.
  • LandOutcome.tla updated to the chosen design.

Related: #818 (the same symptom through the speculate path).

主要语言
Go
星标
247
派生
16
平均合并
2 天 1 小时
30 天内合并 PR
125

环境准备

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

uber/submitqueue 的其他 Issue

查看 uber/submitqueue 的全部 Issue

相似的 Issue

更多 Go Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。