Problem
RedisKVStorage.__post_init__ and RedisDocStatusStorage.__post_init__ call RedisConnectionManager.get_pool(url) at construction. That manager is class-level state: one shared ConnectionPool per URL plus a reference count (_pool_refs), incremented by get_pool and decremented only by release_pool_ref / release_pool, which run from the storage's async close() / finalize().
LightRAG.__post_init__ is synchronous, constructs twelve storage objects in sequence, and then runs a series of validations (llm_model_func present, role_llm_configs well-formed, ...). It has no teardown path: if any constructor or validation raises, every Redis storage already constructed keeps its pool reference forever. Repeated failed constructions grow the count without bound, and a later valid instance can never bring it to zero, so the shared pool is never closed.
Why only Redis
Every other backend defers connecting to initialize() and leaves the constructor side-effect free:
| backend |
constructor |
connection |
| MongoDB (all four storages) |
validate workspace, build collection name |
initialize() → await ClientManager.get_client() |
| PostgreSQL (all four) |
validate workspace |
initialize() → await ClientManager.get_client(), pool via asyncpg.create_pool |
| OpenSearch (three) |
build index name |
initialize() → await ClientManager.get_client() |
| Milvus / Qdrant |
validate embedding, compute suffix, self._client = None |
initialize() creates the client |
| Neo4j / Memgraph |
handle workspace override, self._driver = None |
initialize() → AsyncGraphDatabase.driver() |
| JSON KV / DocStatus, NetworkX, NanoVectorDB, Faiss |
record paths |
initialize() binds shared dict / loads file |
| Redis KV / DocStatus |
RedisConnectionManager.get_pool() |
initialize() only pings |
How it surfaced
Found by review on #4011 (configuration storage, slice 1). That PR adds one more KV storage object to __post_init__ and a new refusal path (validate_workspace_override, raised in an ordinary storage's constructor), which made the leak reachable through the configuration storage's reference. The PR sidesteps it by constructing the configuration storage as the last statement of __post_init__ that can raise, and records the leak between the business storages themselves as a pre-existing residue in docs/design/ConfigurationStorage.md (Cleanup before INITIALIZED exists, last paragraph). This issue tracks the root fix.
Proposed fix
Align Redis with the other ten backends: move RedisConnectionManager.get_pool() and the Redis(connection_pool=...) client creation from __post_init__ into initialize() (under get_data_init_lock(), guarded by _initialized as the ping already is). The constructor then records _redis_url only and sets _pool = _redis = None.
Things to check while doing it:
- The constructor's existing error path (
release_pool_ref + schedule_pool_close on a failed get_pool) moves with the acquisition.
close() must stay idempotent and tolerate a never-initialized instance (_pool is None).- Tests that construct a Redis storage and read
_pool / _redis before initialize(), and tests that mock RedisConnectionManager.get_pool around construction (e.g. tests/workspace/test_reserved_workspace.py, tests/kg/redis_impl/), need to move their expectations to after initialize().
_get_redis_connection() should raise the existing "Redis connection not initialized" error when called before initialize(); the strict-read path in config_store already relies on that message.
Acceptance
- Constructing any Redis storage, alone or via
LightRAG(...), leaves RedisConnectionManager._pools / _pool_refs untouched.
- A
LightRAG construction that fails after Redis storages were built leaves the reference count unchanged (the tests/workspace/test_reserved_workspace.py pool-reference test can then drop its "constructed last" premise and assert directly).
- After the fix lands, the construction-order paragraph in
ConfigurationStorage.md can be reduced to a note that the ordering is no longer load-bearing.
Related: #4011, #4006.
Problem
RedisKVStorage.__post_init__andRedisDocStatusStorage.__post_init__callRedisConnectionManager.get_pool(url)at construction. That manager is class-level state: one sharedConnectionPoolper URL plus a reference count (_pool_refs), incremented byget_pooland decremented only byrelease_pool_ref/release_pool, which run from the storage's asyncclose()/finalize().LightRAG.__post_init__is synchronous, constructs twelve storage objects in sequence, and then runs a series of validations (llm_model_funcpresent,role_llm_configswell-formed, ...). It has no teardown path: if any constructor or validation raises, every Redis storage already constructed keeps its pool reference forever. Repeated failed constructions grow the count without bound, and a later valid instance can never bring it to zero, so the shared pool is never closed.Why only Redis
Every other backend defers connecting to
initialize()and leaves the constructor side-effect free:initialize()→await ClientManager.get_client()initialize()→await ClientManager.get_client(), pool viaasyncpg.create_poolinitialize()→await ClientManager.get_client()self._client = Noneinitialize()creates the clientself._driver = Noneinitialize()→AsyncGraphDatabase.driver()initialize()binds shared dict / loads fileRedisConnectionManager.get_pool()initialize()only pingsHow it surfaced
Found by review on #4011 (configuration storage, slice 1). That PR adds one more KV storage object to
__post_init__and a new refusal path (validate_workspace_override, raised in an ordinary storage's constructor), which made the leak reachable through the configuration storage's reference. The PR sidesteps it by constructing the configuration storage as the last statement of__post_init__that can raise, and records the leak between the business storages themselves as a pre-existing residue indocs/design/ConfigurationStorage.md(Cleanup beforeINITIALIZEDexists, last paragraph). This issue tracks the root fix.Proposed fix
Align Redis with the other ten backends: move
RedisConnectionManager.get_pool()and theRedis(connection_pool=...)client creation from__post_init__intoinitialize()(underget_data_init_lock(), guarded by_initializedas the ping already is). The constructor then records_redis_urlonly and sets_pool = _redis = None.Things to check while doing it:
release_pool_ref+schedule_pool_closeon a failedget_pool) moves with the acquisition.close()must stay idempotent and tolerate a never-initialized instance (_pool is None)._pool/_redisbeforeinitialize(), and tests that mockRedisConnectionManager.get_poolaround construction (e.g.tests/workspace/test_reserved_workspace.py,tests/kg/redis_impl/), need to move their expectations to afterinitialize()._get_redis_connection()should raise the existing "Redis connection not initialized" error when called beforeinitialize(); the strict-read path inconfig_storealready relies on that message.Acceptance
LightRAG(...), leavesRedisConnectionManager._pools/_pool_refsuntouched.LightRAGconstruction that fails after Redis storages were built leaves the reference count unchanged (thetests/workspace/test_reserved_workspace.pypool-reference test can then drop its "constructed last" premise and assert directly).ConfigurationStorage.mdcan be reduced to a note that the ordering is no longer load-bearing.Related: #4011, #4006.