go/store/nbs: Lazily load chunk journal database resources when constructing a NomsBlockStore.#11117
Merged
Conversation
…ckStore in some cases.
…ructing a NomsBlockStore. Most Dolt commands currently construct a MultiRepoEnv and then call into the SQL layer to perform the actual work. In turn, the SQL layer has some logic which looks at the state of the loaded databases and sees whether the process has ended up loading some of them in ReadOnly mode. If it has, then in some cases the Dolt process will attempt to connect to a running `dolt sql-server` instead of doing the work against the database files itself. This PR makes is to that the application code paths which inspect AccessMode == ReadOnly can get that signal and make the attempt to reach out to the server *before* all of the database state has to be loaded from disk. This greatly improves the performance of running `dolt` CLI commands against a running `dolt sql-server`, from the same directory as the server data-dir.
Contributor
|
@reltuk DOLT
|
macneale4
approved these changes
May 28, 2026
Contributor
macneale4
left a comment
macneale4
left a comment
There was a problem hiding this comment.
One println to remove before shipping. Other comments are up to you
| // The |warningsCb| callback is called with any errors encountered that we automatically recover from. This allows the caller | ||
| // to handle the situation in a context specific way. | ||
| func processJournalRecords(ctx context.Context, r io.ReadSeeker, tryTruncate bool, off int64, cb func(o int64, r journalRec) error, warningsCb func(error)) (int64, error) { | ||
| func processJournalRecords(ctx context.Context, path string, r io.ReadSeeker, tryTruncate bool, off int64, cb func(o int64, r journalRec) error, warningsCb func(error)) (int64, error) { |
Contributor
There was a problem hiding this comment.
nit. may be worth a gDoc comment that the path is used only for error reporting.
| } | ||
|
|
||
| func (nbs *NomsBlockStore) ChunkJournalSize() (int64, bool) { | ||
| if err := nbs.ensureLoad(context.Background()); err != nil { |
Contributor
There was a problem hiding this comment.
using a Background() ctx here, but a TODO() above. What's the thinking there?
Alternatively - plumb through an actual context?
Comment on lines
+824
to
+825
| // XXX: Just printing here for now so that we can audit when the databases are actually loaded. | ||
| fmt.Fprintf(color.Error, "actually loading database at %s\n%s\n", dir, string(debug.Stack())) |
| LoadDoltDB(ctx, dEnv) | ||
| dbErr := dEnv.DBLoadError | ||
| if dbErr != nil { | ||
| // XXX: We are not guaranteed to see these errors here, because the database might |
Contributor
There was a problem hiding this comment.
Not sure why we have a pronographic comment here, but we can probably delete it
Contributor
Contributor
…zy load NomsBlockStore work.
…ing Dolt to panic when it loads table files.
…orrect behavior of table file loading in the presence and absence of a running sql-server.
Contributor
4 tasks
6 tasks
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
3 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.
Most Dolt commands currently construct a MultiRepoEnv and then call
into the SQL layer to perform the actual work. In turn, the SQL layer
has some logic which looks at the state of the loaded databases and
sees whether the process has ended up loading some of them in ReadOnly
mode. If it has, then in some cases the Dolt process will attempt to
connect to a running
dolt sql-serverinstead of doing the workagainst the database files itself.
This PR makes is to that the application code paths which inspect
AccessMode == ReadOnly can get that signal and make the attempt to
reach out to the server before all of the database state has to be
loaded from disk. This greatly improves the performance of running
doltCLI commands against a runningdolt sql-server, from the samedirectory as the server data-dir.