diff --git a/lib/model/folder.go b/lib/model/folder.go index 70fe941f2..71797316b 100644 --- a/lib/model/folder.go +++ b/lib/model/folder.go @@ -556,12 +556,14 @@ type scanBatch struct { f *folder updateBatch *FileInfoBatch toRemove []string + deleted map[string]struct{} } func (f *folder) newScanBatch() *scanBatch { b := &scanBatch{ f: f, toRemove: make([]string, 0, maxToRemove), + deleted: make(map[string]struct{}), } b.updateBatch = NewFileInfoBatch(func(fs []protocol.FileInfo) error { if err := b.f.getHealthErrorWithoutIgnores(); err != nil { @@ -569,6 +571,7 @@ func (f *folder) newScanBatch() *scanBatch { return err } b.f.updateLocalsFromScanning(fs) + clear(b.deleted) return nil }) return b @@ -604,6 +607,15 @@ func (b *scanBatch) FlushIfFull() error { return b.updateBatch.FlushIfFull() } +func (b *scanBatch) markDeleted(name string) { + b.deleted[name] = struct{}{} +} + +func (b *scanBatch) hasDeleted(name string) bool { + _, ok := b.deleted[name] + return ok +} + // Update adds the fileinfo to the batch for updating, and does a few checks. // It returns false if the checks result in the file not going to be updated or removed. func (b *scanBatch) Update(fi protocol.FileInfo) (bool, error) { @@ -680,7 +692,6 @@ func (f *folder) scanSubdirsChangedAndNew(ctx context.Context, subDirs []string, fchan = scanner.Walk(scanCtx, scanConfig) } - alreadyUsedOrExisting := make(map[string]struct{}) for res := range fchan { if res.Err != nil { f.newScanError(res.Path, res.Err) @@ -705,11 +716,12 @@ func (f *folder) scanSubdirsChangedAndNew(ctx context.Context, subDirs []string, switch f.Type { case config.FolderTypeReceiveOnly, config.FolderTypeReceiveEncrypted: default: - if nf, ok := f.findRename(ctx, res.File, alreadyUsedOrExisting); ok { + if nf, ok := f.findRename(ctx, res.File, batch); ok { if ok, err := batch.Update(nf); err != nil { return 0, err } else if ok { changes++ + batch.markDeleted(nf.Name) } } } @@ -878,7 +890,7 @@ outer: return changes, nil } -func (f *folder) findRename(ctx context.Context, file protocol.FileInfo, alreadyUsedOrExisting map[string]struct{}) (protocol.FileInfo, bool) { +func (f *folder) findRename(ctx context.Context, file protocol.FileInfo, batch *scanBatch) (protocol.FileInfo, bool) { if len(file.Blocks) == 0 || file.Size == 0 { return protocol.FileInfo{}, false } @@ -899,11 +911,10 @@ loop: } if fi.Name == file.Name { - alreadyUsedOrExisting[fi.Name] = struct{}{} continue } - if _, ok := alreadyUsedOrExisting[fi.Name]; ok { + if batch.hasDeleted(fi.Name) { continue } @@ -922,8 +933,6 @@ loop: continue } - alreadyUsedOrExisting[fi.Name] = struct{}{} - if !osutil.IsDeleted(f.mtimefs, fi.Name) { continue } diff --git a/lib/model/model_test.go b/lib/model/model_test.go index e132f08ec..ba29c8dab 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -3344,6 +3344,87 @@ func TestRenameSameFile(t *testing.T) { } } +// TestRenameBatchFlush verifies that rename detection works correctly when +// a batch flush happens mid-scan. With enough files to exceed +// MaxBatchSizeFiles the scan batch flushes at least once, clearing the +// in-memory deleted-tracking map. After the flush the database itself +// guards against reusing an already-consumed rename source. The fake +// filesystem iterates in non-deterministic order, so the two rename pairs +// may or may not straddle a flush boundary on any given run; with +// 2*MaxBatchSizeFiles filler files the cross-flush case is hit roughly half +// the time. +func TestRenameBatchFlush(t *testing.T) { + wcfg, fcfg := newDefaultCfgWrapper(t) + m := setupModel(t, wcfg) + defer cleanupModel(m) + + ffs := fcfg.Filesystem() + + // Two source files with identical content so they share a blocks hash. + content := []byte("shared-content-for-rename-detection") + writeFile(t, ffs, "src-a", content) + writeFile(t, ffs, "src-b", content) + + m.ScanFolders() + + // Delete the sources and create two new destinations with the same + // content plus enough filler files to force at least one batch flush. + must(t, ffs.Remove("src-a")) + must(t, ffs.Remove("src-b")) + writeFile(t, ffs, "dst-a", content) + writeFile(t, ffs, "dst-b", content) + for i := range MaxBatchSizeFiles * 2 { + writeFile(t, ffs, fmt.Sprintf("filler-%04d", i), []byte(fmt.Sprintf("filler-%04d", i))) + } + + m.ScanFolders() + + // Collect all files keyed by name. + files := make(map[string]protocol.FileInfo) + it, errFn := m.LocalFilesSequenced("default", protocol.LocalDeviceID, 0) + for fi := range it { + files[fi.FileName()] = fi + } + if err := errFn(); err != nil { + t.Fatal(err) + } + + for _, name := range []string{"src-a", "src-b"} { + fi, ok := files[name] + if !ok { + t.Fatalf("%q not found in DB", name) + } + if !fi.IsDeleted() { + t.Fatalf("%q should be deleted", name) + } + } + + for _, name := range []string{"dst-a", "dst-b"} { + fi, ok := files[name] + if !ok { + t.Fatalf("%q not found in DB", name) + } + if fi.IsDeleted() { + t.Fatalf("%q should not be deleted", name) + } + } + + // When rename detection works the deleted source is appended to the + // batch right after its destination, so their sequences are adjacent + // (src.seq == dst.seq + 1). If detection failed the sources would + // only be deleted in a later scan phase with much higher sequences. + dstSeqs := map[int64]bool{ + files["dst-a"].SequenceNo(): true, + files["dst-b"].SequenceNo(): true, + } + for _, name := range []string{"src-a", "src-b"} { + srcSeq := files[name].SequenceNo() + if !dstSeqs[srcSeq-1] { + t.Errorf("deleted %q (seq %d) not adjacent to a destination file; rename was not detected", name, srcSeq) + } + } +} + func TestBlockListMap(t *testing.T) { wcfg, fcfg := newDefaultCfgWrapper(t) m := setupModel(t, wcfg)