From 2a47d402b22889b45a799f385ceeb75fd4abcc2b Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Thu, 13 Feb 2025 17:24:03 -0800 Subject: [PATCH] go: sqle: auto_gc: Responding to PR feedback. Comments and cleanup. --- go/cmd/dolt/commands/engine/sqlengine.go | 7 +++++++ go/libraries/doltcore/doltdb/doltdb.go | 12 ++++-------- go/libraries/doltcore/sqle/auto_gc.go | 14 +++++++++----- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/go/cmd/dolt/commands/engine/sqlengine.go b/go/cmd/dolt/commands/engine/sqlengine.go index e646da048a..adc14af9a2 100644 --- a/go/cmd/dolt/commands/engine/sqlengine.go +++ b/go/cmd/dolt/commands/engine/sqlengine.go @@ -115,6 +115,13 @@ func NewSqlEngine( return nil, err } + // Make a copy of the databases. |all| is going to be provided + // as the set of all initial databases to dsqle + // DatabaseProvider. |dbs| is only the databases that came + // from MultiRepoEnv, and they are all real databases based on + // DoltDB instances. |all| is going to include some extension, + // informational databases like |dolt_cluster| sometimes, + // depending on config. all := make([]dsess.SqlDatabase, len(dbs)) copy(all, dbs) diff --git a/go/libraries/doltcore/doltdb/doltdb.go b/go/libraries/doltcore/doltdb/doltdb.go index 9982f3abc6..55d160e410 100644 --- a/go/libraries/doltcore/doltdb/doltdb.go +++ b/go/libraries/doltcore/doltdb/doltdb.go @@ -1919,16 +1919,12 @@ func (ddb *DoltDB) IsTableFileStore() bool { func (ddb *DoltDB) ChunkJournal() *nbs.ChunkJournal { cs := datas.ChunkStoreFromDatabase(ddb.db) - var store *nbs.NomsBlockStore - generationalNBS, ok := cs.(*nbs.GenerationalNBS) - if ok { - store = generationalNBS.NewGen().(*nbs.NomsBlockStore) - } else { - store = cs.(*nbs.NomsBlockStore) + if generationalNBS, ok := cs.(*nbs.GenerationalNBS); ok { + cs = generationalNBS.NewGen() } - if store != nil { - return store.ChunkJournal() + if nbsStore, ok := cs.(*nbs.NomsBlockStore); ok { + return nbsStore.ChunkJournal() } else { return nil } diff --git a/go/libraries/doltcore/sqle/auto_gc.go b/go/libraries/doltcore/sqle/auto_gc.go index 5d35e94c2e..0ea5038b13 100644 --- a/go/libraries/doltcore/sqle/auto_gc.go +++ b/go/libraries/doltcore/sqle/auto_gc.go @@ -154,13 +154,17 @@ func (c *AutoGCController) newCommitHook(name string) doltdb.CommitHook { type autoGCCommitHook struct { c *AutoGCController name string - // Always non-nil, if this channel delivers this channel - // delivers when no GC is currently running. + // When |done| is closed, there is no GC currently running or + // pending for this database. If it is open, then there is a + // pending request for GC or a GC is currently running. Once + // |done| is closed, we can check for auto GC conditions on + // the database to see if we should request a new GC. done chan struct{} // It simplifies the logic and efficiency of the - // implementation a bit to have a - // we can send. It becomes our new |done| channel on a - // successful send. + // implementation a bit to have an already allocated // in the commit hook. +channel + // we can try to send, which will become our new |done| + // channel once we send it successfully. next chan struct{} // |done| and |next| are mutable and |Execute| can be called // concurrently. We protect them with |mu|.