Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions go/cmd/dolt/commands/engine/lock_release_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Copyright 2026 Dolthub, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package engine

import (
"context"
"errors"
"os"
"path/filepath"
"runtime"
"testing"
"time"

"github.com/dolthub/fslock"
"github.com/stretchr/testify/require"

"github.com/dolthub/dolt/go/libraries/doltcore/dbfactory"
"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
"github.com/dolthub/dolt/go/libraries/doltcore/env"
"github.com/dolthub/dolt/go/libraries/utils/config"
"github.com/dolthub/dolt/go/libraries/utils/filesys"
)

// TestCreateDatabase_ReleasesLockOnEngineClose asserts that when embedded callers opt into
// disable_singleton_cache, closing the SQL engine releases the underlying filesystem lock
// for a newly created database so subsequent opens can proceed.
func TestCreateDatabase_ReleasesLockOnEngineClose(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("skipping on windows due to differing file locking semantics")
}

ctx := context.Background()
root := t.TempDir()

fs, err := filesys.LocalFS.WithWorkingDir(root)
require.NoError(t, err)

cfg := config.NewMapConfig(map[string]string{
config.UserNameKey: "test",
config.UserEmailKey: "test@example.com",
})

dbLoadParams := map[string]interface{}{
dbfactory.DisableSingletonCacheParam: struct{}{},
dbfactory.FailOnJournalLockTimeoutParam: struct{}{},
}

rootEnv := env.LoadWithoutDB(ctx, env.GetCurrentUserHomeDir, fs, doltdb.LocalDirDoltDB, "test")
rootEnv.DBLoadParams = map[string]interface{}{
dbfactory.DisableSingletonCacheParam: struct{}{},
dbfactory.FailOnJournalLockTimeoutParam: struct{}{},
}
mrEnv, err := env.MultiEnvForDirectory(ctx, cfg, fs, "test", rootEnv)
require.NoError(t, err)

seCfg := &SqlEngineConfig{
ServerUser: "root",
ServerHost: "localhost",
Autocommit: true,
DBLoadParams: dbLoadParams,
}

se, err := NewSqlEngine(ctx, mrEnv, seCfg)
require.NoError(t, err)

sqlCtx, err := se.NewLocalContext(ctx)
require.NoError(t, err)

_, _, _, err = se.Query(sqlCtx, "CREATE DATABASE IF NOT EXISTS testdb")
require.NoError(t, err)

err = se.Close()
require.True(t, err == nil || errors.Is(err, context.Canceled), "unexpected close error: %v", err)

// If the DB is properly closed, we should be able to take the lock quickly.
lockPath := filepath.Join(root, "testdb", ".dolt", "noms", "LOCK")
_, err = os.Stat(lockPath)
require.NoError(t, err, "expected lock file to exist at %s", lockPath)

lck := fslock.New(lockPath)
err = lck.LockWithTimeout(25 * time.Millisecond)
require.NoError(t, err, "expected lock to be free after engine close (path=%s)", lockPath)
require.NoError(t, lck.Unlock())
}
3 changes: 3 additions & 0 deletions go/cmd/dolt/commands/engine/sqlengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ func NewSqlEngine(
return nil, err
}
pro = pro.WithRemoteDialer(mrEnv.RemoteDialProvider())
if config != nil && len(config.DBLoadParams) > 0 {
pro.SetDBLoadParams(config.DBLoadParams)
}

config.ClusterController.RegisterStoredProcedures(pro)
if config.ClusterController != nil {
Expand Down
18 changes: 10 additions & 8 deletions go/libraries/doltcore/env/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ func (dEnv *DoltEnv) DoltDB(ctx context.Context) *doltdb.DoltDB {

func (dEnv *DoltEnv) LoadDoltDBWithParams(ctx context.Context, nbf *types.NomsBinFormat, urlStr string, fs filesys.Filesys, params map[string]interface{}) error {
if dEnv.doltDB == nil {
if nbf == nil {
nbf = types.Format_Default
}
// Merge any environment-level DB load params without mutating the caller's map.
if len(dEnv.DBLoadParams) > 0 {
if params == nil {
Expand All @@ -136,11 +139,14 @@ func (dEnv *DoltEnv) LoadDoltDBWithParams(ctx context.Context, nbf *types.NomsBi
maps.Copy(params, dEnv.DBLoadParams)
}
}
ddb, err := doltdb.LoadDoltDBWithParams(ctx, types.Format_Default, urlStr, fs, params)
ddb, err := doltdb.LoadDoltDBWithParams(ctx, nbf, urlStr, fs, params)
if err != nil {
dEnv.DBLoadError = err
return err
}
dEnv.doltDB = ddb
dEnv.urlStr = urlStr
dEnv.DBLoadError = nil
}
return nil
}
Expand Down Expand Up @@ -506,9 +512,7 @@ func (dEnv *DoltEnv) InitRepoWithNoData(ctx context.Context, nbf *types.NomsBinF
return err
}

dEnv.doltDB, err = doltdb.LoadDoltDB(ctx, nbf, dEnv.urlStr, dEnv.FS)

return err
return dEnv.LoadDoltDBWithParams(ctx, nbf, dEnv.urlStr, dEnv.FS, nil)
}

var ErrCannotCreateDirDoesNotExist = errors.New("dir does not exist")
Expand Down Expand Up @@ -640,13 +644,11 @@ func (dEnv *DoltEnv) InitDBWithTime(ctx context.Context, nbf *types.NomsBinForma
}

func (dEnv *DoltEnv) InitDBWithCommitMetaGenerator(ctx context.Context, nbf *types.NomsBinFormat, branchName string, commitMeta datas.CommitMetaGenerator) error {
var err error
dEnv.doltDB, err = doltdb.LoadDoltDB(ctx, nbf, dEnv.urlStr, dEnv.FS)
if err != nil {
if err := dEnv.LoadDoltDBWithParams(ctx, nbf, dEnv.urlStr, dEnv.FS, nil); err != nil {
return err
}

err = dEnv.DoltDB(ctx).WriteEmptyRepoWithCommitMetaGenerator(ctx, branchName, commitMeta)
err := dEnv.DoltDB(ctx).WriteEmptyRepoWithCommitMetaGenerator(ctx, branchName, commitMeta)
if err != nil {
return fmt.Errorf("%w: %v", doltdb.ErrNomsIO, err)
}
Expand Down
79 changes: 77 additions & 2 deletions go/libraries/doltcore/sqle/database_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"errors"
"fmt"
"maps"
"path/filepath"
"sort"
"strings"
Expand Down Expand Up @@ -52,6 +53,13 @@ type DoltDatabaseProvider struct {
fs filesys.Filesys
remoteDialer dbfactory.GRPCDialProvider // TODO: why isn't this a method defined on the remote object

// dbLoadParams are optional parameters to apply when loading local file-backed databases created
// via provider-managed code paths (e.g. CREATE DATABASE / dolt_clone / undrop). These should match
// the params threaded into env.DoltEnv.DBLoadParams by higher layers (e.g. engine.SqlEngineConfig.DBLoadParams).
//
// Note: these params are only effective if they are set before the underlying DoltDB is loaded.
dbLoadParams map[string]interface{}

// dbLocations maps a database name to its file system root
dbLocations map[string]filesys.Filesys
databases map[string]dsess.SqlDatabase
Expand Down Expand Up @@ -195,6 +203,31 @@ func (p *DoltDatabaseProvider) WithRemoteDialer(provider dbfactory.GRPCDialProvi
return &cp
}

// SetDBLoadParams sets optional DB load params for newly created / registered databases. The provided map is cloned.
func (p *DoltDatabaseProvider) SetDBLoadParams(params map[string]interface{}) {
p.mu.Lock()
defer p.mu.Unlock()
if len(params) == 0 {
p.dbLoadParams = nil
return
}
p.dbLoadParams = maps.Clone(params)
}

func (p *DoltDatabaseProvider) applyDBLoadParamsToEnv(dEnv *env.DoltEnv) {
if dEnv == nil {
return
}
if len(p.dbLoadParams) == 0 {
return
}
if dEnv.DBLoadParams == nil {
dEnv.DBLoadParams = maps.Clone(p.dbLoadParams)
return
}
maps.Copy(dEnv.DBLoadParams, p.dbLoadParams)
}

// AddInitDatabaseHook adds an InitDatabaseHook to this provider. The hook will be invoked
// whenever this provider creates a new database.
func (p *DoltDatabaseProvider) AddInitDatabaseHook(hook InitDatabaseHook) {
Expand All @@ -212,9 +245,43 @@ func (p *DoltDatabaseProvider) FileSystem() filesys.Filesys {
}

func (p *DoltDatabaseProvider) Close() {
p.mu.RLock()
closeDoltDBs := p.dbLoadParams != nil
if closeDoltDBs {
_, closeDoltDBs = p.dbLoadParams[dbfactory.DisableSingletonCacheParam]
}

// Copy the databases so we can close outside the lock.
dbs := make([]dsess.SqlDatabase, 0, len(p.databases))
for _, db := range p.databases {
dbs = append(dbs, db)
}
p.mu.RUnlock()

for _, db := range dbs {
db.Close()
}

// In embedded / nocache mode, the underlying DoltDBs are not tracked by the singleton cache, so
// the provider is responsible for closing them to release filesystem locks.
if closeDoltDBs {
seen := make(map[*doltdb.DoltDB]struct{})
for _, db := range dbs {
if db == nil {
continue
}
for _, ddb := range db.DoltDatabases() {
if ddb == nil {
continue
}
if _, ok := seen[ddb]; ok {
continue
}
seen[ddb] = struct{}{}
_ = ddb.Close()
}
}
}
}

// Installs an InitDatabaseHook which configures new databases--those
Expand Down Expand Up @@ -512,7 +579,9 @@ func (p *DoltDatabaseProvider) CreateCollatedDatabase(ctx *sql.Context, name str
}

// TODO: fill in version appropriately
newEnv := env.Load(ctx, env.GetCurrentUserHomeDir, newFs, p.dbFactoryUrl, "TODO")
// Use LoadWithoutDB so we can apply db-load params before any DB is opened.
newEnv := env.LoadWithoutDB(ctx, env.GetCurrentUserHomeDir, newFs, p.dbFactoryUrl, "TODO")
p.applyDBLoadParamsToEnv(newEnv)

newDbStorageFormat := types.Format_Default
err = newEnv.InitRepo(ctx, newDbStorageFormat, sess.Username(), sess.Email(), p.defaultBranch)
Expand Down Expand Up @@ -753,6 +822,7 @@ func (p *DoltDatabaseProvider) cloneDatabaseFromRemote(
if err != nil {
return err
}
p.applyDBLoadParamsToEnv(dEnv)

err = actions.CloneRemote(ctx, srcDB, remoteName, branch, false, depth, dEnv)
if err != nil {
Expand Down Expand Up @@ -860,7 +930,9 @@ func (p *DoltDatabaseProvider) UndropDatabase(ctx *sql.Context, name string) (er
return err
}

newEnv := env.Load(ctx, env.GetCurrentUserHomeDir, newFs, p.dbFactoryUrl, "TODO")
// Use LoadWithoutDB so we can apply db-load params before any DB is opened.
newEnv := env.LoadWithoutDB(ctx, env.GetCurrentUserHomeDir, newFs, p.dbFactoryUrl, "TODO")
p.applyDBLoadParamsToEnv(newEnv)
return p.registerNewDatabase(ctx, exactCaseName, newEnv)
}

Expand All @@ -882,6 +954,9 @@ func (p *DoltDatabaseProvider) registerNewDatabase(ctx *sql.Context, name string
return fmt.Errorf("unable to register new database without database provider mutex being locked")
}

// Ensure any provider-supplied DB load params are applied before any lazy DB load occurs.
p.applyDBLoadParamsToEnv(newEnv)

fkChecks, err := ctx.GetSessionVariable(ctx, "foreign_key_checks")
if err != nil {
return err
Expand Down
5 changes: 4 additions & 1 deletion go/libraries/doltcore/sqle/logictest/dolt/doltharness.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ type DoltHarness struct {
func (h *DoltHarness) Close() {
dbs := h.sess.Provider().AllDatabases(sql.NewEmptyContext())
for _, db := range dbs {
db.(dsess.SqlDatabase).DbData().Ddb.Close()
// Close the sql-layer database resources (global state, background threads, etc).
// Do NOT close the underlying DoltDB here; this harness reuses a shared *env.DoltEnv
// across multiple init/teardown cycles (see doltharness_test.go).
db.(dsess.SqlDatabase).Close()
}
}

Expand Down
10 changes: 9 additions & 1 deletion go/libraries/doltcore/sqle/statspro/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"errors"
"fmt"
"log"
"maps"
"path"
"path/filepath"
"strings"
Expand Down Expand Up @@ -588,7 +589,11 @@ func (sc *StatsController) initStorage(ctx context.Context, fs filesys.Filesys)
return nil, fmt.Errorf("unable to make directory '%s', cause: %s", dbfactory.DoltStatsDir, err.Error())
}

dEnv = env.Load(ctx, sc.hdpEnv.GetUserHomeDir, statsFs, urlPath, "test")
// Use LoadWithoutDB so DB load params can be applied before any DB is opened.
dEnv = env.LoadWithoutDB(ctx, sc.hdpEnv.GetUserHomeDir, statsFs, urlPath, doltversion.Version)
if sc.hdpEnv != nil && len(sc.hdpEnv.DBLoadParams) > 0 {
dEnv.DBLoadParams = maps.Clone(sc.hdpEnv.DBLoadParams)
}
err = dEnv.InitRepo(ctx, types.Format_Default, "stats", "stats@stats.com", env.DefaultInitBranch)
if err != nil {
return nil, err
Expand All @@ -597,6 +602,9 @@ func (sc *StatsController) initStorage(ctx context.Context, fs filesys.Filesys)
return nil, fmt.Errorf("file exists where the dolt stats directory should be")
} else {
dEnv = env.LoadWithoutDB(ctx, sc.hdpEnv.GetUserHomeDir, statsFs, "", doltversion.Version)
if sc.hdpEnv != nil && len(sc.hdpEnv.DBLoadParams) > 0 {
dEnv.DBLoadParams = maps.Clone(sc.hdpEnv.DBLoadParams)
}
}

if err := dEnv.LoadDoltDBWithParams(ctx, types.Format_Default, urlPath, statsFs, params); err != nil {
Expand Down
28 changes: 27 additions & 1 deletion go/libraries/doltcore/sqle/statspro/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,17 @@ func (sc *StatsController) Gc(ctx *sql.Context) error {
}

func (sc *StatsController) Close() {
var (
doneCh chan struct{}
kv StatsKv
)

sc.mu.Lock()
defer sc.mu.Unlock()

// Already closed.
select {
case <-sc.closed:
sc.mu.Unlock()
return
default:
}
Expand All @@ -260,5 +265,26 @@ func (sc *StatsController) Close() {
sc.signalListener(leStop)

close(sc.closed)
doneCh = sc.workerDoneCh
kv = sc.kv
sc.mu.Unlock()

// Best-effort wait for worker exit to avoid racing a close of underlying storage.
if doneCh != nil {
select {
case <-doneCh:
case <-time.After(1 * time.Second):
}
}

// If we're using a prolly-backed stats store, it owns a DoltDB with its own filesystem locks.
// Close it best-effort on shutdown so embedded callers can reopen without contention.
if ps, ok := kv.(*prollyStats); ok && ps != nil && ps.destDb != nil {
for _, ddb := range ps.destDb.DoltDatabases() {
if ddb != nil {
_ = ddb.Close()
}
}
}
return
}
Loading