From 23a0e18292a65e85fa2603e2e6a6e50bc52ebeb9 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Thu, 17 Jun 2021 10:15:11 +0200 Subject: [PATCH] lib/db: Fix accounting bug when dropping indexes (#7774) --- lib/db/schemaupdater.go | 4 ++-- lib/db/set_test.go | 35 +++++++++++++++++++++++++++++++++++ lib/db/structs.go | 11 ++++------- lib/db/transactions.go | 29 ++++++++++------------------- 4 files changed, 51 insertions(+), 28 deletions(-) diff --git a/lib/db/schemaupdater.go b/lib/db/schemaupdater.go index 2e2fc1864..3e9227d1c 100644 --- a/lib/db/schemaupdater.go +++ b/lib/db/schemaupdater.go @@ -20,7 +20,7 @@ import ( // do not put restrictions on downgrades (e.g. for repairs after a bugfix). const ( dbVersion = 14 - dbMigrationVersion = 18 + dbMigrationVersion = 19 dbMinSyncthingVersion = "v1.9.0" ) @@ -102,7 +102,7 @@ func (db *schemaUpdater) updateSchema() error { {14, 14, "v1.9.0", db.updateSchemaTo14}, {14, 16, "v1.9.0", db.checkRepairMigration}, {14, 17, "v1.9.0", db.migration17}, - {14, 18, "v1.9.0", db.dropIndexIDsMigration}, + {14, 19, "v1.9.0", db.dropIndexIDsMigration}, } for _, m := range migrations { diff --git a/lib/db/set_test.go b/lib/db/set_test.go index f4ec31239..eb7d6ed21 100644 --- a/lib/db/set_test.go +++ b/lib/db/set_test.go @@ -1784,6 +1784,41 @@ func TestConcurrentIndexID(t *testing.T) { } } +func TestNeedRemoveLastValid(t *testing.T) { + db := newLowlevelMemory(t) + defer db.Close() + + folder := "test" + testFs := fs.NewFilesystem(fs.FilesystemTypeFake, "") + + fs := newFileSet(t, folder, testFs, db) + + files := []protocol.FileInfo{ + {Name: "foo", Version: protocol.Vector{}.Update(myID), Sequence: 1}, + } + fs.Update(remoteDevice0, files) + files[0].Version = files[0].Version.Update(myID) + fs.Update(remoteDevice1, files) + files[0].LocalFlags = protocol.FlagLocalIgnored + fs.Update(protocol.LocalDeviceID, files) + + snap := snapshot(t, fs) + c := snap.NeedSize(remoteDevice0) + if c.Files != 1 { + t.Errorf("Expected 1 needed files initially, got %v", c.Files) + } + snap.Release() + + fs.Drop(remoteDevice1) + + snap = snapshot(t, fs) + c = snap.NeedSize(remoteDevice0) + if c.Files != 0 { + t.Errorf("Expected no needed files, got %v", c.Files) + } + snap.Release() +} + func replace(fs *db.FileSet, device protocol.DeviceID, files []protocol.FileInfo) { fs.Drop(device) fs.Update(device, files) diff --git a/lib/db/structs.go b/lib/db/structs.go index 2443ea21a..40e4a48a7 100644 --- a/lib/db/structs.go +++ b/lib/db/structs.go @@ -331,15 +331,12 @@ func (vl *VersionList) pop(device, name []byte) (FileVersion, bool, bool, error) oldFV := vl.RawVersions[i].copy() if invDevice { vl.RawVersions[i].InvalidDevices = popDeviceAt(vl.RawVersions[i].InvalidDevices, j) - } else { - vl.RawVersions[i].Devices = popDeviceAt(vl.RawVersions[i].Devices, j) + return oldFV, true, false, nil } + vl.RawVersions[i].Devices = popDeviceAt(vl.RawVersions[i].Devices, j) // If the last valid device of the previous global was removed above, - // the next entry is now the global entry (unless all entries are invalid). - if len(vl.RawVersions[i].Devices) == 0 && globalPos == i { - return oldFV, true, globalPos == vl.findGlobal(), nil - } - return oldFV, true, false, nil + // the global changed. + return oldFV, true, len(vl.RawVersions[i].Devices) == 0 && globalPos == i, nil } // Get returns a FileVersion that contains the given device and whether it has diff --git a/lib/db/transactions.go b/lib/db/transactions.go index ce0061b0c..d8ab3b29b 100644 --- a/lib/db/transactions.go +++ b/lib/db/transactions.go @@ -659,10 +659,7 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi // Must happen before updating global meta: If this is the first // item from this device, it will be initialized with the global state. - needBefore := false - if haveOldGlobal { - needBefore = Need(oldGlobalFV, haveRemoved, removedFV.Version) - } + needBefore := haveOldGlobal && Need(oldGlobalFV, haveRemoved, removedFV.Version) needNow := Need(globalFV, true, file.Version) if needBefore { if keyBuf, oldGlobal, err = t.getGlobalFromFileVersion(keyBuf, folder, name, true, oldGlobalFV); err != nil { @@ -677,7 +674,8 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi } } if needNow { - if keyBuf, global, err = t.updateGlobalGetGlobal(keyBuf, folder, name, file, globalFV); err != nil { + keyBuf, global, err = t.getGlobalFromFileVersion(keyBuf, folder, name, true, globalFV) + if err != nil { return nil, false, err } gotGlobal = true @@ -711,8 +709,11 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi // Add the new global to the global size counter if !gotGlobal { - if keyBuf, global, err = t.updateGlobalGetGlobal(keyBuf, folder, name, file, globalFV); err != nil { - return nil, false, err + if globalFV.Version.Equal(file.Version) { + // The inserted file is the global file + global = file + } else { + keyBuf, global, err = t.getGlobalFromFileVersion(keyBuf, folder, name, true, globalFV) } gotGlobal = true } @@ -721,10 +722,7 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi // check for local (if not already done before) if !bytes.Equal(device, protocol.LocalDeviceID[:]) { localFV, haveLocal := fl.Get(protocol.LocalDeviceID[:]) - needBefore := false - if haveOldGlobal { - needBefore = Need(oldGlobalFV, haveLocal, localFV.Version) - } + needBefore := haveOldGlobal && Need(oldGlobalFV, haveLocal, localFV.Version) needNow := Need(globalFV, haveLocal, localFV.Version) if needBefore { meta.removeNeeded(protocol.LocalDeviceID, oldGlobal) @@ -761,14 +759,6 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi return keyBuf, true, nil } -func (t readWriteTransaction) updateGlobalGetGlobal(keyBuf, folder, name []byte, file protocol.FileInfo, fv FileVersion) ([]byte, protocol.FileIntf, error) { - if fv.Version.Equal(file.Version) { - // Inserted a new newest version - return keyBuf, file, nil - } - return t.getGlobalFromFileVersion(keyBuf, folder, name, true, fv) -} - func (t readWriteTransaction) updateLocalNeed(keyBuf, folder, name []byte, add bool) ([]byte, error) { var err error keyBuf, err = t.keyer.GenerateNeedFileKey(keyBuf, folder, name) @@ -824,6 +814,7 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device, file } oldGlobalFV, haveOldGlobal := fl.GetGlobal() + oldGlobalFV = oldGlobalFV.copy() if !haveOldGlobal { // Shouldn't ever happen, but doesn't hurt to handle.