From 850dd4cd25eb7ba862567a8140e69359f21529a8 Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Thu, 30 Jul 2020 13:49:14 +0200 Subject: [PATCH] lib/db: Don't count invalid items to global state (ref #6850) (#6863) --- lib/db/meta.go | 34 +++++++++++-------------------- lib/db/set.go | 4 +--- lib/db/set_test.go | 4 ++-- lib/db/transactions.go | 27 ++++++++---------------- lib/model/folder_recvonly_test.go | 10 +++------ 5 files changed, 27 insertions(+), 52 deletions(-) diff --git a/lib/db/meta.go b/lib/db/meta.go index a5aebe165..28b440c00 100644 --- a/lib/db/meta.go +++ b/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 diff --git a/lib/db/set.go b/lib/db/set.go index 543f14606..04952fa6f 100644 --- a/lib/db/set.go +++ b/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 { diff --git a/lib/db/set_test.go b/lib/db/set_test.go index 05fc4d1f9..be82eb9ec 100644 --- a/lib/db/set_test.go +++ b/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) diff --git a/lib/db/transactions.go b/lib/db/transactions.go index 05ce2fa34..9c3f2d2ee 100644 --- a/lib/db/transactions.go +++ b/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 diff --git a/lib/model/folder_recvonly_test.go b/lib/model/folder_recvonly_test.go index 69523f080..62fc34c51 100644 --- a/lib/model/folder_recvonly_test.go +++ b/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")