aminghadersohi commented on code in PR #45861:
URL: https://github.com/apache/superset/pull/45861#discussion_r4448086954
##########
superset/models/core.py:
##########
@@ -1740,16 +1740,29 @@ def _evict_engine_cache(
"""Evict all cached engines for a database when it is updated or deleted.
URL/kwargs changes already produce a new cache key, so stale engines are
- never served to callers. This eviction step is purely to reclaim memory:
- without it, old engines for a renamed host or rotated password would linger
- in _ENGINE_CACHE until the process restarted.
+ never served to callers. Eviction reclaims memory (old engines for a
+ renamed host or rotated password would otherwise linger in _ENGINE_CACHE
+ until the process restarted) and disposes the evicted engines so that the
+ connections their pools hold idle are closed. Some databases, such as
+ DuckDB, allow only one configuration per database file per process, so an
+ idle connection kept by an engine with the previous settings would block
+ opening the edited ones.
+
+ Disposal is not deterministic for connections checked out at that moment:
+ they stay on the engine's previous pool and are released when it is. It
runs
+ after the cache lock is released, and a failure to dispose never interrupts
+ the update or delete.
"""
if target.id is None:
return
with _ENGINE_CACHE_LOCK:
stale = [k for k in _ENGINE_CACHE if k[0] == target.id]
- for k in stale:
- _ENGINE_CACHE.pop(k, None)
+ evicted = [_ENGINE_CACHE.pop(k) for k in stale]
+ for engine in evicted:
+ try:
+ engine.dispose()
Review Comment:
When `DB_CONNECTION_MUTATOR` selects `StaticPool` or `SingletonThreadPool`,
`dispose()` closes checked-out connections too. Even renaming a database then
breaks active queries with `Cannot operate on a closed database`; defer
disposal until those connections are released.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
