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
58 changes: 57 additions & 1 deletion go/libraries/doltcore/sqle/database_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ type DoltDatabaseProvider struct {
droppedDatabaseManager *droppedDatabaseManager
overrides sql.EngineOverrides

// remoteDbs caches remote DoltDB instances by URL so that repeated push calls
// to the same remote reuse the store (and its already-opened table chunk sources)
// instead of re-opening every table file from the blobstore each time.
remoteDbs map[string]*doltdb.DoltDB

defaultBranch string
dbFactoryUrl string
DropDatabaseHooks []DropDatabaseHook
Expand Down Expand Up @@ -181,6 +186,7 @@ func NewDoltDatabaseProviderWithDatabases(defaultBranch string, fs filesys.Files
isStandby: new(bool),
droppedDatabaseManager: newDroppedDatabaseManager(fs),
overrides: overrides,
remoteDbs: make(map[string]*doltdb.DoltDB),
}, nil
}

Expand Down Expand Up @@ -294,6 +300,20 @@ func (p *DoltDatabaseProvider) Close() {
}
}
}

// Close cached remote databases.
var remoteDbs []*doltdb.DoltDB
func() {
p.mu.RLock()
defer p.mu.RUnlock()
remoteDbs = make([]*doltdb.DoltDB, 0, len(p.remoteDbs))
for _, rdb := range p.remoteDbs {
remoteDbs = append(remoteDbs, rdb)
}
}()
for _, rdb := range remoteDbs {
_ = rdb.Close()
}
}

// Installs an InitDatabaseHook which configures new databases--those
Expand Down Expand Up @@ -539,7 +559,43 @@ func (p *DoltDatabaseProvider) GetRemoteDB(ctx context.Context, format *types.No
}

if withCaching {
return r.GetRemoteDB(ctx, format, dialer)
// Only cache git-backed remote DBs. Other remote types (file://, aws, etc.)
// register their underlying NBS in a global singleton cache that is closed
// separately by CloseAllLocalDatabases(). Caching those here would cause a
// double-close panic on process exit.
isGitRemote := strings.HasPrefix(strings.ToLower(r.Url), "git+")
if isGitRemote {
cached := func() *doltdb.DoltDB {
p.mu.RLock()
defer p.mu.RUnlock()
return p.remoteDbs[r.Url]
}()
if cached != nil {
return cached, nil
}
}

remoteDB, err := r.GetRemoteDB(ctx, format, dialer)
if err != nil {
return nil, err
}

if isGitRemote {
cached := func() *doltdb.DoltDB {
p.mu.Lock()
defer p.mu.Unlock()
if existing, ok := p.remoteDbs[r.Url]; ok {
return existing
}
p.remoteDbs[r.Url] = remoteDB
return nil
}()
if cached != nil {
_ = remoteDB.Close()
return cached, nil
}
}
return remoteDB, nil
Comment on lines +566 to +598
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The git-remote DB cache can race: two goroutines can both miss p.remoteDbs[r.Url], both open a new remote DB, and then the later store overwrites the earlier without closing it. To avoid leaking open remotes, consider re-checking the cache under p.mu.Lock() (double-checked locking) and closing the newly-opened DB if another goroutine already cached one.

Copilot uses AI. Check for mistakes.
}
return r.GetRemoteDBWithoutCaching(ctx, format, dialer)
}
Expand Down
Loading
Loading