lib/db: Don't count invalid items to global state (ref #6850) (#6863)

lib/db/meta.go

@@ -173,22 +173,27 @@ func (m *metadataTracker) addFile(dev protocol.DeviceID, f protocol.FileIntf) {
 	m.mut.Lock()
 	defer m.mut.Unlock()
 
-	m.dirty = true
-
 	m.updateSeqLocked(dev, f)
 
-	if f.IsInvalid() && f.FileLocalFlags() == 0 {
-		// This is a remote invalid file; it does not count.
+	m.updateFileLocked(dev, f, m.addFileLocked)
+}
+
+func (m *metadataTracker) updateFileLocked(dev protocol.DeviceID, f protocol.FileIntf, fn func(protocol.DeviceID, uint32, protocol.FileIntf)) {
+	m.dirty = true
+
+	if f.IsInvalid() && (f.FileLocalFlags() == 0 || dev == protocol.GlobalDeviceID) {
+		// This is a remote invalid file or concern the global state.
+		// In either case invalid files are not accounted.
 		return
 	}
 
 	if flags := f.FileLocalFlags(); flags == 0 {
 		// Account regular files in the zero-flags bucket.
-		m.addFileLocked(dev, 0, f)
+		fn(dev, 0, f)
 	} else {
 		// Account in flag specific buckets.
 		eachFlagBit(flags, func(flag uint32) {
-			m.addFileLocked(dev, flag, f)
+			fn(dev, flag, f)
 		})
 	}
 }
@@ -250,25 +255,10 @@ func (m *metadataTracker) addFileLocked(dev protocol.DeviceID, flag uint32, f pr
 
 // removeFile removes a file from the counts
 func (m *metadataTracker) removeFile(dev protocol.DeviceID, f protocol.FileIntf) {
-	if f.IsInvalid() && f.FileLocalFlags() == 0 {
-		// This is a remote invalid file; it does not count.
-		return
-	}
-
 	m.mut.Lock()
 	defer m.mut.Unlock()
 
-	m.dirty = true
-
-	if flags := f.FileLocalFlags(); flags == 0 {
-		// Remove regular files from the zero-flags bucket
-		m.removeFileLocked(dev, 0, f)
-	} else {
-		// Remove from flag specific buckets.
-		eachFlagBit(flags, func(flag uint32) {
-			m.removeFileLocked(dev, flag, f)
-		})
-	}
+	m.updateFileLocked(dev, f, m.removeFileLocked)
 }
 
 // removeNeeded removes a file from the needed counts

lib/db/set.go

@@ -288,9 +288,7 @@ func (s *Snapshot) ReceiveOnlyChangedSize() Counts {
 }
 
 func (s *Snapshot) GlobalSize() Counts {
-	global := s.meta.Counts(protocol.GlobalDeviceID, 0)
-	recvOnlyChanged := s.meta.Counts(protocol.GlobalDeviceID, protocol.FlagLocalReceiveOnly)
-	return global.Add(recvOnlyChanged)
+	return s.meta.Counts(protocol.GlobalDeviceID, 0)
 }
 
 func (s *Snapshot) NeedSize(device protocol.DeviceID) Counts {

lib/db/set_test.go

@@ -1720,8 +1720,8 @@ func TestIgnoreLocalChanged(t *testing.T) {
 	}
 	s.Update(protocol.LocalDeviceID, files)
 
-	if c := globalSize(s).Files; c != 1 {
-		t.Error("Expected one global file, got", c)
+	if c := globalSize(s).Files; c != 0 {
+		t.Error("Expected no global file, got", c)
 	}
 	if c := localSize(s).Files; c != 1 {
 		t.Error("Expected one local file, got", c)

lib/db/transactions.go

@@ -662,11 +662,13 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi
 		}
 	}
 
-	// Update global size counter.
-	// It's done regardless of if the global changed, as two files might
-	// both be invalid, but for different reasons i.e. have different flags
-	// (e.g. ignored vs receive only).
-	// https://github.com/syncthing/syncthing/issues/6850
+	// Update global size counter if necessary
+
+	if !globalChanged {
+		// Neither the global state nor the needs of any devices, except
+		// the one updated, changed.
+		return keyBuf, true, nil
+	}
 
 	// Remove the old global from the global size counter
 	if haveOldGlobal {
@@ -689,12 +691,6 @@ func (t readWriteTransaction) updateGlobal(gk, keyBuf, folder, device []byte, fi
 	}
 	meta.addFile(protocol.GlobalDeviceID, global)
 
-	if !globalChanged {
-		// Neither the global state nor the needs of any devices, except
-		// the one updated, changed.
-		return keyBuf, true, nil
-	}
-
 	// check for local (if not already done before)
 	if !bytes.Equal(device, protocol.LocalDeviceID[:]) {
 		localFV, haveLocal := fl.Get(protocol.LocalDeviceID[:])
@@ -842,16 +838,11 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device, file
 		return keyBuf, nil
 	}
 
-	keyBuf, err = t.keyer.GenerateDeviceFileKey(keyBuf, folder, device, file)
+	var f protocol.FileIntf
+	keyBuf, f, err = t.getGlobalFromFileVersion(keyBuf, folder, file, true, oldGlobalFV)
 	if err != nil {
 		return nil, err
 	}
-	f, ok, err := t.getFileTrunc(keyBuf, true)
-	if err != nil {
-		return nil, err
-	} else if !ok {
-		return nil, errEntryFromGlobalMissing
-	}
 	meta.removeFile(protocol.GlobalDeviceID, f)
 
 	// Remove potential device needs

lib/model/folder_recvonly_test.go

@@ -46,11 +46,7 @@ func TestRecvOnlyRevertDeletes(t *testing.T) {
 	m.Index(device1, "ro", knownFiles)
 	f.updateLocalsFromScanning(knownFiles)
 
-	m.fmut.RLock()
-	snap := m.folderFiles["ro"].Snapshot()
-	m.fmut.RUnlock()
-	size := snap.GlobalSize()
-	snap.Release()
+	size := globalSize(t, m, "ro")
 	if size.Files != 1 || size.Directories != 1 {
 		t.Fatalf("Global: expected 1 file and 1 directory: %+v", size)
 	}
@@ -59,10 +55,10 @@ func TestRecvOnlyRevertDeletes(t *testing.T) {
 
 	must(t, m.ScanFolder("ro"))
 
-	// We should now have two files and two directories.
+	// We should now have two files and two directories, with global state unchanged.
 
 	size = globalSize(t, m, "ro")
-	if size.Files != 2 || size.Directories != 2 {
+	if size.Files != 1 || size.Directories != 1 {
 		t.Fatalf("Global: expected 2 files and 2 directories: %+v", size)
 	}
 	size = localSize(t, m, "ro")