Skip to content

fix: ResponseStore SQLite connection pooling to prevent FD leak (#36111, #37369) - #37389

Closed
datin-antasena wants to merge 1 commit into
NousResearch:mainfrom
datin-antasena:fix/response-store-fd-leak
Closed

fix: ResponseStore SQLite connection pooling to prevent FD leak (#36111, #37369)#37389
datin-antasena wants to merge 1 commit into
NousResearch:mainfrom
datin-antasena:fix/response-store-fd-leak

Conversation

@datin-antasena

Copy link
Copy Markdown

Summary

Fixes the file descriptor leak in ResponseStore that causes the gateway to hit OSError: [Errno 24] Too many open files after ~2 days of uptime.

Credit: This issue was first identified by @jscoltock in #36111 (May 31), with excellent root cause analysis. We independently hit the same problem in production (#37369) and built on their findings to implement the fix. Their proposed approach (singleton + close()) was the right direction.

Root Cause

ResponseStore.__init__() opens a new SQLite connection every time an APIServerAdapter is created (line 706), but:

  1. No close() method was called on adapter teardown
  2. No connection reuse between instances pointing to the same DB file
  3. Each gateway retry cycle accumulated 3 more FDs (db + WAL + SHM)

After ~45 hours with 8 Telegram topics, we observed 15 FDs to response_store.db (7-8 separate connections) when 3 is the healthy baseline.

Fix

Connection pooling with reference counting:

  • _connection_pool (class-level dict) keyed by resolved DB path
  • _ResponseStoreConnection wraps sqlite3.Connection with an RLock and ref_count
  • Multiple ResponseStore() instances for the same file share ONE underlying connection
  • close() decrements ref_count; connection is only closed when last reference is released
  • __del__ as safety net
  • APIServerAdapter.disconnect() now calls store.close()
  • APIServerAdapter.connect() recreates store if adapter is reused after disconnect

Thread safety: All store operations (get, put, delete, set_conversation, get_conversation) are serialized via shared.lock (RLock).

Tests

  • test_reuses_one_connection_per_db_path — verifies connection pooling
  • test_adapter_disconnect_closes_response_store — verifies adapter lifecycle cleanup
  • All 158 existing tests pass

Verification

# Before fix (20 min after restart):
ls -l /proc/$PID/fd | grep response_store | wc -l
# 15

# After fix:
ls -l /proc/$PID/fd | grep response_store | wc -l
# 3

Closes #37369
Closes #36111

@teknium1

Copy link
Copy Markdown
Contributor

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

Evidence:

  • gateway/platforms/api_server.py:527 defines ResponseStore.close(), closing the SQLite connection opened by the store.
  • gateway/platforms/api_server.py:4502 calls self._response_store.close() from APIServerAdapter.disconnect(), so adapter teardown releases the leaked response_store.db handle.
  • gateway/run.py:2404 adds _dispose_unused_adapter(), and the reconnect watcher calls it on failed adapter attempts, covering the path where adapters were previously dropped without disconnect.
  • tests/gateway/test_platform_reconnect_fd_leak.py:303 regression-tests that APIServerAdapter.disconnect() closes the ResponseStore connection.
  • The implementation landed in 4b06c98fe4f4686f89873933ac616cb6e76b298d (fix(gateway): close ResponseStore + dispose unowned adapter on reconnect failure).

Thanks for the report and production verification. The exact reference-counted pooling design from this PR was not adopted, but the FD leak it targets is fixed on main with teardown coverage.

@teknium1 teknium1 closed this Jun 21, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 21, 2026
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:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

3 participants