Skip to content

fix(state): bound reusable SQLite WAL readers - #81082

Closed
HenryGHJ wants to merge 2 commits into
NousResearch:mainfrom
HenryGHJ:fix/bounded-sqlite-read-pool
Closed

fix(state): bound reusable SQLite WAL readers#81082
HenryGHJ wants to merge 2 commits into
NousResearch:mainfrom
HenryGHJ:fix/bounded-sqlite-read-pool

Conversation

@HenryGHJ

@HenryGHJ HenryGHJ commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

A long-lived shared SessionDB currently retains one SQLite WAL reader for every thread that has ever touched the read path. Finished workers can therefore accumulate database and WAL descriptors until final shutdown.

This change replaces permanent per-thread readers with an exclusive reusable pool capped at eight read-only connections:

  • workers lease readers only for the duration of a read context;
  • readers are reused across threads with check_same_thread=False;
  • normal and exceptional paths return the lease automatically;
  • broken readers and partially initialized connections are discarded;
  • saturation never opens reader N+1 and instead uses the existing writer-lock fallback;
  • SessionDB.close() seals the pool, closes idle readers immediately, and closes active leases when returned.

This is an alternative implementation to #75352. That PR preserves per-thread caching and reaps connections after their owner thread exits. This PR instead applies a fixed process-level reader ceiling, making descriptor use independent of historical or current worker-thread count.

Related Issue

Fixes #75269

Related alternative: #75352

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Updated hermes_state.py with a bounded reusable WAL-reader pool and exclusive lease lifecycle.
  • Preserved the existing locked writer-connection fallback when WAL is unavailable, reader creation fails, or the pool is saturated.
  • Updated tests/test_session_db_read_path_split.py with regression coverage for finished-thread reuse, exclusive concurrent leases, hard-cap saturation, non-SQL exception return, initialization failure, broken-reader disposal, and close/return behavior.

How to Test

  1. Run the focused SessionDB, search, gateway, and delegation regression suite:

    uv run --with pytest --with pytest-timeout --with pytest-asyncio \
      python -m pytest \
      tests/test_session_db_read_path_split.py \
      tests/test_hermes_state.py \
      tests/tools/test_session_search.py \
      tests/gateway/test_async_session_db.py \
      tests/gateway/test_async_delegation_session_binding.py \
      tests/tools/test_delegate.py \
      -q -o 'addopts='
  2. Confirm the result: 312 passed, 4 skipped.

  3. Run the cross-platform static guard:

    python scripts/check-windows-footguns.py \
      hermes_state.py tests/test_session_db_read_path_split.py

    Result: No Windows footguns found.

  4. Run git diff --check origin/main...HEAD and confirm no whitespace errors.

The repository's preferred scripts/run_tests.sh wrapper was also attempted. The isolated worktree did not have a project dev virtualenv with pytest and full extras, so the wrapper could not provide a valid CI-parity run there; the focused suite above was run directly with explicit pytest dependencies, as permitted by the contributing guide when the wrapper is unavailable.

Manual validation used a long-running gateway workload after the patch. A short-lived-thread stress run plateaued at eight pooled readers and returned to zero after SessionDB.close(); the restarted gateway remained connected with a stable descriptor count. No deployment logs or identifying deployment details are included in this PR.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs and documented the alternative implementation in fix(state): safely reclaim finished-thread WAL readers #75352
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — focused affected suites passed; the full repository suite was not run
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1 arm64, Python 3.11.15, SQLite 3.50.4

Documentation & Housekeeping

  • Relevant documentation: N/A — internal connection-lifecycle fix with no user-facing API or configuration change
  • cli-config.yaml.example: N/A — no configuration keys added or changed
  • CONTRIBUTING.md / AGENTS.md: N/A — no contributor workflow change
  • Cross-platform impact considered — implementation uses Python synchronization and SQLite APIs only
  • Tool descriptions/schemas: N/A — no tool behavior changed

Screenshots / Logs

Not applicable. Validation results are listed above; no deployment logs, session content, credentials, profile data, or local paths are included.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-decision Awaiting maintainer decision before any implementation labels Aug 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #75352 and #78287: this is an alternative bounded-pool design for #75269, so a maintainer should choose the SessionDB reader-lifecycle contract.

@HenryGHJ

Copy link
Copy Markdown
Contributor Author

Closing as superseded. Upstream independently implemented the same SQLite reader-lifecycle fix in 87aedbe and strengthened peak connection bounding in 0472c31. After reconciling this branch with current main, the PR has no remaining unique diff. Thanks for the triage.

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

Labels

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists 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]: SessionDB retains WAL readers from finished worker threads until shutdown, exhausting RLIMIT_NOFILE

2 participants