Jakob BorgandGitHub
946e2b83a1
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>
112 lines
2.7 KiB
Go
112 lines
2.7 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"
|
|
"time"
|
|
|
|
"github.com/syncthing/syncthing/lib/protocol"
|
|
)
|
|
|
|
type folderDB struct {
|
|
*baseDB
|
|
|
|
folderID string
|
|
|
|
localDeviceIdx int64
|
|
deleteRetention time.Duration
|
|
}
|
|
|
|
func openFolderDB(folder, path string, deleteRetention time.Duration) (*folderDB, error) {
|
|
pragmas := []string{
|
|
"journal_mode = WAL",
|
|
"optimize = 0x10002",
|
|
"auto_vacuum = INCREMENTAL",
|
|
fmt.Sprintf("application_id = %d", applicationIDFolder),
|
|
}
|
|
schemas := []string{
|
|
"sql/schema/common/*",
|
|
"sql/schema/folder/*",
|
|
}
|
|
migrations := []string{
|
|
"sql/migrations/common/*",
|
|
"sql/migrations/folder/*",
|
|
}
|
|
|
|
base, err := openBase(path, maxOpenConns, maxIdleConns, pragmas, schemas, migrations)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
fdb := &folderDB{
|
|
folderID: folder,
|
|
baseDB: base,
|
|
deleteRetention: deleteRetention,
|
|
}
|
|
|
|
_ = fdb.PutKV("folderID", []byte(folder))
|
|
|
|
// Touch device IDs that should always exist and have a low index
|
|
// numbers, and will never change
|
|
fdb.localDeviceIdx, _ = fdb.deviceIdxLocked(protocol.LocalDeviceID)
|
|
fdb.tplInput["LocalDeviceIdx"] = fdb.localDeviceIdx
|
|
|
|
return fdb, 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 openFolderDBForMigration(folder, path string, deleteRetention time.Duration) (*folderDB, error) {
|
|
pragmas := []string{
|
|
"journal_mode = OFF",
|
|
"foreign_keys = 0",
|
|
"synchronous = 0",
|
|
"locking_mode = EXCLUSIVE",
|
|
fmt.Sprintf("application_id = %d", applicationIDFolder),
|
|
}
|
|
schemas := []string{
|
|
"sql/schema/common/*",
|
|
"sql/schema/folder/*",
|
|
}
|
|
|
|
base, err := openBase(path, 1, 1, pragmas, schemas, nil)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
fdb := &folderDB{
|
|
folderID: folder,
|
|
baseDB: base,
|
|
deleteRetention: deleteRetention,
|
|
}
|
|
|
|
// Touch device IDs that should always exist and have a low index
|
|
// numbers, and will never change
|
|
fdb.localDeviceIdx, _ = fdb.deviceIdxLocked(protocol.LocalDeviceID)
|
|
fdb.tplInput["LocalDeviceIdx"] = fdb.localDeviceIdx
|
|
|
|
return fdb, nil
|
|
}
|
|
|
|
func (s *folderDB) deviceIdxLocked(deviceID protocol.DeviceID) (int64, error) {
|
|
devStr := deviceID.String()
|
|
var idx int64
|
|
if err := s.stmt(`
|
|
INSERT INTO devices(device_id)
|
|
VALUES (?)
|
|
ON CONFLICT(device_id) DO UPDATE
|
|
SET device_id = excluded.device_id
|
|
RETURNING idx
|
|
`).Get(&idx, devStr); err != nil {
|
|
return 0, wrap(err)
|
|
}
|
|
|
|
return idx, nil
|
|
}
|