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
