fix(sync-service): prevent permanent flush-boundary stalls from unexpected consumer lifecycle transitions#4730
Draft
alco wants to merge 5 commits into
Draft
fix(sync-service): prevent permanent flush-boundary stalls from unexpected consumer lifecycle transitions#4730alco wants to merge 5 commits into
alco wants to merge 5 commits into
Conversation
Adds last_progress_at (caller-injected monotonic ms) to incomplete flush entries, refreshed only by flush notifications; new stalled_shapes/3 and touch/3 API; handle_txn_fragment reports newly-tracked shapes. Groundwork for SLC writer monitors and stall detection (spec electric-2026-07-16-flush-tracker-writer-monitors). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… suspend contract
Shrinks the benign clause of handle_writer_termination to exactly
:normal/:killed/:shutdown so untagged {:shutdown, x} reasons trigger
shape removal; maps tagged dependency-materializer shutdowns to
stop_and_clean instead of propagating the reason verbatim; suspend now
additionally requires all txn state flushed AND notified; Consumer.stop
escalates a wedged consumer to a kill after the stop call times out.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lled shapes The SLC now holds a monitor on every consumer with an incomplete FlushTracker entry: crash DOWNs unpin the flush boundary immediately and trigger shape invalidation; deliberate-cleanup DOWNs unpin only; bare :shutdown/:killed DOWNs are left to the grace-period stall tick, which invalidates shapes whose entries make no flush progress for ELECTRIC_FLUSH_STALL_GRACE_PERIOD (default 60s). Fixes the permanent confirmed_flush_lsn stall when a consumer dies without cleanup on a quiet table. Includes changeset and telemetry events. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…acking
- Re-issue the ShapeLogCollector removal from the ShapeCleaner when the
shape is already gone from ShapeStatus: an earlier removal chain that
died between the ShapeStatus removal and the SLC removal used to leave
the shape's flush entry pinned forever, because every stall-check
re-fire short-circuited at {:error, :data_removed}. The SLC removal is
idempotent, so the retry now issues it unconditionally.
- Stop the stall check from invalidating healthy shapes that deliberately
defer flush notifications. A consumer buffering transactions ahead of
PG snapshot info, or sitting in a subquery move-in awaiting splice, can
legitimately produce no flush notifications for longer than the grace
period. While in either deferral phase the consumer now periodically
casts notify_flush_deferred to the SLC (at a third of the grace
period), which touches its FlushTracker entry and re-arms the grace
window. The tick stops itself once the deferral phase ends, and a
wedged consumer can't send it, so the stall backstop stays intact.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4730 +/- ##
==========================================
+ Coverage 60.04% 60.21% +0.17%
==========================================
Files 397 412 +15
Lines 43766 44396 +630
Branches 12586 12590 +4
==========================================
+ Hits 26281 26735 +454
- Misses 17403 17580 +177
+ Partials 82 81 -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:
|
…t is delivered to
Monitors were only established for shapes newly tracked by a commit, so
when the suspend-retry path delivered a commit for an already-tracked
shape to a fresh consumer pid, the monitor stayed on the suspended
predecessor. Its queued {:shutdown, :suspend} DOWN was then classified
as a contract-violating crash, spuriously invalidating a healthy shape
and unpinning its boundary while the fresh consumer was still writing.
Monitor every delivered shape that is still tracked after the commit
instead: the swap clause hands the monitor over and flushes the old
pid's queued DOWN. This also makes FlushTracker.handle_txn_fragment's
newly-tracked return value redundant, so it returns just the tracker
again.
Also derive the stall-check re-arm interval from the configured grace
period (clamped to [1s, 10s]) so a short grace period is enforced at
matching granularity, and add test coverage for the monitor hand-over
and for crash classification of exits observed at publish time.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Problem
The FlushTracker pins
confirmed_flush_lsnfor the whole stack on the minimum incomplete flush entry, and cleanup of entries is purely event-driven: either new traffic routed to the same shape (publish-time DOWN detection) or an explicitShapeLogCollector.remove_shape/2arriving via the ShapeCleaner chain. Several real-world paths break both — a storage error that crashes the consumer and aborts itsterminate/2beforehandle_writer_terminationruns, a removal chain that dies between theShapeStatusremoval and the SLC removal (unretryable: the retry short-circuits at{:error, :data_removed}), untagged{:shutdown, x}exit reasons classified as benign, and a consumer that stays alive but stops flushing. On a subsequently-quiet table any of these pins the WAL ack forever, causing unbounded WAL growth on Postgres. PR #4713 has a minimal reproduction of the stall.Solution
The SLC becomes the authority on consumer-death handling for flush accounting. Invariant: an incomplete FlushTracker entry always has a live monitor watching the pid responsible for completing it, and a timestamp bounding how long it may sit without flush progress.
{:shutdown, :cleanup}) unpin only; bare:shutdown/:killedare assumed to be supervisor teardown and left alone.last_progress_attimestamp refreshed only by flush notifications. A periodic tick invalidates shapes whose entries make no flush progress forELECTRIC_FLUSH_STALL_GRACE_PERIOD(default 60s) via the normal ShapeCleaner path, re-arming on each fire so a lost cleanup chain retries instead of pinning forever. Misclassified DOWNs self-heal through the same mechanism. Consumers that legitimately defer flush notifications (buffering ahead of PG snapshot info, subquery move-in awaiting splice) periodically notify the SLC so healthy deferral phases are not misread as stalls.handle_writer_terminationshrinks to exactly:normal/:killed/:shutdown, so untagged{:shutdown, x}reasons now trigger removal; tagged dependency-materializer shutdowns map tostop_and_clean; suspend additionally requires all txn state flushed and notified (guaranteeing a suspended consumer never leaves an incomplete entry behind);Consumer.stopescalates a wedged consumer to a kill after the stop call times out.ShapeStatus, healing pins left by removal chains that died partway through.Telemetry: new
[:electric, :flush_tracker, :writer_down]and[:electric, :flush_tracker, :stall_detected]events.🤖 Generated with Claude Code