Skip to content

Commit 9dfa0eb

Browse files
erik-the-implementeralcoclaude
authored
fix(sync-service): don't suspend a consumer mid-transaction (#4501) (#4503)
## Summary Fixes #4501 — `KeyError: key :consider_flushed? not found in: nil` in `Electric.Shapes.Consumer.process_txn_fragment/2`. A shape consumer could **suspend (terminate to reclaim memory) on its idle timeout while still holding a `pending_txn`** for an in-flight multi-fragment transaction. The producer's `EventRouter` tracks "this shape already saw the begin for the current xid" keyed by `shape_handle`, independently of consumer liveness — so when a later fragment of that transaction arrived, `ConsumerRegistry` started a fresh consumer and delivered a `has_begin?: false` fragment to it. The fresh consumer has `pending_txn: nil`, so `process_txn_fragment/2` dereferenced `nil` at `txn.consider_flushed?`. ## Fix `consumer_can_suspend?/1` now also requires `is_nil(state.pending_txn)`. A consumer that is mid-transaction hibernates instead of suspending, and only suspends once the transaction completes and it goes idle again. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Oleksii Sholik <oleksii@sholik.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d3e1a64 commit 9dfa0eb

3 files changed

Lines changed: 77 additions & 1 deletion

File tree

.changeset/fix-suspend-mid-txn.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@core/sync-service": patch
3+
---
4+
5+
Fix a `KeyError: key :consider_flushed? not found in: nil` crash in `Electric.Shapes.Consumer`. `consumer_can_suspend?/1` now refuses to suspend while a transaction is pending, so the consumer hibernates instead and suspends only once the transaction completes.

packages/sync-service/lib/electric/shapes/consumer.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,7 @@ defmodule Electric.Shapes.Consumer do
399399
# 3. we are not part of a subquery dependency tree, that is either
400400
# a. we have no dependent shapes
401401
# b. we don't have a materializer subscribed
402+
# 4. we're not in the middle of processing a multi-fragment transaction
402403

403404
if consumer_suspend_enabled?(state) and consumer_can_suspend?(state) do
404405
Logger.debug(fn -> ["Suspending consumer ", to_string(state.shape_handle)] end)
@@ -416,7 +417,7 @@ defmodule Electric.Shapes.Consumer do
416417

417418
defp consumer_can_suspend?(state) do
418419
is_snapshot_started(state) and not Shape.has_dependencies(state.shape) and
419-
not state.materializer_subscribed?
420+
not state.materializer_subscribed? and is_nil(state.pending_txn)
420421
end
421422

422423
@impl GenServer

packages/sync-service/test/electric/shapes/consumer_test.exs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,6 +1461,76 @@ defmodule Electric.Shapes.ConsumerTest do
14611461
assert is_pid(Consumer.whereis(ctx.stack_id, shape_handle))
14621462
end
14631463

1464+
@tag hibernate_after: 10, with_pure_file_storage_opts: [flush_period: 1]
1465+
@tag suspend: true
1466+
test "should hibernate not suspend while a multi-fragment transaction is pending", ctx do
1467+
register_as_replication_client(ctx.stack_id)
1468+
1469+
{shape_handle, _} = ShapeCache.get_or_create_shape_handle(@shape1, ctx.stack_id)
1470+
lsn1 = Lsn.from_integer(300)
1471+
1472+
:started = ShapeCache.await_snapshot_start(shape_handle, ctx.stack_id)
1473+
1474+
consumer_pid = Consumer.whereis(ctx.stack_id, shape_handle)
1475+
assert is_pid(consumer_pid)
1476+
ref = Process.monitor(consumer_pid)
1477+
1478+
# The begin fragment of a multi-fragment transaction leaves the consumer
1479+
# holding a pending_txn until the matching commit fragment arrives.
1480+
begin_fragment =
1481+
txn_fragment(
1482+
2,
1483+
lsn1,
1484+
[
1485+
%Changes.NewRecord{
1486+
relation: {"public", "test_table"},
1487+
record: %{"id" => "21"},
1488+
log_offset: LogOffset.new(lsn1, 0)
1489+
}
1490+
],
1491+
has_begin?: true,
1492+
has_commit?: false
1493+
)
1494+
1495+
assert :ok = ShapeLogCollector.handle_event(begin_fragment, ctx.stack_id)
1496+
1497+
# The idle timer (hibernate_after: 10ms) fires, but with a transaction still
1498+
# pending the consumer must hibernate rather than suspend, so it survives to
1499+
# receive the rest of the transaction. Suspending here would drop pending_txn
1500+
# and crash on the next fragment (issue #4501).
1501+
refute_receive {:DOWN, ^ref, :process, ^consumer_pid, {:shutdown, :suspend}}, 400
1502+
1503+
assert {:current_function, {:gen_server, :loop_hibernate, 4}} =
1504+
Process.info(consumer_pid, :current_function)
1505+
1506+
assert is_pid(Consumer.whereis(ctx.stack_id, shape_handle))
1507+
1508+
# Completing the transaction clears pending_txn, so the consumer is free to
1509+
# suspend on the next idle timeout.
1510+
commit_fragment =
1511+
txn_fragment(
1512+
2,
1513+
lsn1,
1514+
[
1515+
%Changes.NewRecord{
1516+
relation: {"public", "test_table"},
1517+
record: %{"id" => "22"},
1518+
log_offset: LogOffset.new(lsn1, 1)
1519+
}
1520+
],
1521+
has_begin?: false,
1522+
has_commit?: true
1523+
)
1524+
1525+
assert :ok = ShapeLogCollector.handle_event(commit_fragment, ctx.stack_id)
1526+
1527+
assert_receive {:flush_boundary_updated, 300}, 1_000
1528+
1529+
assert_receive {:DOWN, ^ref, :process, ^consumer_pid, {:shutdown, :suspend}}
1530+
1531+
refute Consumer.whereis(ctx.stack_id, shape_handle)
1532+
end
1533+
14641534
@tag with_pure_file_storage_opts: [flush_period: 1]
14651535
@tag suspend: false
14661536
test "ConsumerRegistry.enable_suspend should suspend hibernated consumers", ctx do

0 commit comments

Comments
 (0)