From 5de6f6d3496c0cc9ed4755073a265a53083559ce Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Fri, 28 Feb 2020 11:16:33 +0100 Subject: [PATCH] lib/db: Correct metadata recalculation (fixes #6381) (#6382) If we decide to recalculate the metadata we shouldn't start from whatever we loaded from the database, as that data is wrong. We should start from a clean slate. --- lib/db/meta_test.go | 74 +++++++++++++++++++++++++++++++++++++++++++++ lib/db/set.go | 29 ++++++++++-------- 2 files changed, 90 insertions(+), 13 deletions(-) diff --git a/lib/db/meta_test.go b/lib/db/meta_test.go index 661b8ed27..fca86fd01 100644 --- a/lib/db/meta_test.go +++ b/lib/db/meta_test.go @@ -11,6 +11,8 @@ import ( "sort" "testing" + "github.com/syncthing/syncthing/lib/db/backend" + "github.com/syncthing/syncthing/lib/fs" "github.com/syncthing/syncthing/lib/protocol" ) @@ -101,3 +103,75 @@ func TestMetaSequences(t *testing.T) { t.Error("sequence of first device should be 4, not", seq) } } + +func TestRecalcMeta(t *testing.T) { + ldb := NewLowlevel(backend.OpenMemory()) + defer ldb.Close() + + // Add some files + s1 := NewFileSet("test", fs.NewFilesystem(fs.FilesystemTypeFake, "fake"), ldb) + files := []protocol.FileInfo{ + {Name: "a", Size: 1000}, + {Name: "b", Size: 2000}, + } + s1.Update(protocol.LocalDeviceID, files) + + // Verify local/global size + snap := s1.Snapshot() + ls := snap.LocalSize() + gs := snap.GlobalSize() + snap.Release() + if ls.Bytes != 3000 { + t.Fatalf("Wrong initial local byte count, %d != 3000", ls.Bytes) + } + if gs.Bytes != 3000 { + t.Fatalf("Wrong initial global byte count, %d != 3000", gs.Bytes) + } + + // Reach into the database to make the metadata tracker intentionally + // wrong and out of date + curSeq := s1.meta.Sequence(protocol.LocalDeviceID) + tran, err := ldb.newReadWriteTransaction() + if err != nil { + t.Fatal(err) + } + s1.meta.mut.Lock() + s1.meta.countsPtr(protocol.LocalDeviceID, 0).Sequence = curSeq - 1 // too low + s1.meta.countsPtr(protocol.LocalDeviceID, 0).Bytes = 1234 // wrong + s1.meta.countsPtr(protocol.GlobalDeviceID, 0).Bytes = 1234 // wrong + s1.meta.dirty = true + s1.meta.mut.Unlock() + if err := s1.meta.toDB(tran, []byte("test")); err != nil { + t.Fatal(err) + } + if err := tran.Commit(); err != nil { + t.Fatal(err) + } + + // Verify that our bad data "took" + snap = s1.Snapshot() + ls = snap.LocalSize() + gs = snap.GlobalSize() + snap.Release() + if ls.Bytes != 1234 { + t.Fatalf("Wrong changed local byte count, %d != 1234", ls.Bytes) + } + if gs.Bytes != 1234 { + t.Fatalf("Wrong changed global byte count, %d != 1234", gs.Bytes) + } + + // Create a new fileset, which will realize the inconsistency and recalculate + s2 := NewFileSet("test", fs.NewFilesystem(fs.FilesystemTypeFake, "fake"), ldb) + + // Verify local/global size + snap = s2.Snapshot() + ls = snap.LocalSize() + gs = snap.GlobalSize() + snap.Release() + if ls.Bytes != 3000 { + t.Fatalf("Wrong fixed local byte count, %d != 3000", ls.Bytes) + } + if gs.Bytes != 3000 { + t.Fatalf("Wrong fixed global byte count, %d != 3000", gs.Bytes) + } +} diff --git a/lib/db/set.go b/lib/db/set.go index e84659d28..b20782cd7 100644 --- a/lib/db/set.go +++ b/lib/db/set.go @@ -81,10 +81,9 @@ func NewFileSet(folder string, fs fs.Filesystem, db *Lowlevel) *FileSet { } func loadMetadataTracker(db *Lowlevel, folder string) *metadataTracker { - meta := newMetadataTracker() - recalc := func() *metadataTracker { - if err := recalcMeta(meta, db, folder); backend.IsClosed(err) { + meta, err := recalcMeta(db, folder) + if backend.IsClosed(err) { return nil } else if err != nil { panic(err) @@ -92,12 +91,14 @@ func loadMetadataTracker(db *Lowlevel, folder string) *metadataTracker { return meta } + meta := newMetadataTracker() if err := meta.fromDB(db, []byte(folder)); err != nil { l.Infof("No stored folder metadata for %q; recalculating", folder) return recalc() } - if metaOK := verifyLocalSequence(meta, db, folder); !metaOK { + curSeq := meta.Sequence(protocol.LocalDeviceID) + if metaOK := verifyLocalSequence(curSeq, db, folder); !metaOK { l.Infof("Stored folder metadata for %q is out of date after crash; recalculating", folder) return recalc() } @@ -110,14 +111,15 @@ func loadMetadataTracker(db *Lowlevel, folder string) *metadataTracker { return meta } -func recalcMeta(meta *metadataTracker, db *Lowlevel, folder string) error { +func recalcMeta(db *Lowlevel, folder string) (*metadataTracker, error) { + meta := newMetadataTracker() if err := db.checkGlobals([]byte(folder), meta); err != nil { - return err + return nil, err } t, err := db.newReadWriteTransaction() if err != nil { - return err + return nil, err } defer t.close() @@ -128,19 +130,22 @@ func recalcMeta(meta *metadataTracker, db *Lowlevel, folder string) error { return true }) if err != nil { - return err + return nil, err } meta.SetCreated() if err := meta.toDB(t, []byte(folder)); err != nil { - return err + return nil, err } - return t.Commit() + if err := t.Commit(); err != nil { + return nil, err + } + return meta, nil } // Verify the local sequence number from actual sequence entries. Returns // true if it was all good, or false if a fixup was necessary. -func verifyLocalSequence(meta *metadataTracker, db *Lowlevel, folder string) bool { +func verifyLocalSequence(curSeq int64, db *Lowlevel, folder string) bool { // Walk the sequence index from the current (supposedly) highest // sequence number and raise the alarm if we get anything. This recovers // from the occasion where we have written sequence entries to disk but @@ -151,8 +156,6 @@ func verifyLocalSequence(meta *metadataTracker, db *Lowlevel, folder string) boo // number than we've actually seen and receive some duplicate updates // and then be in sync again. - curSeq := meta.Sequence(protocol.LocalDeviceID) - t, err := db.newReadOnlyTransaction() if err != nil { panic(err)