lib/db: Fix and improve removing entries from global (ref #6501) (#7336)

lib/db/db_test.go

@@ -951,6 +951,49 @@ func TestDuplicateNeedCount(t *testing.T) {
 	}
 }
 
+func TestNeedAfterDropGlobal(t *testing.T) {
+	db := newLowlevelMemory(t)
+	defer db.Close()
+
+	folder := "test"
+	testFs := fs.NewFilesystem(fs.FilesystemTypeFake, "")
+
+	fs := newFileSet(t, folder, testFs, db)
+
+	// Initial:
+	// Three devices and a file "test": local has Version 1, remoteDevice0
+	// Version 2 and remoteDevice2 doesn't have it.
+	// All of them have "bar", just so the db knows about remoteDevice2.
+	files := []protocol.FileInfo{
+		{Name: "foo", Version: protocol.Vector{}.Update(myID), Sequence: 1},
+		{Name: "bar", Version: protocol.Vector{}.Update(myID), Sequence: 2},
+	}
+	fs.Update(protocol.LocalDeviceID, files)
+	files[0].Version = files[0].Version.Update(myID)
+	fs.Update(remoteDevice0, files)
+	fs.Update(remoteDevice1, files[1:])
+
+	// remoteDevice1 needs one file: test
+	snap := fs.Snapshot()
+	c := snap.NeedSize(remoteDevice1)
+	if c.Files != 1 {
+		t.Errorf("Expected 1 needed files initially, got %v", c.Files)
+	}
+	snap.Release()
+
+	// Drop remoteDevice0, i.e. remove all their files from db.
+	// That changes the global file, which is now what local has.
+	fs.Drop(remoteDevice0)
+
+	// remoteDevice1 still needs test.
+	snap = fs.Snapshot()
+	c = snap.NeedSize(remoteDevice1)
+	if c.Files != 1 {
+		t.Errorf("Expected still 1 needed files, got %v", c.Files)
+	}
+	snap.Release()
+}
+
 func numBlockLists(db *Lowlevel) (int, error) {
 	it, err := db.Backend.NewPrefixIterator([]byte{KeyTypeBlockList})
 	if err != nil {

lib/db/transactions.go

@@ -817,11 +817,11 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device, file
 	}
 
 	var global protocol.FileIntf
-	var gotGlobal, ok bool
+	var gotGlobal bool
 
-	globalFV, ok := fl.GetGlobal()
+	globalFV, haveGlobal := fl.GetGlobal()
 	// Add potential needs of the removed device
-	if ok && !globalFV.IsInvalid() && Need(globalFV, false, protocol.Vector{}) && !Need(oldGlobalFV, haveRemoved, removedFV.Version) {
+	if haveGlobal && !globalFV.IsInvalid() && Need(globalFV, false, protocol.Vector{}) && !Need(oldGlobalFV, haveRemoved, removedFV.Version) {
 		keyBuf, global, _, err = t.getGlobalFromVersionList(keyBuf, folder, file, true, fl)
 		if err != nil {
 			return nil, err
@@ -844,16 +844,23 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device, file
 		return keyBuf, nil
 	}
 
-	var f protocol.FileIntf
+	var oldGlobal protocol.FileIntf
-	keyBuf, f, err = t.getGlobalFromFileVersion(keyBuf, folder, file, true, oldGlobalFV)
+	keyBuf, oldGlobal, err = t.getGlobalFromFileVersion(keyBuf, folder, file, true, oldGlobalFV)
 	if err != nil {
 		return nil, err
 	}
-	meta.removeFile(protocol.GlobalDeviceID, f)
+	meta.removeFile(protocol.GlobalDeviceID, oldGlobal)
 
 	// Remove potential device needs
-	if fv, have := fl.Get(protocol.LocalDeviceID[:]); Need(removedFV, have, fv.Version) {
+	shouldRemoveNeed := func(dev protocol.DeviceID) bool {
-		meta.removeNeeded(protocol.LocalDeviceID, f)
+		fv, have := fl.Get(dev[:])
+		if !Need(oldGlobalFV, have, fv.Version) {
+			return false // Didn't need it before
+		}
+		return !haveGlobal || !Need(globalFV, have, fv.Version)
+	}
+	if shouldRemoveNeed(protocol.LocalDeviceID) {
+		meta.removeNeeded(protocol.LocalDeviceID, oldGlobal)
 		if keyBuf, err = t.updateLocalNeed(keyBuf, folder, file, false); err != nil {
 			return nil, err
 		}
@@ -862,8 +869,8 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device, file
 		if bytes.Equal(dev[:], device) { // Was the previous global
 			continue
 		}
-		if fv, have := fl.Get(dev[:]); Need(removedFV, have, fv.Version) {
+		if shouldRemoveNeed(dev) {
-			meta.removeNeeded(dev, f)
+			meta.removeNeeded(dev, oldGlobal)
 		}
 	}