Files
syncthing/internal/db/sqlite/db_open.go
T
Jakob BorgandGitHub 946e2b83a1 fix(db, model): better handle db connections, puller concurrency, avoid deadlock (fixes #10841) (#10842)
There is a danger of deadlock wherever we have database operations
inside a database iterator. The iterator itself pins a connection, so
the nested operations need another connection; if we've reached
maxOpenConns then it blocks until a connection is released. If all
connections are consumed by such iterators then none can make progress.

Luckily, we have limited concurrency for most such iterator loops. They
are part of scanning, pulling, reverting, etc where there is only ever
one such routine per folder. The exception is block reuse in the copier
routine, which is limited by the `Copiers` setting per folder. This
could in practice deadlock since you could set copiers to eight and end
up with six `AllLocalBlocksWithHash` iterators when `maxDBConns=6`, all
of which need to make additional database calls inside the loop.

This PR fixes the problem twice;

- The problematic loop does not need to be reentrant. The set of blocks
that may be returned by the iterator is finite so we can easily just
collect them to a slice before we start processing them. This avoids the
problem entirely.
- We do not need to limit database connections as strictly as we
currently do. Increase the maximum allowed, while reducing the number of
held-open idle connections slightly. This is not an exact science, but
ideally we want "most" operations to be able to use the pinned
connections to avoid cache churn. Most operations are short lived
queries, or single-goroutine iterators with short lived queries inside,
so four connections seems like it should usually be enough. 🤷
- Additionally, set a cap on Copiers. Currently you could set it to an
arbitrarily large number, which is not advantageous. Limit it to
2*NumCPU which scales somewhat with system performance.

Signed-off-by: Jakob Borg <jakob@kastelo.net>
2026-08-03 23:36:05 +02:00

186 lines
4.8 KiB
Go

// Copyright (C) 2025 The Syncthing Authors.
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.
package sqlite
import (
"fmt"
"log/slog"
"os"
"path/filepath"
"runtime"
"sync"
"time"
"github.com/syncthing/syncthing/internal/db"
"github.com/syncthing/syncthing/internal/slogutil"
"github.com/syncthing/syncthing/lib/build"
)
const (
// maxIdleConns is set so that most db operations will usually fit
// within one of those connections and hence use their persistent page
// cache. Additional connections on top of these will allocate their own
// page cache (same as any other), but it will be deallocated on
// connection close.
maxIdleConns = 4
minDeleteRetention = 24 * time.Hour
)
// maxOpenConns is sized for handling spikes. The primary driver is the
// Copiers folder option which may result in up to 2*NumCPU concurrent
// iterations. We reserve additional space on top of this to serve
// additional operations, some of which may be reentrant (queries within
// iterators) without deadlock.
var maxOpenConns = max(16, 4*runtime.NumCPU())
type DB struct {
*baseDB
pathBase string
deleteRetention time.Duration
folderDBsMut sync.RWMutex
folderDBs map[string]*folderDB
folderDBOpener func(folder, path string, deleteRetention time.Duration) (*folderDB, error)
}
var _ db.DB = (*DB)(nil)
type Option func(*DB)
func WithDeleteRetention(d time.Duration) Option {
return func(s *DB) {
if d <= 0 {
s.deleteRetention = 0
} else {
s.deleteRetention = max(d, minDeleteRetention)
}
}
}
func Open(path string, opts ...Option) (*DB, error) {
pragmas := []string{
"journal_mode = WAL",
"optimize = 0x10002",
"auto_vacuum = INCREMENTAL",
fmt.Sprintf("application_id = %d", applicationIDMain),
}
schemas := []string{
"sql/schema/common/*",
"sql/schema/main/*",
}
migrations := []string{
"sql/migrations/common/*",
"sql/migrations/main/*",
}
_ = os.MkdirAll(path, 0o700)
initTmpDir(path)
mainPath := filepath.Join(path, "main.db")
mainBase, err := openBase(mainPath, maxOpenConns, maxIdleConns, pragmas, schemas, migrations)
if err != nil {
return nil, err
}
db := &DB{
pathBase: path,
baseDB: mainBase,
folderDBs: make(map[string]*folderDB),
folderDBOpener: openFolderDB,
}
for _, opt := range opts {
opt(db)
}
if err := db.cleanDroppedFolders(); err != nil {
slog.Warn("Failed to clean dropped folders", slogutil.Error(err))
}
if err := db.startFolderDatabases(); err != nil {
return nil, wrap(err)
}
return db, nil
}
// Open the database with options suitable for the migration inserts. This
// is not a safe mode of operation for normal processing, use only for bulk
// inserts with a close afterwards.
func OpenForMigration(path string) (*DB, error) {
pragmas := []string{
"journal_mode = OFF",
"foreign_keys = 0",
"synchronous = 0",
"locking_mode = EXCLUSIVE",
fmt.Sprintf("application_id = %d", applicationIDMain),
}
schemas := []string{
"sql/schema/common/*",
"sql/schema/main/*",
}
migrations := []string{
"sql/migrations/common/*",
"sql/migrations/main/*",
}
_ = os.MkdirAll(path, 0o700)
initTmpDir(path)
mainPath := filepath.Join(path, "main.db")
mainBase, err := openBase(mainPath, 1, 1, pragmas, schemas, migrations)
if err != nil {
return nil, err
}
db := &DB{
pathBase: path,
baseDB: mainBase,
folderDBs: make(map[string]*folderDB),
folderDBOpener: openFolderDBForMigration,
}
if err := db.cleanDroppedFolders(); err != nil {
slog.Warn("Failed to clean dropped folders", slogutil.Error(err))
}
return db, nil
}
func (s *DB) Close() error {
s.folderDBsMut.Lock()
defer s.folderDBsMut.Unlock()
for folder, fdb := range s.folderDBs {
fdb.Close()
delete(s.folderDBs, folder)
}
return wrap(s.baseDB.Close())
}
func initTmpDir(path string) {
if build.IsWindows || build.IsDarwin || os.Getenv("SQLITE_TMPDIR") != "" {
// Doesn't use SQLITE_TMPDIR, isn't likely to have a tiny
// ram-backed temp directory, or already set to something.
return
}
// Attempt to override the SQLite temporary directory by setting the
// env var prior to the (first) database being opened and hence
// SQLite becoming initialized. We set the temp dir to the same
// place we store the database, in the hope that there will be
// enough space there for the operations it needs to perform, as
// opposed to /tmp and similar, on some systems.
dbTmpDir := filepath.Join(path, ".tmp")
if err := os.MkdirAll(dbTmpDir, 0o700); err == nil {
os.Setenv("SQLITE_TMPDIR", dbTmpDir)
} else {
slog.Warn("Failed to create temp directory for SQLite", slogutil.FilePath(dbTmpDir), slogutil.Error(err))
}
}