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

Abierto
  • ¿Algo no está bien?
#819 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 2 días

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
5/5
Tiempo estimado
Más de una semana
Aptitud para principiantes
35/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Activo
Stack tecnológico
go
Área
distributed-systems

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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).

Lenguaje dominante
Go
Estrellas
247
Forks
16
Merge medio
2 d 1 h
PR fusionados (30 d)
125

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de uber/submitqueue

Todos los issues de uber/submitqueue

Issues similares

Más issues de Go

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.