Skip to content

Commit ca04983

Browse files
alcoclaudemagnetised
authored
fix(sync-service): prevent permanent flush-boundary stalls from unexpected consumer lifecycle transitions (#4730)
## Problem The FlushTracker pins `confirmed_flush_lsn` for 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 explicit `ShapeLogCollector.remove_shape/2` arriving via the ShapeCleaner chain. Several real-world paths break both — a storage error that crashes the consumer *and* aborts its `terminate/2` before `handle_writer_termination` runs, a removal chain that dies between the `ShapeStatus` removal 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. - **Writer monitors**: the SLC monitors every consumer with an incomplete FlushTracker entry (created when a commit-fragment publish tracks the shape, released when the entry completes or the shape is removed). DOWNs are classified by exit reason: crashes unpin the flush boundary immediately and trigger shape invalidation; deliberate-cleanup DOWNs (`{:shutdown, :cleanup}`) unpin only; bare `:shutdown`/`:killed` are assumed to be supervisor teardown and left alone. - **Stall detection**: incomplete entries carry a `last_progress_at` timestamp refreshed only by flush notifications. A periodic tick finds shapes whose entries make no flush progress for `ELECTRIC_FLUSH_STALL_GRACE_PERIOD` (default 60s) and invalidates them 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. - **Stall challenge-response**: a stalled entry whose writer is still alive is challenged (`:verify_flush_progress`) before being acted on. A consumer answers only while legitimately deferring flush notifications (buffering ahead of PG snapshot info, subquery move-in awaiting splice); the answer re-arms the entry's grace period so healthy deferral phases are not misread as stalls. Only a suspect still stalled and silent at the next check — or a shape whose writer is already dead and demonitored, which skips the challenge — is invalidated. Consumers stay silent in steady state: no periodic messaging, and a wedged consumer can't answer, so the backstop stays armed. - **Reason normalization**: the benign clause of `handle_writer_termination` shrinks to exactly `:normal`/`:killed`/`:shutdown`, so untagged `{:shutdown, x}` reasons now trigger removal; tagged dependency-materializer shutdowns map to `stop_and_clean`; suspend additionally requires all txn state flushed and notified (guaranteeing a suspended consumer never leaves an incomplete entry behind); `Consumer.stop` escalates a wedged consumer to a kill after the stop call times out. - ShapeCleaner re-issues the idempotent SLC removal even when the shape is already gone from `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](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Garry Hill <garry@electric-sql.com>
1 parent 88793e7 commit ca04983

18 files changed

Lines changed: 1244 additions & 140 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@core/sync-service": patch
3+
"@core/electric-telemetry": patch
4+
---
5+
6+
Prevent a dead or stalled shape consumer from pinning the replication slot's `confirmed_flush_lsn` indefinitely, which caused unbounded WAL retention. The collector now monitors the writer behind every pending flush entry — a crashed writer unpins its entry immediately, and a shape making no flush progress past a grace period is challenged and invalidated if it doesn't respond.
7+

‎packages/electric-telemetry/lib/electric/telemetry/stack_telemetry.ex‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,9 @@ defmodule ElectricTelemetry.StackTelemetry do
135135
last_value("electric.admission_control.acquire.limit", tags: [:kind]),
136136
sum("electric.admission_control.reject.count", tags: [:kind]),
137137
last_value("electric.admission_control.reject.limit", tags: [:kind]),
138-
distribution("electric.shape_log_collector.transaction.affected_shape_count")
138+
distribution("electric.shape_log_collector.transaction.affected_shape_count"),
139+
counter("electric.flush_tracker.writer_down.count", tags: [:reason_class]),
140+
sum("electric.flush_tracker.stall_detected.count")
139141
| additional_metrics(telemetry_opts)
140142
]
141143
|> ElectricTelemetry.keep_for_stack(telemetry_opts.stack_id)

‎packages/sync-service/config/runtime.exs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ shape_enable_suspend? = env!("ELECTRIC_SHAPE_SUSPEND_CONSUMER", :boolean, nil)
173173
shape_suspend_after =
174174
env!("ELECTRIC_SHAPE_SUSPEND_AFTER", &Electric.Config.parse_human_readable_time!/1, nil)
175175

176+
flush_stall_grace_period =
177+
env!("ELECTRIC_FLUSH_STALL_GRACE_PERIOD", &Electric.Config.parse_human_readable_time!/1, nil)
178+
176179
system_metrics_poll_interval =
177180
env!(
178181
"ELECTRIC_SYSTEM_METRICS_POLL_INTERVAL",
@@ -276,6 +279,7 @@ config :electric,
276279
shape_hibernate_after: shape_hibernate_after,
277280
shape_enable_suspend?: shape_enable_suspend?,
278281
shape_suspend_after: shape_suspend_after,
282+
flush_stall_grace_period: flush_stall_grace_period,
279283
storage_dir: storage_dir,
280284
storage: storage_spec,
281285
cleanup_interval_ms:

‎packages/sync-service/lib/electric/application.ex‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ defmodule Electric.Application do
154154
shape_hibernate_after: get_env(opts, :shape_hibernate_after),
155155
shape_enable_suspend?: get_env(opts, :shape_enable_suspend?),
156156
shape_suspend_after: get_env(opts, :shape_suspend_after),
157+
flush_stall_grace_period: get_env(opts, :flush_stall_grace_period),
157158
conn_max_requests: get_env(opts, :conn_max_requests),
158159
handler_fullsweep_after: get_env(opts, :handler_fullsweep_after),
159160
process_spawn_opts: get_env(opts, :process_spawn_opts),

‎packages/sync-service/lib/electric/config.ex‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ defmodule Electric.Config do
104104
# After hibernating, wait this duration before suspending (terminating).
105105
# Only applies when shape_enable_suspend? is true.
106106
shape_suspend_after: :timer.minutes(10),
107+
# How long a pending flush entry may sit without flush progress before its
108+
# shape is invalidated to unpin the stack-wide flush boundary. The storage
109+
# contract already says writes slower than this should raise.
110+
flush_stall_grace_period: :timer.minutes(1),
107111
# Sets max_requests for Bandit handler processes:
108112
# https://hexdocs.pm/bandit/Bandit.html#t:http_1_options/0
109113
# "The maximum number of requests to serve in a single HTTP/{1,2}

0 commit comments

Comments
 (0)