Skip to content

Commit 036feb5

Browse files
committed
fix(server): invalidate executor cache after dropping databases
Executors were cached with stale database manager references, causing dropped databases to still appear in SHOW DATABASES queries. Added invalidateAllExecutors() and call it after DropDatabase() and DropCompositeDatabase() operations to ensure fresh executor references see the updated database list. Fixes TestMultiDatabase_E2E_FullSequence test failures in Step12 and Step13.
1 parent 1ad1dc4 commit 036feb5

File tree

3 files changed

+38
-4
lines changed

3 files changed

+38
-4
lines changed

pkg/multidb/composite.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,14 @@ func (m *DatabaseManager) DropCompositeDatabase(name string) error {
135135
delete(m.databases, name)
136136
delete(m.engines, name) // Clear cached engine
137137

138-
return m.persistMetadata()
138+
if err := m.persistMetadata(); err != nil {
139+
// If persistence fails, restore the database to maintain consistency
140+
// This prevents the database from being dropped in memory but still existing in storage
141+
m.databases[name] = info
142+
return fmt.Errorf("failed to persist metadata after drop: %w", err)
143+
}
144+
145+
return nil
139146
}
140147

141148
// AddConstituent adds a constituent to an existing composite database.

pkg/multidb/manager.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,14 @@ func (m *DatabaseManager) DropDatabase(name string) error {
379379
delete(m.databases, name)
380380
delete(m.engines, name) // Clear cached engine
381381

382-
return m.persistMetadata()
382+
if err := m.persistMetadata(); err != nil {
383+
// If persistence fails, restore the database to maintain consistency
384+
// This prevents the database from being dropped in memory but still existing in storage
385+
m.databases[name] = info
386+
return fmt.Errorf("failed to persist metadata after drop: %w", err)
387+
}
388+
389+
return nil
383390
}
384391

385392
// GetStorage returns a namespaced storage engine for the specified database.

pkg/server/server_db.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,16 @@ func (s *Server) invalidateExecutor(dbName string) {
142142
delete(s.executors, dbName)
143143
}
144144

145+
// invalidateAllExecutors clears all cached executors to force fresh database manager references.
146+
// This is used when database metadata changes (e.g., dropping databases) to ensure
147+
// all executors see the updated state.
148+
func (s *Server) invalidateAllExecutors() {
149+
s.executorsMu.Lock()
150+
defer s.executorsMu.Unlock()
151+
// Clear all executors - they will be recreated with fresh database manager references
152+
s.executors = make(map[string]*cypher.StorageExecutor)
153+
}
154+
145155
// databaseManagerAdapter wraps multidb.DatabaseManager to implement
146156
// cypher.DatabaseManagerInterface, avoiding import cycles.
147157
type databaseManagerAdapter struct {
@@ -165,6 +175,9 @@ func (a *databaseManagerAdapter) DropDatabase(name string) error {
165175
// Invalidate cached executor for dropped database
166176
if a.server != nil {
167177
a.server.invalidateExecutor(name)
178+
// Also invalidate all executors to ensure fresh database manager references
179+
// This ensures queries from other databases see the updated database list
180+
a.server.invalidateAllExecutors()
168181
}
169182
return nil
170183
}
@@ -238,8 +251,15 @@ func (a *databaseManagerAdapter) DropCompositeDatabase(name string) error {
238251
if err := a.manager.DropCompositeDatabase(name); err != nil {
239252
return err
240253
}
241-
// Note: Composite databases don't have their own executors cached
242-
// since queries go through constituent databases. No cleanup needed.
254+
// Invalidate any cached executors that might reference this composite database
255+
// Note: Composite databases don't have their own executors cached, but we should
256+
// invalidate the executor for the database we're querying from (usually "nornic")
257+
// to ensure subsequent queries see the updated state
258+
if a.server != nil {
259+
// Invalidate executor cache to force fresh database manager reference
260+
// This ensures all executors see the updated database list
261+
a.server.invalidateAllExecutors()
262+
}
243263
return nil
244264
}
245265

0 commit comments

Comments
 (0)