Skip to content

Commit 680a81a

Browse files
authored
Recycle worker connections when the fetch loop reconnects (#263)
After a silent primary loss the fetch loop hit query_timeout_seconds and reconnected, but each worker then wrote on its own idle connection to the dead primary and waited out a second timeout. Workers now track a generation bumped on every fetch-loop error; a worker whose connection predates it requeues the row, drops the connection and reconnects before running the handler.
1 parent 8396b98 commit 680a81a

4 files changed

Lines changed: 95 additions & 5 deletions

File tree

‎docs/operations/checklist.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ story.
2828
- [ ] Engine URL follows the primary across a failover (a proxy, or a
2929
multi-host URL with `target_session_attrs=primary`). Subscribers with a
3030
long lease set a shorter `query_timeout_seconds`; recovery from a primary
31-
that went silent takes about twice that value. See [Subscriber § Database
32-
failover](../usage/subscriber.md#database-failover).
31+
that went silent takes about that value plus `max_fetch_interval`. See
32+
[Subscriber § Database failover](../usage/subscriber.md#database-failover).
3333
- [ ] Slow handlers segregated onto their own subscriber with a
3434
taller `lease_ttl_seconds`. Don't raise it globally; that delays
3535
reclaim of *actually* stuck rows everywhere.

‎docs/usage/subscriber.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,9 +383,10 @@ How fast the subscriber recovers depends on how the old primary went away:
383383
- **It went silent** (the host lost power, or the network was partitioned).
384384
Statements hang because no error ever arrives. Each fetch and terminal write
385385
is capped at `query_timeout_seconds`, after which the connection is dropped and
386-
the loop reconnects. The fetch loop hits that cap first; the worker hits it
387-
again on its own stale connection once it gets a row. Recovery takes about
388-
`2 × query_timeout_seconds`, which is two minutes with the default 60 s lease.
386+
the loop reconnects. When the fetch loop reconnects, the workers drop their
387+
connections too before writing again. Recovery takes about
388+
`query_timeout_seconds` plus up to `max_fetch_interval` for an idle subscriber
389+
to run its next fetch, which is about 70 s with the defaults.
389390
A subscriber with a long lease for slow handlers should set a shorter
390391
`query_timeout_seconds`.
391392

‎faststream_outbox/subscriber/usecase.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@
7070
_FLUSH_ON_EXIT_TIMEOUT_SECONDS = 2.0
7171

7272

73+
class _StaleWriterError(Exception):
74+
"""The fetch loop reconnected since this worker opened its writer connection."""
75+
76+
7377
@dataclasses.dataclass(slots=True)
7478
class _PendingFlush:
7579
"""A batchable terminal delete awaiting its batch flush, with the metric to emit once it lands."""
@@ -216,6 +220,8 @@ def __init__(
216220
# from self.running: running must stay True during drain so FastStream's
217221
# SubscriberUsecase.consume() doesn't early-exit on rows already in _inflight.
218222
self._stopping: bool = False
223+
# Bumped on every fetch-loop error; a worker whose writer predates the bump reconnects.
224+
self._fetch_generation: int = 0
219225

220226
@property
221227
def _client(self) -> "AbstractOutboxClient":
@@ -366,6 +372,7 @@ async def _fetch_loop(self) -> None:
366372
open_resources=self._open_fetch_resources,
367373
inner=self._fetch_inner,
368374
halt_on_drain=True,
375+
recycle_writers_on_error=True,
369376
)
370377

371378
@asynccontextmanager
@@ -586,6 +593,7 @@ async def _run_with_reconnect(
586593
open_resources: Callable[["AsyncEngine | None"], AbstractAsyncContextManager[Mapping[str, object]]],
587594
inner: Callable[..., Awaitable[None]],
588595
halt_on_drain: bool = False,
596+
recycle_writers_on_error: bool = False,
589597
) -> None:
590598
"""Reconnect-with-backoff scaffold shared by ``_fetch_loop`` and ``_worker_loop``.
591599
@@ -614,7 +622,14 @@ async def _run_with_reconnect(
614622
async with open_resources(client.engine) as kwargs:
615623
started = time.monotonic()
616624
await inner(**kwargs)
625+
except _StaleWriterError:
626+
self._log(
627+
log_level=logging.INFO,
628+
message=f"Outbox {name} loop reconnecting: the fetch loop lost its connection",
629+
)
617630
except Exception as e: # noqa: BLE001
631+
if recycle_writers_on_error:
632+
self._fetch_generation += 1
618633
# Reset the backoff counter when the connection was healthy for a
619634
# sustained window before failing — otherwise the lifetime error count
620635
# accrues and every transient blip after the first handful costs the
@@ -638,6 +653,7 @@ async def _worker_inner(self, *, writer_conn: "AsyncConnection | None") -> None:
638653
actual delete, not merely for dispatch.
639654
"""
640655
buffer: list[_PendingFlush] = []
656+
generation = self._fetch_generation
641657
batch_size = self._config.terminal_flush_batch_size
642658
max_buffer_age = self._config.lease_ttl_seconds / 2
643659
try:
@@ -650,6 +666,12 @@ async def _worker_inner(self, *, writer_conn: "AsyncConnection | None") -> None:
650666
row = await self._inflight.get()
651667
else:
652668
row = await self._inflight.get()
669+
if writer_conn is not None and self._fetch_generation != generation:
670+
self._inflight.put_nowait(row)
671+
self._inflight.task_done()
672+
await self._flush_buffer(buffer, writer_conn=writer_conn)
673+
await writer_conn.invalidate()
674+
raise _StaleWriterError
653675
if self._inflight.maxsize - self._inflight.qsize() >= self._refill_at:
654676
self._notify_event.set()
655677
buffered = False

‎tests/test_integration.py‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,73 @@ async def handle(body: dict) -> None:
890890
assert received == [{"order_id": 1}]
891891

892892

893+
async def test_worker_recycles_writer_connection_after_fetch_loop_reconnects(pg_engine, outbox_table) -> None:
894+
"""After the fetch loop loses its connection, the worker must not write on its own stale one.
895+
896+
A silently dropped primary leaves the idle writer connection dead too; reusing it costs a
897+
second ``query_timeout_seconds`` before the first post-failover row can be acknowledged.
898+
"""
899+
received: list[dict] = []
900+
delete_pids: list[int] = []
901+
hang_next_fetch: list[bool] = []
902+
broker = OutboxBroker(pg_engine, outbox_table=outbox_table, graceful_timeout=2.0)
903+
original_fetch = OutboxClient.fetch
904+
original_delete = OutboxClient.delete_with_lease
905+
906+
async def hang_on_demand(
907+
self: OutboxClient,
908+
conn: AsyncConnection | None,
909+
queues: Sequence[str],
910+
*,
911+
limit: int,
912+
lease_ttl_seconds: float,
913+
) -> list[OutboxInnerMessage]:
914+
if hang_next_fetch:
915+
hang_next_fetch.clear()
916+
await asyncio.Event().wait()
917+
return await original_fetch(self, conn, queues, limit=limit, lease_ttl_seconds=lease_ttl_seconds)
918+
919+
async def record_pid(
920+
self: OutboxClient,
921+
conn: AsyncConnection | None,
922+
row_id: int,
923+
token: uuid.UUID,
924+
*,
925+
dlq_payload: Mapping[str, Any] | None = None,
926+
) -> bool:
927+
assert conn is not None
928+
delete_pids.append((await conn.exec_driver_sql("SELECT pg_backend_pid()")).scalar_one())
929+
return await original_delete(self, conn, row_id, token, dlq_payload=dlq_payload)
930+
931+
@broker.subscriber(
932+
"orders",
933+
min_fetch_interval=0.05,
934+
max_fetch_interval=0.2,
935+
lease_ttl_seconds=30.0,
936+
query_timeout_seconds=0.5,
937+
)
938+
async def handle(body: dict) -> None:
939+
received.append(body)
940+
941+
session_factory = async_sessionmaker(pg_engine, expire_on_commit=False)
942+
with (
943+
mock.patch.object(OutboxClient, "fetch", hang_on_demand),
944+
mock.patch.object(OutboxClient, "delete_with_lease", record_pid),
945+
):
946+
async with broker:
947+
async with session_factory() as session, session.begin():
948+
await broker.publish({"order_id": 1}, queue="orders", session=session)
949+
await _wait_until(lambda: len(delete_pids) == 1, timeout=5.0)
950+
hang_next_fetch.append(True)
951+
await _wait_until(lambda: not hang_next_fetch, timeout=5.0)
952+
async with session_factory() as session, session.begin():
953+
await broker.publish({"order_id": 2}, queue="orders", session=session)
954+
await _wait_until(lambda: len(delete_pids) == 2, timeout=10.0)
955+
956+
assert received == [{"order_id": 1}, {"order_id": 2}]
957+
assert delete_pids[0] != delete_pids[1]
958+
959+
893960
async def test_hung_terminal_write_times_out_and_worker_reconnects(pg_engine, outbox_table) -> None:
894961
"""A terminal DELETE stuck on a silently dropped socket must not wedge the worker forever."""
895962
received: list[dict] = []

0 commit comments

Comments
 (0)