lib/db: Remove updated invalid files from need bucket (fixes #5007) (#5008)

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()
+}

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 {

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)

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