lib/db: Update local need on device removal (fixes #5294) (#5295)

lib/db/schemaupdater.go

@@ -22,9 +22,10 @@ import (
 //   4: v0.14.49
 //   5: v0.14.49
 //   6: v0.14.50
+//   7: v0.14.53
 const (
-	dbVersion             = 6
-	dbMinSyncthingVersion = "v0.14.50"
+	dbVersion             = 7
+	dbMinSyncthingVersion = "v0.14.53"
 )
 
 type databaseDowngradeError struct {
@@ -79,6 +80,9 @@ func (db *schemaUpdater) updateSchema() error {
 	if prevVersion < 6 {
 		db.updateSchema5to6()
 	}
+	if prevVersion < 7 {
+		db.updateSchema6to7()
+	}
 
 	miscDB.PutInt64("dbVersion", dbVersion)
 	miscDB.PutString("dbMinSyncthingVersion", dbMinSyncthingVersion)
@@ -259,3 +263,39 @@ func (db *schemaUpdater) updateSchema5to6() {
 		})
 	}
 }
+
+// updateSchema6to7 checks whether all currently locally needed files are really
+// needed and removes them if not.
+func (db *schemaUpdater) updateSchema6to7() {
+	t := db.newReadWriteTransaction()
+	defer t.close()
+
+	var gk []byte
+	var nk []byte
+
+	for _, folderStr := range db.ListFolders() {
+		folder := []byte(folderStr)
+		db.withNeedLocal(folder, false, func(f FileIntf) bool {
+			name := []byte(f.FileName())
+			global := f.(protocol.FileInfo)
+			gk = db.keyer.GenerateGlobalVersionKey(gk, folder, name)
+			svl, err := t.Get(gk, nil)
+			if err != nil {
+				// If there is no global list, we hardly need it.
+				t.Delete(t.db.keyer.GenerateNeedFileKey(nk, folder, name))
+				return true
+			}
+			var fl VersionList
+			err = fl.Unmarshal(svl)
+			if err != nil {
+				// This can't happen, but it's ignored everywhere else too,
+				// so lets not act on it.
+				return true
+			}
+			if localFV, haveLocalFV := fl.Get(protocol.LocalDeviceID[:]); !need(global, haveLocalFV, localFV.Version) {
+				t.Delete(t.db.keyer.GenerateNeedFileKey(nk, folder, name))
+			}
+			return true
+		})
+	}
+}

lib/db/set_test.go

@@ -1309,6 +1309,31 @@ func TestNeedWithNewerInvalid(t *testing.T) {
 	}
 }
 
+func TestNeedAfterDeviceRemove(t *testing.T) {
+	ldb := db.OpenMemory()
+
+	file := "foo"
+	s := db.NewFileSet("test", fs.NewFilesystem(fs.FilesystemTypeBasic, "."), ldb)
+
+	fs := fileList{{Name: file, Version: protocol.Vector{Counters: []protocol.Counter{{ID: myID, Value: 1}}}}}
+
+	s.Update(protocol.LocalDeviceID, fs)
+
+	fs[0].Version = fs[0].Version.Update(myID)
+
+	s.Update(remoteDevice0, fs)
+
+	if need := needList(s, protocol.LocalDeviceID); len(need) != 1 {
+		t.Fatal("Expected one local need, got", need)
+	}
+
+	s.Drop(remoteDevice0)
+
+	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/db/structs.go

@@ -161,15 +161,7 @@ func (vl VersionList) String() string {
 // VersionList, a potentially removed old FileVersion and its index, as well as
 // the index where the new FileVersion was inserted.
 func (vl VersionList) update(folder, device []byte, file protocol.FileInfo, db *instance) (_ VersionList, removedFV FileVersion, removedAt int, insertedAt int) {
-	removedAt, insertedAt = -1, -1
-	for i, v := range vl.Versions {
-		if bytes.Equal(v.Device, device) {
-			removedAt = i
-			removedFV = v
-			vl.Versions = append(vl.Versions[:i], vl.Versions[i+1:]...)
-			break
-		}
-	}
+	vl, removedFV, removedAt = vl.pop(device)
 
 	nv := FileVersion{
 		Device:  device,
@@ -222,6 +214,20 @@ func (vl VersionList) insertAt(i int, v FileVersion) VersionList {
 	return vl
 }
 
+// pop returns the VersionList without the entry for the given device, as well
+// as the removed FileVersion and the position, where that FileVersion was.
+// If there is no FileVersion for the given device, the position is -1.
+func (vl VersionList) pop(device []byte) (VersionList, FileVersion, int) {
+	removedAt := -1
+	for i, v := range vl.Versions {
+		if bytes.Equal(v.Device, device) {
+			vl.Versions = append(vl.Versions[:i], vl.Versions[i+1:]...)
+			return vl, v, i
+		}
+	}
+	return vl, FileVersion{}, removedAt
+}
+
 func (vl VersionList) Get(device []byte) (FileVersion, bool) {
 	for _, v := range vl.Versions {
 		if bytes.Equal(v.Device, device) {

lib/db/transactions.go

@@ -7,8 +7,6 @@
 package db
 
 import (
-	"bytes"
-
 	"github.com/syncthing/syncthing/lib/protocol"
 	"github.com/syndtr/goleveldb/leveldb"
 	"github.com/syndtr/goleveldb/leveldb/util"
@@ -100,29 +98,18 @@ func (t readWriteTransaction) updateGlobal(gk, folder, device []byte, file proto
 
 	name := []byte(file.Name)
 
-	var newGlobal protocol.FileInfo
+	var global protocol.FileInfo
 	if insertedAt == 0 {
 		// Inserted a new newest version
-		newGlobal = file
+		global = file
 	} else if new, ok := t.getFile(folder, fl.Versions[0].Device, name); ok {
-		// The previous second version is now the first
-		newGlobal = new
+		global = new
 	} else {
 		panic("This file must exist in the db")
 	}
 
 	// Fixup the list of files we need.
-	nk := t.db.keyer.GenerateNeedFileKey(nil, folder, name)
-	hasNeeded, _ := t.db.Has(nk, nil)
-	if localFV, haveLocalFV := fl.Get(protocol.LocalDeviceID[:]); need(newGlobal, haveLocalFV, localFV.Version) {
-		if !hasNeeded {
-			l.Debugf("local need insert; folder=%q, name=%q", folder, name)
-			t.Put(nk, nil)
-		}
-	} else if hasNeeded {
-		l.Debugf("local need delete; folder=%q, name=%q", folder, name)
-		t.Delete(nk)
-	}
+	t.updateLocalNeed(folder, name, fl, global)
 
 	if removedAt != 0 && insertedAt != 0 {
 		l.Debugf(`new global for "%v" after update: %v`, file.Name, fl)
@@ -145,7 +132,7 @@ func (t readWriteTransaction) updateGlobal(gk, folder, device []byte, file proto
 	}
 
 	// Add the new global to the global size counter
-	meta.addFile(protocol.GlobalDeviceID, newGlobal)
+	meta.addFile(protocol.GlobalDeviceID, global)
 
 	l.Debugf(`new global for "%v" after update: %v`, file.Name, fl)
 	t.Put(gk, mustMarshal(&fl))
@@ -153,6 +140,23 @@ func (t readWriteTransaction) updateGlobal(gk, folder, device []byte, file proto
 	return true
 }
 
+// updateLocalNeeds checks whether the given file is still needed on the local
+// device according to the version list and global FileInfo given and updates
+// the db accordingly.
+func (t readWriteTransaction) updateLocalNeed(folder, name []byte, fl VersionList, global protocol.FileInfo) {
+	nk := t.db.keyer.GenerateNeedFileKey(nil, folder, name)
+	hasNeeded, _ := t.db.Has(nk, nil)
+	if localFV, haveLocalFV := fl.Get(protocol.LocalDeviceID[:]); need(global, haveLocalFV, localFV.Version) {
+		if !hasNeeded {
+			l.Debugf("local need insert; folder=%q, name=%q", folder, name)
+			t.Put(nk, nil)
+		}
+	} else if hasNeeded {
+		l.Debugf("local need delete; folder=%q, name=%q", folder, name)
+		t.Delete(nk)
+	}
+}
+
 func need(global FileIntf, haveLocal bool, localVersion protocol.Vector) bool {
 	// We never need an invalid file.
 	if global.IsInvalid() {
@@ -189,36 +193,37 @@ func (t readWriteTransaction) removeFromGlobal(gk, folder, device, file []byte,
 		return
 	}
 
-	removed := false
-	for i := range fl.Versions {
-		if bytes.Equal(fl.Versions[i].Device, device) {
-			if i == 0 && meta != nil {
-				f, ok := t.getFile(folder, device, file)
-				if !ok {
-					// didn't exist anyway, apparently
-					continue
-				}
-				meta.removeFile(protocol.GlobalDeviceID, f)
-				removed = true
-			}
-			fl.Versions = append(fl.Versions[:i], fl.Versions[i+1:]...)
-			break
+	fl, _, removedAt := fl.pop(device)
+	if removedAt == -1 {
+		// There is no version for the given device
+		return
+	}
+
+	if removedAt == 0 {
+		// A failure to get the file here is surprising and our
+		// global size data will be incorrect until a restart...
+		if f, ok := t.getFile(folder, device, file); ok {
+			meta.removeFile(protocol.GlobalDeviceID, f)
 		}
 	}
 
 	if len(fl.Versions) == 0 {
+		t.Delete(t.db.keyer.GenerateNeedFileKey(nil, folder, file))
 		t.Delete(gk)
 		return
 	}
+
+	if removedAt == 0 {
+		global, ok := t.getFile(folder, fl.Versions[0].Device, file)
+		if !ok {
+			panic("This file must exist in the db")
+		}
+		t.updateLocalNeed(folder, file, fl, global)
+		meta.addFile(protocol.GlobalDeviceID, global)
+	}
+
 	l.Debugf("new global after remove: %v", fl)
 	t.Put(gk, mustMarshal(&fl))
-	if removed {
-		if f, ok := t.getFile(folder, fl.Versions[0].Device, file); ok {
-			// A failure to get the file here is surprising and our
-			// global size data will be incorrect until a restart...
-			meta.addFile(protocol.GlobalDeviceID, f)
-		}
-	}
 }
 
 func (t readWriteTransaction) deleteKeyPrefix(prefix []byte) {