Repository navigation
feat(dbms): close idle sessions that keep a FORCE-dropped database pinned - #4843
Merged
Merged
Conversation
andrejtonev
added
CI -build=coverage -test=core
Contributor
Author
Tracking
Standard development
CI Testing Labels
Documentation checklist
|
andrejtonev
changed the title
andrejtonev
force-pushed
the
feat/detached-tenant-registry
branch
2 times, most recently
from
September 23, 2026 11:38
0e1c798 to
1e7412b
Compare
andrejtonev
force-pushed
the
feat/detached-tenant-registry
branch
from
September 24, 2026 13:41
688d257 to
e3d1814
Compare
andrejtonev
force-pushed
the
evict/4583-final
branch
from
September 24, 2026 14:43
55ebd4e to
2384fd3
Compare
andrejtonev
force-pushed
the
evict/4583-final
branch
from
September 25, 2026 07:12
2384fd3 to
5680e1a
Compare
colinbarry
requested changes
Sep 25, 2026
colinbarry
left a comment
colinbarry
left a comment
Contributor
There was a problem hiding this comment.
Mostly great, but a few tiny concerns...
andrejtonev
force-pushed
the
evict/4583-final
branch
2 times, most recently
from
September 28, 2026 09:19
bd02da7 to
3f5006e
Compare
andrejtonev
changed the title
andrejtonev
force-pushed
the
evict/4583-final
branch
from
September 28, 2026 09:46
3f5006e to
3b238d4
Compare
colinbarry
requested changes
Sep 29, 2026
colinbarry
left a comment
colinbarry
left a comment
Contributor
There was a problem hiding this comment.
All good, other than an unlikely use-after-free that could be easily fixed.
andrejtonev
added a commit
that referenced
this pull request
Sep 30, 2026
…self (#4969) Stacked on #4843 (base `evict/4583-final`). #4843 closes sessions that still hold a FORCE-dropped database when the deferred-drop worker ticks (every 10 s). A session that sends a query before that tick releases the database itself (`ResetInterpreter` → `ReleaseDbIfMarked`), so the hook never sees it. A client that names the database in its Bolt metadata (`session(database="x")`) then stays on "Database required", even after `x` is recreated: identical metadata makes `RuntimeConfig::Configure` skip re-resolving it, until the client reconnects. `ReleaseDbIfMarked` now reports whether it released a database, and `ResetInterpreter` then requests termination of its own session (the same `SessionRegistry` → `RequestTermination` path as the drain hook). `RequestTermination` only posts to the session's strand, so the current message still gets its reply; the connection closes after it and the driver reconnects. Enterprise builds only, like the hook. Client-visible rule after this PR: a connection whose current database is FORCE-dropped is closed — an idle one on the next drain tick, an active one after its next message. This replaces the `Configure` re-resolve guard that was trimmed out of #4932.
Contributor
|
This PR has potential conflicts with the following other open pull requests which modify the same files:
|
…nned DROP DATABASE ... FORCE leaves the database draining until nothing holds it. A connection whose current database it was keeps it pinned until the client sends another query, which an idle pooled connection may never do, so the database never drains. The deferred-drop worker now runs a drain hook once per tick while a drop is pending. The InterpreterContext registers one for its lifetime: it asks every session still holding a database that is being dropped to terminate, through the same SessionRegistry path TERMINATE SESSIONS uses. The session closes itself between Bolt messages, never mid-query, and releasing it lets the next try_delete succeed. Running every tick also covers drops applied on a replica and a session that attaches just as the database is dropped.
…self (#4969) Stacked on #4843 (base `evict/4583-final`). #4843 closes sessions that still hold a FORCE-dropped database when the deferred-drop worker ticks (every 10 s). A session that sends a query before that tick releases the database itself (`ResetInterpreter` → `ReleaseDbIfMarked`), so the hook never sees it. A client that names the database in its Bolt metadata (`session(database="x")`) then stays on "Database required", even after `x` is recreated: identical metadata makes `RuntimeConfig::Configure` skip re-resolving it, until the client reconnects. `ReleaseDbIfMarked` now reports whether it released a database, and `ResetInterpreter` then requests termination of its own session (the same `SessionRegistry` → `RequestTermination` path as the drain hook). `RequestTermination` only posts to the session's strand, so the current message still gets its reply; the connection closes after it and the driver reconnects. Enterprise builds only, like the hook. Client-visible rule after this PR: a connection whose current database is FORCE-dropped is closed — an idle one on the next drain tick, an active one after its next message. This replaces the `Configure` re-resolve guard that was trimmed out of #4932.
…estroyed ~Server destroys the Bolt io_context while the defer worker keeps ticking until InterpreterContextLifetimeControl is destroyed, so a tick could leave the worker holding the last reference to a session whose reactor is gone. Clear the hook right after the server has been awaited.
Extract the drain-hook call out of Handler::Tick_() into RunDrainHook_() to bring its cognitive complexity back under the limit (cpp:S3776), and capture only to_close in CloseSessionsOnDroppingDatabases (cpp:S3608).
andrejtonev
force-pushed
the
evict/4583-final
branch
from
September 30, 2026 09:14
2eac26f to
1c5ccea
Compare
|
…tion A replica applying a FORCE drop only seals the database; it never aborted a transaction or closed a connection. Closing sessions on drop changed that: the drain hook also closed sessions idle inside an explicit transaction, aborting it, and every session was closed after its first query on the dropped database, even one that could USE another database. - The drain hook skips sessions with a live transaction (not IDLE and not already TERMINATED by MAIN's FORCE drop); they release the database once the transaction ends, or are closed on a later tick once idle. - A session that releases the dropped database is closed only when the database was named in the Bolt connection metadata; a USE DATABASE session keeps its connection, as before. - Commit() with nothing to commit (e.g. USE DATABASE) now returns the transaction status to IDLE. It stayed ACTIVE, so an idle session looked mid-transaction to foreign readers and the drain hook never closed it. The replica force-drop e2e now waits past a drain tick and asserts that the open transaction survives and the idle session is closed.
andrejtonev
added
CI -build=community -test=core
This was referenced Oct 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
DROP DATABASE … FORCE(#4573) leaves the dropped database draining until nothing holds it. A Bolt connection whose current database it was keeps it pinned until the client sends another query — which an idle pooled connection (e.g. a Neo4j driver pool) may never do — so the database never drains.This closes such connections. Nothing in the interpreter or the Bolt session changes: it reuses the #4577 termination path (
SessionRegistry→RequestTermination).Mechanism
Handler<T>drain hook (SetDrainHook/ClearDrainHook): the deferred-drop worker runs it once per tick while any drop is pending, before the husks'try_delete. It is held under its owndrain_hook_mutex_while it runs, soClearDrainHookreturns only once the hook is not running and never will again. That mutex is taken only by the single defer worker and by set/clear at startup/shutdown — never by a query or aDROP.InterpreterContextowns the hook for its lifetime:InterpreterContextLifetimeControlregisters it after construction and unregisters it before destruction, so capturing the context is safe by construction (no dependency on declaration order inmain()).CloseSessionsOnDroppingDatabases, next toTerminateSessions): underinterpreters.WithLock, collects the session uuid of every interpreter whoseforeign_db_view().marked_for_deletionis set; after the lock,RequestTermination()on each. The session closes itself on its own strand between Bolt messages — a busy one after its current message, never mid-query — and its teardown releases the pin, so the nexttry_deletesucceeds.Delete(uuid)never goes through the DROP handler) and a session that attaches just as the database is dropped.seal()(the FORCE path) marks a database, so plainDROP, same-name recreates and sessions on other databases are untouched. Enterprise builds only (#ifdef MG_ENTERPRISE).Behaviour notes