Fix flush-boundary accounting that got healthy subquery shapes killed after rolling deploys - #4744
Open
robacourt wants to merge 7 commits into
Open
Fix flush-boundary accounting that got healthy subquery shapes killed after rolling deploys#4744robacourt wants to merge 7 commits into
robacourt wants to merge 7 commits into
Conversation
…gnment
A txn_offset_mapping entry {written, boundary} promises "once the writer
has flushed `written`, this shape has processed everything up to
`boundary`". Subquery consumers append move-in/move-out control messages
to the shape log with no boundary bookkeeping of their own, so a storage
flush can land past a mapped written offset without matching it exactly.
align_offset_to_txn_boundary/2 dropped such entries silently and reported
the raw flushed offset, which can sit *below* the dropped boundary — e.g.
splice writes end at (tx,16) with boundary (tx,18), a drained move-out
control message lands at (tx,17), storage flushes (tx,17). The
ShapeLogCollector's flush entry for that transaction then waits for
(tx,18) forever while the consumer has nothing left to flush, pinning the
stack-wide flush boundary and, since the stall watchdog landed, getting
the healthy shape removed after 60s of quiet WAL — observed as spurious
409s for optimized subquery shapes in the first batch after a rolling
deploy (shared dependency shapes cascade their removal into every
dependent).
Report max(flushed offset, max dropped boundary) instead — the same claim
the exact-match path already made, generalized to non-exact flushes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…last_sent FlushTracker cleared a shape's pending-flush entry only when the consumer's notification exactly equalled the recorded last_sent offset. Subquery consumers write move-in/move-out control messages at op offsets past the transaction's last replicated operation, so their flush notifications can overshoot last_sent — e.g. notified (tx,19) against last_sent (tx,18). The entry then survived forever with last_flushed > last_sent: the consumer owed nothing, the entry could never complete, the global flush boundary (and the slot's confirmed_flush_lsn) dragged behind, and the stall watchdog eventually removed the healthy shape. A shape's log offsets are monotone, so a flush at X covers every write at or below X: a notification at or past last_sent means the shape has durably flushed everything it was sent and is caught up. The changeset covers this and the companion consumer-side alignment fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shape-db sqlite lived at storage_dir/meta/shape-db with storage_dir = ctx.tmp_dir, unscoped by stack_id — so the two stacks of a rolling-deploy test (old and replacement, sharing one tmp_dir) shared one metadata database while PureFileStorage keeps their shape data in separate per-stack namespaces. The replacement stack then "restored" hundreds of shape records it had no data for: a storage layout no production deploy has (per-instance disks share neither; a shared bind-mount shares both). Non-optimized restored shapes churned through 409/refetch cycles, stretching post-restart checks by minutes, and a race let a dataless restored shape serve an empty snapshot as up-to-date. Scope the shape-db dir by stack_id, placed *beside* the PureFileStorage data namespace (tmp_dir/<stack_id>) rather than inside it — reset_storage/cleanup_all! trashes that whole directory and must never take the metadata db with it. Graceful and brutal restarts reuse the stack_id and therefore the same path, so restore-from-disk scenarios are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… harness
Each rolling restart derived the replacement stack's id from the previous
generation's id, compounding the suffix ("…_roll1_roll2_…"). Two
consequences: registry-name atoms eventually exceed Erlang's 255-char
atom limit, crashing the replacement stack's boot around the 17th restart
of a long run (StreamData then swallows the exception and re-runs the
test body against a stale ctx, surfacing only as connection-refused
noise); and — worse for fidelity — the slot name was hashed from the
evolving id, so from the second restart onward each replacement silently
created a fresh replication slot instead of contending for and taking
over the original one. Only the first restart ever exercised a true
rolling-deploy slot handover.
Derive every generation's id and the shared slot name from the base
(pre-roll) stack_id, carried through the ctx as base_stack_id: ids stay
bounded and all 66 restarts of a 200-batch oracle run now perform a
genuine advisory-lock handover on one slot.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shared Finch pool for many-client oracle tests used the default pool protocol, which resolves to HTTP/2: every checker multiplexes onto a handful of connections and excess concurrent streams are shed with :pool_not_available, randomly starving checkers under 800 concurrent long-polls — sometimes for a full check window, failing the run as "server unreachable". An http1 pool sized to the client count queues instead of shedding. The `default` pool key applies to every origin, so replacement servers on new ports (rolling deploys) inherit the same configuration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ExUnit's capture_log swallows all server-side logs in the long-running oracle tests, including on failure, which made server-side diagnosis of oracle failures effectively impossible. When ORACLE_LOG_FILE is set, attach a plain :logger_std_h handler writing every Logger event at or above ORACLE_LOG_LEVEL (default info) to the given file, bypassing the capture. No-op when the variable is unset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eploys Adds "rolling" to the restart_type matrix alongside graceful and brutal: a replacement stack with its own storage contends on the running stack's replication slot via the advisory lock and takes over when the old stack stops, exercising slot handover, WAL redelivery into freshly created shapes, and the flush-boundary accounting fixed in the preceding commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
robacourt
requested review from
KyleAMathews,
alco,
balegas,
icehaunter,
kevin-dp,
magnetised,
msfstef,
paulharter,
samwillis and
thruflo
as code owners
July 28, 2026 12:46
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## rob/drop-subqueries-on-restart #4744 +/- ##
===============================================================
Coverage 60.01% 60.01%
===============================================================
Files 397 397
Lines 43766 43766
Branches 12586 12588 +2
===============================================================
+ Hits 26267 26268 +1
+ Misses 17418 17416 -2
- Partials 81 82 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the deterministic failure of the oracle property test under
RESTART_TYPE=rolling: freshly created, healthy subquery shapes were being removed by the flush-stall watchdog in the first batch after a rolling deploy, surfacing to clients as spurious409 must-refetchon shapes that should never invalidate. Two flush-accounting bugs are fixed, four test-harness bugs that either masked or corrupted the rolling scenario are fixed, androllingis added to the oracle restart CI matrix.Related: #3980, #3983 (the FlushTracker stall family these bugs fed), #4730 (the stall watchdog that turned the silent pinning into shape removals).
Problem
ShapeLogCollector's FlushTracker records, per shape, thelast_log_offsetof each routed transaction and considers the shape caught up only when the consumer's flush notification exactly equals it. Subquery consumers append move-in/move-out control messages to the shape log at op offsets past the transaction's last replicated operation, with no flush-boundary bookkeeping of their own. That breaks the exact-match contract in both directions:(tx,16)with mapped boundary(tx,18); a drained move-out control message lands at(tx,17); storage flushes(tx,17).align_offset_to_txn_boundary/2dropped the{(tx,16), (tx,18)}mapping entry — boundary and all — and notified the raw(tx,17), below the boundary, forever.(tx,19)againstlast_sent (tx,18); exact-match keeps the entry alive even though the shape owes nothing.Either way the entry can never complete. It drags
last_global_flushed_offset(so the slot'sconfirmed_flush_lsnlags — the WAL-retention symptom described in #3980), and since #4730 the stall watchdog removes the "stalled" shape after 60s of quiet WAL — the consumer truthfully answers the challenge with "not deferring" because, from its side, everything is flushed. Removing a shared subquery dependency shape cascadesmaterializer_shape_invalidatedinto every dependent, hence the 409s.Rolling deploys expose it deterministically: after a handover every shape is recreated from scratch, the first post-restart batch triggers a wave of move-ins/move-outs on fresh shapes, and the long quiet check window that follows exceeds the stall grace period. Graceful/brutal restarts wipe the tracker every few batches and never leave a long-enough quiet window.
Reproduction
Failed 4/4 runs at batch 4 (first batch after the first rolling restart) with
Unexpected 409 (must-refetch) in optimized shape=…, preceded in server logs byWriters for shapes [16 handles] have made no flush progress in over 60000ms, removing the shapes to unpin the flush boundary.Solution
One commit per bug:
fix: carry dropped txn boundaries in consumer flush alignment—align_offset_to_txn_boundary/2reportsmax(flushed offset, max dropped boundary), the same claim the exact-match path already made, generalized to non-exact flushes.fix: complete FlushTracker entries on flush at or past last_sent— a shape's log offsets are monotone, so a flush at X covers everything ≤ X; at-or-pastlast_sentmeans caught up.test: scope the shape metadata db per stack in the harness— both rolling stacks shared one shape-db sqlite (metadata restored, data absent — a layout no production deploy has), causing 409/refetch churn and an empty-snapshot-served-as-up-to-date race.test: stop compounding rolling-restart stack ids— compounded ids (…_roll1_roll2_…) blow the 255-char atom limit around the 17th restart, and the slot name hashed from the evolving id meant only the first restart ever exercised a real slot takeover. All 66 restarts of a 200-batch run now contend on one slot.test: use sized http1 Finch pools— the h2 pool sheds excess concurrent streams aspool_not_available, randomly starving checkers under 800 concurrent long-polls.test: add ORACLE_LOG_FILE— opt-in file mirror for server logs, bypassingcapture_log, which was swallowing all server-side evidence.ci: add rolling to the oracle restart matrix.Test plan
flush_tracker_test.exs("a notification past last_sent completes the shape", "…not reported as stalled"),state_test.exs("carries a dropped boundary forward…", "uses the max dropped boundary…")mix test: 2278 passedrollingmatrix leg oforacle_property_test_with_restartsis green🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.