Skip to content

fix(gateway): offload _handoff_watcher SQLite calls off the event loop (#40695) - #40782

Closed
r266-tech wants to merge 2 commits into
NousResearch:mainfrom
r266-tech:fix-handoff-watcher-async-db
Closed

r266-tech wants to merge 2 commits into
NousResearch:mainfrom
r266-tech:fix-handoff-watcher-async-db

Conversation

@r266-tech

Copy link
Copy Markdown
Contributor

Fixes #40695.

_handoff_watcher polls state.db for pending CLI→gateway handoffs and made four synchronous SessionDB calls directly on the platform event loop:

  • list_pending_handoffs() (run.py ~L4868)
  • claim_handoff() (~L4873)
  • complete_handoff() (~L4878)
  • fail_handoff() (~L4884)

Under SQLite WAL-lock contention these blocked the Discord gateway heartbeat for 10–40s, causing WebSocket disconnects and 404 Unknown interaction errors. _handoff_watcher is the lone outlier — the same file already offloads its blocking SQLite work via asyncio.to_thread in ~20 places (e.g. L5409/L5420/L5430, with the explicit comment "asyncio.to_thread so the SQLite WAL lock never blocks the…").

Fix: wrap each of the four calls in await asyncio.to_thread(...), matching the file's dominant convention. _process_handoff is already async/awaited, and the visible claim → process → complete/fail ordering is unchanged.

Test: tests/gateway/test_handoff_watcher_async_db.py runs one watcher tick and asserts each blocking SessionDB call executes on a worker thread, not the loop thread — covering both the success path (list/claim/complete) and the failure path (fail_handoff). The tests fail if any call regresses to running synchronously on the loop.

Scope note: this is intentionally minimal and does not change the existing cancellation semantics — cancelling the watcher between claiming a handoff and completing it (already possible at the await self._process_handoff(row) point) can leave a row in the claimed state. Making that sequence individually cancellation-safe is a separate concern, out of scope here.

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery platform/discord Discord bot adapter labels Jun 6, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Positive verification

Clean fix that correctly offloads all 4 synchronous SessionDB calls off the event loop. Verified:

  1. All 4 blocking calls wrapped: list_pending_handoffs, claim_handoff, complete_handoff, fail_handoff — each wrapped in await asyncio.to_thread(...). No calls missed.

  2. Follows file convention: asyncio.to_thread is the dominant async-offload pattern already used elsewhere in gateway/run.py.

  3. Exception safety preserved: The except Exception handler around fail_handoff still works correctly — if to_thread raises (e.g., the thread function throws), the exception propagates through await and is caught by the existing handler.

  4. CancelledError re-raise preserved: except asyncio.CancelledError: raise is untouched, so task cancellation semantics are maintained.

  5. Test coverage: Both happy path (complete_handoff) and failure path (fail_handoff) verify the calls run on a worker thread, not the event loop thread. The _FakeSessionDB correctly captures threading.current_thread() for each call.

Addresses real issue #40695 — SQLite WAL-lock contention stalling the Discord gateway heartbeat.

@r266-tech

Copy link
Copy Markdown
Contributor Author

Confirming the scope boundary on top of your trace, @liuhao1024: only the four SessionDB calls inside _handoff_watcher's loop are offloaded, since that coroutine shares the Discord gateway's event loop where the WAL-lock contention stalled the heartbeat (#40695). Other SessionDB usage outside the gateway loop is intentionally left synchronous to keep the diff scoped to the heartbeat path. The two-path test (complete_handoff happy + fail_handoff failure) asserts execution on a worker thread precisely to lock that contract in.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jun 21, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Automated hermes-sweeper review: this PR's fix is already implemented on main.

Evidence:

  • gateway/run.py:6555, :6560, :6565, and :6571 on origin/main now wrap the four _handoff_watcher SessionDB calls in await asyncio.to_thread(...): list_pending_handoffs, claim_handoff, complete_handoff, and fail_handoff.
  • tests/gateway/test_handoff_watcher_async_db.py:102, :126, and :141 add regression coverage for the success path, failure path, and explicit asyncio.to_thread wrapping.
  • The implementation landed in f0c5d812b0dc5b1df8d1dc24f395c69ef1cb4338 (fix(gateway): offload handoff watcher SessionDB polling off the event loop).

Thanks to @r266-tech for the focused fix and to @liuhao1024 for the detailed verification in the PR discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround platform/discord Discord bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Discord gateway heartbeat can be blocked by synchronous handoff SQLite polling

4 participants