Skip to content

🐛 fix(storage): replay the outcome the transaction read - #2016

Merged
gaborbernat merged 1 commit into
mainfrom
fix/stable-finalize-replays-1374
Aug 31, 2026
Merged

🐛 fix(storage): replay the outcome the transaction read#2016
gaborbernat merged 1 commit into
mainfrom
fix/stable-finalize-replays-1374

Conversation

@gaborbernat

Copy link
Copy Markdown
Member

A finalize retry that found a terminal outcome inside its write transaction discarded that record, and resolve_finalize read the row back from the store after the transaction had ended. write_ledger_reap runs prune_operation_outcomes on its own schedule and deletes terminal outcomes past their retention cutoff, so it can delete that row inside the window. The second read returns None and the expect panics with a race-replay observed a terminal outcome that is still stored. Nothing catches that panic. The finalize task never sends a response, so a client retrying an upload that had already published loses the connection instead of getting the acknowledgement the transaction was holding, and the operator reads a panic for a request that was correct.

FinalizeFlow::RaceReplay now carries the OperationOutcomeRecord the transaction observed, and the replay returns that value, leaving no second read for the reaper to race. resolve_finalize no longer touches the store and drops to a free function over the flow.

flowchart LR
    Txn[Finalize transaction<br/>reads terminal outcome] --> Flow[RaceReplay carries the record]
    Txn --> Prune[Reaper deletes the expired row]
    Flow --> Resolve[Replay returns the observed record]
    Prune -. no longer reaches .-> Resolve
    classDef accent fill:#cfe4ff,stroke:#1f6feb,color:#0b1f3a;
    classDef warn fill:#ffe3a3,stroke:#d29200,color:#3a2c00;
    class Txn,Flow,Resolve accent;
    class Prune warn;
Loading

#1388 was the same defect in the policy-decision tables and took a different remedy. There a bounded audit log held live state, so live state moved to a table the pruner does not touch. OPERATION_OUTCOME is the idempotency ledger and evicting expired terminal rows is what its retention exists for, so this change drops the redundant read and leaves the pruner alone.

Closes #1374

A finalize retry that found a terminal outcome inside its write transaction
threw that record away and read the row again once the transaction had ended.
The write-ledger reaper deletes expired terminal outcomes on its own schedule,
so it could delete the row in that window. The second read then returned None
and the expect panicked, failing a retry whose answer the transaction had
already held.

FinalizeFlow::RaceReplay now carries the record, leaving no second read for
retention to race. Splitting live state away from the pruned table, the remedy
#1388 needed, does not apply here: OPERATION_OUTCOME is the idempotency ledger
itself and evicting expired terminal rows is what its retention is for.

Closes #1374
@gaborbernat gaborbernat added the bug Something isn't working label Aug 31, 2026
@read-the-docs-community

Copy link
Copy Markdown

@codspeed-hq

codspeed-hq Bot commented Aug 31, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 29 untouched benchmarks
⏩ 133 skipped benchmarks1


Comparing fix/stable-finalize-replays-1374 (1668dc7) with main (51c3b19)

Open in CodSpeed

Footnotes

  1. 133 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@gaborbernat
gaborbernat merged commit da3120a into main Aug 31, 2026
27 checks passed
@gaborbernat
gaborbernat deleted the fix/stable-finalize-replays-1374 branch August 31, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep finalize replays stable across outcome pruning

1 participant