Skip to content

Commit

Permalink
Avoid loading DB for commands where it's not necessary.
Browse files Browse the repository at this point in the history
  • Loading branch information
nicktobey committed Jan 22, 2025
1 parent 8ca3bac commit 9358401
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 12 deletions.
7 changes: 6 additions & 1 deletion go/cmd/dolt/commands/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,10 @@ func (cmd BranchCmd) Exec(ctx context.Context, commandStr string, args []string,
return status
}

errorBuilder := errhand.BuildDError("error: failed to create query engine")
queryEngine, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx)
if err != nil {
return HandleVErrAndExitCode(errhand.BuildDError("error: failed to create query engine").AddCause(err).Build(), nil)
return HandleVErrAndExitCode(errorBuilder.AddCause(err).Build(), nil)
}

if closeFunc != nil {
Expand All @@ -138,6 +139,10 @@ func (cmd BranchCmd) Exec(ctx context.Context, commandStr string, args []string,
case apr.Contains(showCurrentFlag):
return printCurrentBranch(sqlCtx, queryEngine)
case apr.Contains(datasetsFlag):
dEnv.ReloadDB(ctx)
if HandleDEnvErrorsAndExitCode(errorBuilder, dEnv, usage) {
return 1
}
return printAllDatasets(ctx, dEnv)
case apr.NArg() > 0:
return createBranch(sqlCtx, queryEngine, apr, args, usage)
Expand Down
4 changes: 4 additions & 0 deletions go/cmd/dolt/commands/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ func (cmd ShowCmd) Exec(ctx context.Context, commandStr string, args []string, d
cli.PrintErrln("`dolt show --no-pretty` or `dolt show (BRANCHNAME)` is not supported when using old LD_1 storage format.")
return 1
}
dEnv.ReloadDB(ctx)
if HandleDEnvErrorsAndExitCode(nil, dEnv, usage) {
return 1
}
}

specRefs := opts.specRefs
Expand Down
12 changes: 12 additions & 0 deletions go/cmd/dolt/commands/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,24 @@ func (cmd SqlCmd) Exec(ctx context.Context, commandStr string, args []string, dE

if query, queryOK := apr.GetValue(QueryFlag); queryOK {
if apr.Contains(saveFlag) {
dEnv.ReloadDB(ctx)
if HandleDEnvErrorsAndExitCode(nil, dEnv, usage) {
return 1
}
return execSaveQuery(sqlCtx, dEnv, queryist, apr, query, format, usage)
}
return queryMode(sqlCtx, queryist, apr, query, format, usage)
} else if savedQueryName, exOk := apr.GetValue(executeFlag); exOk {
dEnv.ReloadDB(ctx)
if HandleDEnvErrorsAndExitCode(nil, dEnv, usage) {
return 1
}
return executeSavedQuery(sqlCtx, queryist, dEnv, savedQueryName, format, usage)
} else if apr.Contains(listSavedFlag) {
dEnv.ReloadDB(ctx)
if HandleDEnvErrorsAndExitCode(nil, dEnv, usage) {
return 1
}
return listSavedQueries(sqlCtx, queryist, dEnv, format, usage)
} else {
// Run in either batch mode for piped input, or shell mode for interactive
Expand Down
22 changes: 22 additions & 0 deletions go/cmd/dolt/commands/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ func BuildSqlEngineQueryist(ctx context.Context, cwdFS filesys.Filesys, mrEnv *e
if ctx == nil || cwdFS == nil || mrEnv == nil || creds == nil || apr == nil {
errhand.VerboseErrorFromError(fmt.Errorf("Invariant violated. Nil argument provided to BuildSqlEngineQueryist"))
}
mrEnv.ReloadDBs(ctx)

// We want to know if the user provided us the data-dir flag, but we want to use the abs value used to
// create the DoltEnv. This is a little messy.
Expand Down Expand Up @@ -865,6 +866,27 @@ func HandleVErrAndExitCode(verr errhand.VerboseError, usage cli.UsagePrinter) in
return 0
}

func HandleDEnvErrorsAndExitCode(errorBuilder *errhand.DErrorBuilder, dEnv *env.DoltEnv, usage cli.UsagePrinter) bool {
errorCount := 0
handleError := func(err error) {
if err != nil {
var verboseError errhand.VerboseError
if errorBuilder != nil {
verboseError = errorBuilder.AddCause(err).Build()
} else {
verboseError = errhand.VerboseErrorFromError(err)
}
HandleVErrAndExitCode(verboseError, usage)
errorCount++
}
}
handleError(dEnv.CfgLoadErr)
handleError(dEnv.RSLoadErr)
handleError(dEnv.DBLoadError)

return errorCount > 0
}

// interpolateStoredProcedureCall returns an interpolated query to call |storedProcedureName| with the arguments
// |args|.
func interpolateStoredProcedureCall(storedProcedureName string, args []string) (string, error) {
Expand Down
46 changes: 45 additions & 1 deletion go/cmd/dolt/dolt.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,32 @@ var commandsWithoutCliCtx = []cli.Command{
commands.ProfileCmd{},
commands.ArchiveCmd{},
commands.FsckCmd{},
commands.ConfigCmd{},
}

// These commands have one of the following properties:
// - They require that the database does not exist
// - They never access the database
// - They only ever access the database through a CliContext.
// - They always call dEnv.Reload() before accessing the database for the first time.
// In these cases, we can defer loading the database until it's actually needed.
var commandsWithoutDBLoad = []cli.Command{
commands.InitCmd{},
commands.CloneCmd{},
commands.StatusCmd{},
commands.LsCmd{},
commands.AddCmd{},
commands.DiffCmd{},
commands.ResetCmd{},
commands.CleanCmd{},
commands.SqlCmd{VersionStr: doltversion.Version},
commands.LogCmd{},
commands.BranchCmd{},
commands.MergeCmd{},
commands.ShowCmd{},
commands.CherryPickCmd{},
commands.ConfigCmd{},
credcmds.Commands,
}

var commandsWithoutGlobalArgSupport = []cli.Command{
Expand Down Expand Up @@ -186,6 +212,15 @@ func initCliContext(commandName string) bool {
return true
}

func eagerlyLoadDB(commandName string) bool {
for _, command := range commandsWithoutDBLoad {
if command.Name() == commandName {
return false
}
}
return true
}

func supportsGlobalArgs(commandName string) bool {
for _, command := range commandsWithoutGlobalArgSupport {
if command.Name() == commandName {
Expand Down Expand Up @@ -474,7 +509,13 @@ func runMain() int {
args = nil

// This is the dEnv passed to sub-commands, and is used to create the multi-repo environment.
dEnv := env.Load(ctx, env.GetCurrentUserHomeDir, cfg.dataDirFS, doltdb.LocalDirDoltDB, doltversion.Version)
var dEnv *env.DoltEnv
if eagerlyLoadDB(cfg.subCommand) {
dEnv = env.Load(ctx, env.GetCurrentUserHomeDir, cfg.dataDirFS, doltdb.LocalDirDoltDB, doltversion.Version)
} else {
dEnv = env.LoadWithDeferredDB(ctx, env.GetCurrentUserHomeDir, cfg.dataDirFS, doltdb.LocalDirDoltDB, doltversion.Version)
}

if dEnv.CfgLoadErr != nil {
cli.PrintErrln(color.RedString("Failed to load the global config. %v", dEnv.CfgLoadErr))
return 1
Expand Down Expand Up @@ -536,6 +577,9 @@ func runMain() int {
cli.PrintErrln("failed to load database names")
return 1
}
if eagerlyLoadDB(cfg.subCommand) {
mrEnv.ReloadDBs(ctx)
}
_ = mrEnv.Iter(func(dbName string, dEnv *env.DoltEnv) (stop bool, err error) {
dsess.DefineSystemVariablesForDB(dbName)
return false, nil
Expand Down
16 changes: 14 additions & 2 deletions go/libraries/doltcore/env/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,21 @@ func LoadWithoutDB(_ context.Context, hdp HomeDirProvider, fs filesys.Filesys, v
// Load loads the DoltEnv for the .dolt directory determined by resolving the specified urlStr with the specified Filesys.
func Load(ctx context.Context, hdp HomeDirProvider, fs filesys.Filesys, urlStr string, version string) *DoltEnv {
dEnv := LoadWithoutDB(ctx, hdp, fs, version)
LoadDoltDB(ctx, fs, urlStr, dEnv)
return dEnv
}

func LoadWithDeferredDB(ctx context.Context, hdp HomeDirProvider, fs filesys.Filesys, urlStr string, version string) *DoltEnv {
dEnv := LoadWithoutDB(ctx, hdp, fs, version)
dEnv.urlStr = urlStr
return dEnv
}

func (dEnv *DoltEnv) ReloadDB(ctx context.Context) {
LoadDoltDB(ctx, dEnv.FS, dEnv.urlStr, dEnv)
}

func LoadDoltDB(ctx context.Context, fs filesys.Filesys, urlStr string, dEnv *DoltEnv) {
ddb, dbLoadErr := doltdb.LoadDoltDB(ctx, types.Format_Default, urlStr, fs)

dEnv.DoltDB = ddb
Expand Down Expand Up @@ -218,8 +232,6 @@ func Load(ctx context.Context, hdp HomeDirProvider, fs filesys.Filesys, urlStr s
dEnv.RSLoadErr = err
}
}

return dEnv
}

func GetDefaultInitBranch(cfg config.ReadableConfig) string {
Expand Down
47 changes: 39 additions & 8 deletions go/libraries/doltcore/env/multi_repo_env.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func MultiEnvForDirectory(
version = dEnv.Version
}

newEnv := Load(ctx, GetCurrentUserHomeDir, newFs, doltdb.LocalDirDoltDB, version)
newEnv := LoadWithDeferredDB(ctx, GetCurrentUserHomeDir, newFs, doltdb.LocalDirDoltDB, version)
if newEnv.Valid() {
envSet[dbfactory.DirToDBName(dir)] = newEnv
} else {
Expand All @@ -205,8 +205,6 @@ func MultiEnvForDirectory(
return false
})

enforceSingleFormat(envSet)

// if the current directory database is in our set, add it first so it will be the current database
if env, ok := envSet[dbName]; ok && env.Valid() {
mrEnv.addEnv(dbName, env)
Expand All @@ -226,6 +224,31 @@ func MultiEnvForDirectory(
return mrEnv, nil
}

func (mrEnv *MultiRepoEnv) ReloadDBs(
ctx context.Context,
) {
for _, namedEnv := range mrEnv.envs {
dEnv := namedEnv.env

if dEnv.DoltDB == nil {
dEnv.ReloadDB(ctx)
}
if !dEnv.Valid() {
dbErr := dEnv.DBLoadError
if dbErr != nil {
if !errors.Is(dbErr, doltdb.ErrMissingDoltDataDir) {
logrus.Warnf("failed to load database at %s with error: %s", dEnv.urlStr, dbErr.Error())
}
}
cfgErr := dEnv.CfgLoadErr
if cfgErr != nil {
logrus.Warnf("failed to load database configuration at %s with error: %s", dEnv.urlStr, cfgErr.Error())
}
}
}
mrEnv.envs = enforceSingleFormat(mrEnv.envs)
}

func (mrEnv *MultiRepoEnv) FileSystem() filesys.Filesys {
return mrEnv.fs
}
Expand Down Expand Up @@ -327,9 +350,10 @@ func getRepoRootDir(path, pathSeparator string) string {
// enforceSingleFormat enforces that constraint that all databases in
// a multi-database environment have the same NomsBinFormat.
// Databases are removed from the MultiRepoEnv to ensure this is true.
func enforceSingleFormat(envSet map[string]*DoltEnv) {
func enforceSingleFormat(envSet []NamedEnv) []NamedEnv {
formats := set.NewEmptyStrSet()
for _, dEnv := range envSet {
for _, namedEnv := range envSet {
dEnv := namedEnv.env
formats.Add(dEnv.DoltDB.Format().VersionString())
}

Expand All @@ -339,17 +363,24 @@ func enforceSingleFormat(envSet map[string]*DoltEnv) {
nbf = types.Format_Default.VersionString()
} else {
// otherwise, pick an arbitrary format
for _, dEnv := range envSet {
for _, namedEnv := range envSet {
dEnv := namedEnv.env
nbf = dEnv.DoltDB.Format().VersionString()
break
}
}

prunedEnvs := make([]NamedEnv, 0, len(envSet))
template := "incompatible format for database '%s'; expected '%s', found '%s'"
for name, dEnv := range envSet {
for _, namedEnv := range envSet {
name := namedEnv.name
dEnv := namedEnv.env
found := dEnv.DoltDB.Format().VersionString()
if found != nbf {
logrus.Infof(template, name, nbf, found)
delete(envSet, name)
} else {
prunedEnvs = append(prunedEnvs, namedEnv)
}
}
return prunedEnvs
}

0 comments on commit 9358401

Please sign in to comment.