Package: syncthing Version: 1.12.1~ds1-2 Severity: important As discussed on the go-team mailing list [1], I am filing this bug to address an important fix in upstream since the version to be included in stable.
There have been reports of full index retransfers on almost every restart [2], which uses significant resources (network, disk io, ...). The underlying bug may also cause inconsistencies in the displayed sync completion, though that has not been observed. I propose to apply the PR fixing this [3] with the patch attached. Please indicate if you'd prefer an MR on salsa, then I'll do that instead in the future. [1]: https://lists.debian.org/debian-go/2021/02/msg00067.html [2]: https://forum.syncthing.net/t/mismatching-index-id-for-us-on-almost-every-restart-again/16036 [3]: https://github.com/syncthing/syncthing/pull/7211
>From 5ee8b8a6aa4b5366a721f0531ef4df45780a3801 Mon Sep 17 00:00:00 2001 From: Simon Frei <freisi...@gmail.com> Date: Mon, 21 Dec 2020 11:32:59 +0100 Subject: [PATCH 1/4] lib/db: Prevent IndexID creation race (#7211) --- lib/db/lowlevel.go | 2 ++ lib/db/set.go | 33 ++++++++++++++++++++++----------- lib/db/set_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 11 deletions(-) diff --git a/lib/db/lowlevel.go b/lib/db/lowlevel.go index 8074564aa..347237ea4 100644 --- a/lib/db/lowlevel.go +++ b/lib/db/lowlevel.go @@ -65,6 +65,7 @@ type Lowlevel struct { gcKeyCount int indirectGCInterval time.Duration recheckInterval time.Duration + oneFileSetCreated chan struct{} } func NewLowlevel(backend backend.Backend, opts ...Option) *Lowlevel { @@ -81,6 +82,7 @@ func NewLowlevel(backend backend.Backend, opts ...Option) *Lowlevel { gcMut: sync.NewRWMutex(), indirectGCInterval: indirectGCDefaultInterval, recheckInterval: recheckDefaultInterval, + oneFileSetCreated: make(chan struct{}), } for _, opt := range opts { opt(db) diff --git a/lib/db/set.go b/lib/db/set.go index 41e1c7ecd..66bf2ccc6 100644 --- a/lib/db/set.go +++ b/lib/db/set.go @@ -39,13 +39,27 @@ type FileSet struct { type Iterator func(f protocol.FileIntf) bool func NewFileSet(folder string, fs fs.Filesystem, db *Lowlevel) *FileSet { - return &FileSet{ + select { + case <-db.oneFileSetCreated: + default: + close(db.oneFileSetCreated) + } + s := &FileSet{ folder: folder, fs: fs, db: db, meta: db.loadMetadataTracker(folder), updateMutex: sync.NewMutex(), } + if id := s.IndexID(protocol.LocalDeviceID); id == 0 { + // No index ID set yet. We create one now. + id = protocol.NewIndexID() + err := s.db.setIndexID(protocol.LocalDeviceID[:], []byte(s.folder), id) + if err != nil && !backend.IsClosed(err) { + fatalError(err, fmt.Sprintf("%s Creating new IndexID", s.folder), s.db) + } + } + return s } func (s *FileSet) Drop(device protocol.DeviceID) { @@ -357,16 +371,6 @@ func (s *FileSet) IndexID(device protocol.DeviceID) protocol.IndexID { } else if err != nil { fatalError(err, opStr, s.db) } - if id == 0 && device == protocol.LocalDeviceID { - // No index ID set yet. We create one now. - id = protocol.NewIndexID() - err := s.db.setIndexID(device[:], []byte(s.folder), id) - if backend.IsClosed(err) { - return 0 - } else if err != nil { - fatalError(err, opStr, s.db) - } - } return id } @@ -432,7 +436,14 @@ func DropFolder(db *Lowlevel, folder string) { // DropDeltaIndexIDs removes all delta index IDs from the database. // This will cause a full index transmission on the next connection. +// Must be called before using FileSets, i.e. before NewFileSet is called for +// the first time. func DropDeltaIndexIDs(db *Lowlevel) { + select { + case <-db.oneFileSetCreated: + panic("DropDeltaIndexIDs must not be called after NewFileSet for the same Lowlevel") + default: + } opStr := "DropDeltaIndexIDs" l.Debugf(opStr) dbi, err := db.NewPrefixIterator([]byte{KeyTypeIndexID}) diff --git a/lib/db/set_test.go b/lib/db/set_test.go index e2833ffbb..89adee96d 100644 --- a/lib/db/set_test.go +++ b/lib/db/set_test.go @@ -1757,6 +1757,32 @@ func TestNoIndexIDResetOnDrop(t *testing.T) { } } +func TestConcurrentIndexID(t *testing.T) { + done := make(chan struct{}) + var ids [2]protocol.IndexID + setID := func(s *db.FileSet, i int) { + ids[i] = s.IndexID(protocol.LocalDeviceID) + done <- struct{}{} + } + + max := 100 + if testing.Short() { + max = 10 + } + for i := 0; i < max; i++ { + ldb := db.NewLowlevel(backend.OpenMemory()) + s := db.NewFileSet("test", fs.NewFilesystem(fs.FilesystemTypeFake, ""), ldb) + go setID(s, 0) + go setID(s, 1) + <-done + <-done + ldb.Close() + if ids[0] != ids[1] { + t.Fatalf("IDs differ after %v rounds", i) + } + } +} + func replace(fs *db.FileSet, device protocol.DeviceID, files []protocol.FileInfo) { fs.Drop(device) fs.Update(device, files) -- 2.30.0
_______________________________________________ Pkg-go-maintainers mailing list Pkg-go-maintainers@alioth-lists.debian.net https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-go-maintainers