From c2784d76e4d9b5b8ebf3f7af1b9b9ff77b9fa859 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Mon, 18 Jun 2018 08:23:40 +0200 Subject: [PATCH] lib/db: Remove updated invalid files from need bucket (fixes #5007) (#5008) --- lib/db/leveldb_dbinstance_updateschema.go | 21 +++++++- lib/db/leveldb_transactions.go | 8 ++- lib/db/set_test.go | 65 +++++++++++++++++++++++ lib/model/folder_sendrecv.go | 3 +- 4 files changed, 93 insertions(+), 4 deletions(-) diff --git a/lib/db/leveldb_dbinstance_updateschema.go b/lib/db/leveldb_dbinstance_updateschema.go index f65df5982..5ef2a7998 100644 --- a/lib/db/leveldb_dbinstance_updateschema.go +++ b/lib/db/leveldb_dbinstance_updateschema.go @@ -13,7 +13,7 @@ import ( "github.com/syndtr/goleveldb/leveldb/util" ) -const dbVersion = 3 +const dbVersion = 4 func (db *Instance) updateSchema() { miscDB := NewNamespacedKV(db, string(KeyTypeMiscData)) @@ -34,6 +34,10 @@ func (db *Instance) updateSchema() { if prevVersion < 3 { db.updateSchema2to3() } + // This update fixes a problem that only exists in dbVersion 3. + if prevVersion == 3 { + db.updateSchema3to4() + } miscDB.PutInt64("dbVersion", dbVersion) } @@ -153,3 +157,18 @@ func (db *Instance) updateSchema2to3() { }) } } + +// updateSchema3to4 resets the need bucket due a bug existing in dbVersion 3 / +// v0.14.49-rc.1 +// https://github.com/syncthing/syncthing/issues/5007 +func (db *Instance) updateSchema3to4() { + t := db.newReadWriteTransaction() + var nk []byte + for _, folderStr := range db.ListFolders() { + nk = db.needKeyInto(nk, []byte(folderStr), nil) + t.deleteKeyPrefix(nk[:keyPrefixLen+keyFolderLen]) + } + t.close() + + db.updateSchema2to3() +} diff --git a/lib/db/leveldb_transactions.go b/lib/db/leveldb_transactions.go index f9c949175..2a5b4727e 100644 --- a/lib/db/leveldb_transactions.go +++ b/lib/db/leveldb_transactions.go @@ -97,14 +97,18 @@ func (t readWriteTransaction) updateGlobal(gk, folder, device []byte, file proto return false } + name := []byte(file.Name) + if removedAt != 0 && insertedAt != 0 { + if bytes.Equal(device, protocol.LocalDeviceID[:]) && file.Version.Equal(fl.Versions[0].Version) { + l.Debugf("local need delete; folder=%q, name=%q", folder, name) + t.Delete(t.db.needKey(folder, name)) + } l.Debugf(`new global for "%v" after update: %v`, file.Name, fl) t.Put(gk, mustMarshal(&fl)) return true } - name := []byte(file.Name) - // Remove the old global from the global size counter var oldGlobalFV FileVersion if removedAt == 0 { diff --git a/lib/db/set_test.go b/lib/db/set_test.go index 6c5afb650..a8b61e667 100644 --- a/lib/db/set_test.go +++ b/lib/db/set_test.go @@ -997,6 +997,71 @@ func TestMoveGlobalBack(t *testing.T) { } } +// TestIssue5007 checks, that updating the local device with an invalid file +// info with the newest version does indeed remove that file from the list of +// needed files. +// https://github.com/syncthing/syncthing/issues/5007 +func TestIssue5007(t *testing.T) { + ldb := db.OpenMemory() + + folder := "test" + file := "foo" + s := db.NewFileSet(folder, fs.NewFilesystem(fs.FilesystemTypeBasic, "."), ldb) + + fs := fileList{{Name: file, Version: protocol.Vector{Counters: []protocol.Counter{{ID: myID, Value: 1}}}}} + + s.Update(remoteDevice0, fs) + + if need := needList(s, protocol.LocalDeviceID); len(need) != 1 { + t.Fatal("Expected 1 local need, got", need) + } else if !need[0].IsEquivalent(fs[0], false, false) { + t.Fatalf("Local need incorrect;\n A: %v !=\n E: %v", need[0], fs[0]) + } + + fs[0].Invalid = true + s.Update(protocol.LocalDeviceID, fs) + + if need := needList(s, protocol.LocalDeviceID); len(need) != 0 { + t.Fatal("Expected no local need, got", need) + } +} + +// TestNeedDeleted checks that a file that doesn't exist locally isn't needed +// when the global file is deleted. +func TestNeedDeleted(t *testing.T) { + ldb := db.OpenMemory() + + folder := "test" + file := "foo" + s := db.NewFileSet(folder, fs.NewFilesystem(fs.FilesystemTypeBasic, "."), ldb) + + fs := fileList{{Name: file, Version: protocol.Vector{Counters: []protocol.Counter{{ID: myID, Value: 1}}}, Deleted: true}} + + s.Update(remoteDevice0, fs) + + if need := needList(s, protocol.LocalDeviceID); len(need) != 0 { + t.Fatal("Expected no local need, got", need) + } + + fs[0].Deleted = false + fs[0].Version = fs[0].Version.Update(remoteDevice0.Short()) + s.Update(remoteDevice0, fs) + + if need := needList(s, protocol.LocalDeviceID); len(need) != 1 { + t.Fatal("Expected 1 local need, got", need) + } else if !need[0].IsEquivalent(fs[0], false, false) { + t.Fatalf("Local need incorrect;\n A: %v !=\n E: %v", need[0], fs[0]) + } + + fs[0].Deleted = true + fs[0].Version = fs[0].Version.Update(remoteDevice0.Short()) + s.Update(remoteDevice0, fs) + + if need := needList(s, protocol.LocalDeviceID); len(need) != 0 { + t.Fatal("Expected no local need, got", need) + } +} + func replace(fs *db.FileSet, device protocol.DeviceID, files []protocol.FileInfo) { fs.Drop(device) fs.Update(device, files) diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go index e62aa66db..2ee172685 100644 --- a/lib/model/folder_sendrecv.go +++ b/lib/model/folder_sendrecv.go @@ -272,11 +272,11 @@ func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher, ignoresChan file.Invalidate(f.shortID) l.Debugln(f, "Handling ignored file", file) dbUpdateChan <- dbUpdateJob{file, dbUpdateInvalidate} + changed++ case runtime.GOOS == "windows" && fs.WindowsInvalidFilename(file.Name): f.newError("need", file.Name, fs.ErrInvalidFilename) changed++ - return true case file.IsDeleted(): processDirectly = append(processDirectly, file) @@ -300,6 +300,7 @@ func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher, ignoresChan file.Invalidate(f.shortID) l.Debugln(f, "Invalidating symlink (unsupported)", file.Name) dbUpdateChan <- dbUpdateJob{file, dbUpdateInvalidate} + changed++ default: // Directories, symlinks