Skip to content

fix(gateway): enforce async SessionStore boundary#61905

Merged
kshitijk4poor merged 7 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/61842-61847-async-session-store
Jul 10, 2026
Merged

fix(gateway): enforce async SessionStore boundary#61905
kshitijk4poor merged 7 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/61842-61847-async-session-store

Conversation

@kshitijk4poor

@kshitijk4poor kshitijk4poor commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Gateway SessionStore work now crosses one enforced async boundary, so synchronous SQLite, routing-index, transcript, and filesystem operations cannot freeze the shared event loop.

This salvages #61842 and #61847 by @kenyonxu with all three contributor commits and authorship preserved. Follow-ups fix the missing PRIORITY-path await, replace scattered offloads with AsyncSessionStore, close residual normal-message/lifecycle sites, remove accidental machine-local config, and harden concurrent routing publication/persistence.

Changes

  • gateway/session.py: add AsyncSessionStore; move get_or_create_session I/O outside its state lock; serialize latest-state persistence; converge concurrent same-key creation; retain cross-profile recovery isolation.
  • gateway/run.py, gateway/slash_commands.py: route loop-side SessionStore operations through the facade, including /usage transcript work.
  • Tests: pin off-thread execution, complete async-boundary coverage, PRIORITY await behavior, lock/I/O separation, concurrent same-key publication, stale-writer prevention, and profile recovery isolation.

Validation

Check Result
Targeted gateway tests 276 passed
Real SessionStore + SQLite E2E off-loop; 19 loop ticks during injected 100 ms block
Concurrency controls same-key convergence + latest-index persistence pass
Mutation check raw hot-path call makes boundary guard fail
Ruff / diff check / compile clean

Closes #53297.

Credits: #61842 and #61847 by @kenyonxu; contributor commits retained with authorship preserved.

kenyonxu and others added 5 commits July 10, 2026 11:55
…cio.to_thread

Every inbound message calls get_or_create_session which synchronously
executes _is_session_ended_in_db → db.get_session → conn.execute on
the asyncio event loop. On a ~1.4GB state.db, this blocks the loop
for seconds to minutes, starving Discord heartbeats.

Upstream NousResearch#55159 fixed the same pattern for self._session_db in
gateway/run.py but missed SessionStore._db in gateway/session.py.

This follows the exact same approach as NousResearch#55159:
- session.py internals stay fully synchronous (zero changes)
- Threading.Lock contract is preserved
- All hot-path callers in run.py and slash_commands.py wrap calls
  with await asyncio.to_thread(self.session_store.method, ...)

Affected: get_or_create_session, switch_session, update_session,
load_transcript, rewrite_transcript, rewind_session, reset_session,
set_model_override, _save — ~24 call sites across 2 files.

# Conflicts:
#	gateway/slash_commands.py
…check (#5)

The sync _session_has_compression_in_flight sat on the message hot path
and blocked the event loop twice: under session_store._lock during
_ensure_loaded_locked (JSON read) and via db.get_compression_lock_holder
(SQLite SELECT). Async-ify the method and offload both sources via
asyncio.to_thread; await the call site in _handle_active_session_busy_message.
…e_session

The second lock block in get_or_create_session held self._lock during six
blocking operations on every inbound message: _is_session_ended_in_db
(SQLite SELECT), _should_reset (callback), _save (SQLite write + JSON write
+ os.fsync), and _recover_session_from_db (SQLite SELECT + UPDATE).

A code comment at line 1607 claimed 'SQLite calls are made outside the
lock' -- true only for _compression_tip_for_session_id, which was moved
out in a prior fix. The remaining I/O was never addressed.

Restructure into a four-phase lock/no-lock split that mirrors the pattern
already established at the bottom of the function:

  Phase 1  (lock)    -- read entry + session_id
  Phase 1b (no lock) -- stale check + reset policy
  Phase 2  (lock)    -- apply decisions to _entries, capture snapshot + flags
  Phase 3  (no lock) -- recovery DB query, _save from snapshot, end/create

_save_entries(snapshot) replaces _save() to avoid dict-mutation races when
called outside the lock. _query_recoverable_session splits the DB I/O out
of _recover_session_from_db so only the _entries assignment needs the lock.

Three early returns inside the lock block are eliminated in favour of a
unified save + return path.
@kshitijk4poor kshitijk4poor force-pushed the salvage/61842-61847-async-session-store branch from b84e53a to 9caec7b Compare July 10, 2026 06:26
@alt-glitch alt-glitch added type/perf Performance improvement or optimization P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery 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 labels Jul 10, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Salvage of #61842 and #61847 (by @kenyonxu) — contributor commits and authorship preserved, review gaps closed (PRIORITY-path await, facade routing, raw-loop-side-call guard). Treated as the active version of this event-loop-offload work; related to the SessionStore-starvation family (merged #60984/#53603/#52090). Not a duplicate — #61842/#61847 are the salvaged predecessors.

@kshitijk4poor kshitijk4poor force-pushed the salvage/61842-61847-async-session-store branch from 7bad39b to 22374a1 Compare July 10, 2026 06:41
@kshitijk4poor kshitijk4poor merged commit ed36edd into NousResearch:main Jul 10, 2026
31 checks passed
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 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/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Telegram existing sessions can take 15-30s to activate while new chats respond immediately

3 participants