From 8fea354b7416f168fa4f3f284ace08e025220b3f Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Fri, 18 Mar 2016 12:48:24 +0100 Subject: [PATCH] lib/model: Properly handle deleting multiple files when doing scans with subs (fixes #2851) --- lib/db/leveldb_dbinstance.go | 7 +-- lib/db/set.go | 8 ++- lib/model/model.go | 113 ++++++++++++++++------------------- 3 files changed, 61 insertions(+), 67 deletions(-) diff --git a/lib/db/leveldb_dbinstance.go b/lib/db/leveldb_dbinstance.go index 9814fdd74..b333f7f0a 100644 --- a/lib/db/leveldb_dbinstance.go +++ b/lib/db/leveldb_dbinstance.go @@ -236,14 +236,11 @@ func (db *Instance) updateFiles(folder, device []byte, fs []protocol.FileInfo, l return maxLocalVer } -func (db *Instance) withHave(folder, device []byte, truncate bool, fn Iterator) { - start := db.deviceKey(folder, device, nil) // before all folder/device files - limit := db.deviceKey(folder, device, []byte{0xff, 0xff, 0xff, 0xff}) // after all folder/device files - +func (db *Instance) withHave(folder, device, prefix []byte, truncate bool, fn Iterator) { t := db.newReadOnlyTransaction() defer t.close() - dbi := t.NewIterator(&util.Range{Start: start, Limit: limit}, nil) + dbi := t.NewIterator(util.BytesPrefix(db.deviceKey(folder, device, prefix)[:1+64+32+len(prefix)]), nil) defer dbi.Release() for dbi.Next() { diff --git a/lib/db/set.go b/lib/db/set.go index 481adbe6e..f505caed5 100644 --- a/lib/db/set.go +++ b/lib/db/set.go @@ -171,14 +171,18 @@ func (s *FileSet) WithNeedTruncated(device protocol.DeviceID, fn Iterator) { func (s *FileSet) WithHave(device protocol.DeviceID, fn Iterator) { l.Debugf("%s WithHave(%v)", s.folder, device) - s.db.withHave([]byte(s.folder), device[:], false, nativeFileIterator(fn)) + s.db.withHave([]byte(s.folder), device[:], nil, false, nativeFileIterator(fn)) } func (s *FileSet) WithHaveTruncated(device protocol.DeviceID, fn Iterator) { l.Debugf("%s WithHaveTruncated(%v)", s.folder, device) - s.db.withHave([]byte(s.folder), device[:], true, nativeFileIterator(fn)) + s.db.withHave([]byte(s.folder), device[:], nil, true, nativeFileIterator(fn)) } +func (s *FileSet) WithPrefixedHaveTruncated(device protocol.DeviceID, prefix string, fn Iterator) { + l.Debugf("%s WithPrefixedHaveTruncated(%v)", s.folder, device) + s.db.withHave([]byte(s.folder), device[:], []byte(prefix), true, nativeFileIterator(fn)) +} func (s *FileSet) WithGlobal(fn Iterator) { l.Debugf("%s WithGlobal()", s.folder) s.db.withGlobal([]byte(s.folder), nil, false, nativeFileIterator(fn)) diff --git a/lib/model/model.go b/lib/model/model.go index d63b63b1c..447132de7 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -1391,76 +1391,69 @@ func (m *Model) internalScanFolderSubs(folder string, subs []string) error { m.updateLocals(folder, batch) } + if len(subs) == 0 { + // If we have no specific subdirectories to traverse, set it to one + // empty prefix so we traverse the entire folder contents once. + subs = []string{""} + } + + // Do a scan of the database for each prefix, to check for deleted files. batch = batch[:0] - // TODO: We should limit the Have scanning to start at sub - seenPrefix := false - var iterError error - fs.WithHaveTruncated(protocol.LocalDeviceID, func(fi db.FileIntf) bool { - f := fi.(db.FileInfoTruncated) - hasPrefix := len(subs) == 0 - for _, sub := range subs { - if strings.HasPrefix(f.Name, sub) { - hasPrefix = true - break - } - } - // Return true so that we keep iterating, until we get to the part - // of the tree we are interested in. Then return false so we stop - // iterating when we've passed the end of the subtree. - if !hasPrefix { - return !seenPrefix - } + for _, sub := range subs { + var iterError error - seenPrefix = true - if !f.IsDeleted() { - if f.IsInvalid() { - return true - } - - if len(batch) == batchSizeFiles { - if err := m.CheckFolderHealth(folder); err != nil { - iterError = err - return false + fs.WithPrefixedHaveTruncated(protocol.LocalDeviceID, sub, func(fi db.FileIntf) bool { + f := fi.(db.FileInfoTruncated) + if !f.IsDeleted() { + if f.IsInvalid() { + return true } - m.updateLocals(folder, batch) - batch = batch[:0] - } - if ignores.Match(f.Name) || symlinkInvalid(folder, f) { - // File has been ignored or an unsupported symlink. Set invalid bit. - l.Debugln("setting invalid bit on ignored", f) - nf := protocol.FileInfo{ - Name: f.Name, - Flags: f.Flags | protocol.FlagInvalid, - Modified: f.Modified, - Version: f.Version, // The file is still the same, so don't bump version + if len(batch) == batchSizeFiles { + if err := m.CheckFolderHealth(folder); err != nil { + iterError = err + return false + } + m.updateLocals(folder, batch) + batch = batch[:0] } - batch = append(batch, nf) - } else if _, err := osutil.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil { - // File has been deleted. - // We don't specifically verify that the error is - // os.IsNotExist because there is a corner case when a - // directory is suddenly transformed into a file. When that - // happens, files that were in the directory (that is now a - // file) are deleted but will return a confusing error ("not a - // directory") when we try to Lstat() them. + if ignores.Match(f.Name) || symlinkInvalid(folder, f) { + // File has been ignored or an unsupported symlink. Set invalid bit. + l.Debugln("setting invalid bit on ignored", f) + nf := protocol.FileInfo{ + Name: f.Name, + Flags: f.Flags | protocol.FlagInvalid, + Modified: f.Modified, + Version: f.Version, // The file is still the same, so don't bump version + } + batch = append(batch, nf) + } else if _, err := osutil.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil { + // File has been deleted. - nf := protocol.FileInfo{ - Name: f.Name, - Flags: f.Flags | protocol.FlagDeleted, - Modified: f.Modified, - Version: f.Version.Update(m.shortID), + // We don't specifically verify that the error is + // os.IsNotExist because there is a corner case when a + // directory is suddenly transformed into a file. When that + // happens, files that were in the directory (that is now a + // file) are deleted but will return a confusing error ("not a + // directory") when we try to Lstat() them. + + nf := protocol.FileInfo{ + Name: f.Name, + Flags: f.Flags | protocol.FlagDeleted, + Modified: f.Modified, + Version: f.Version.Update(m.shortID), + } + batch = append(batch, nf) } - batch = append(batch, nf) } + return true + }) + + if iterError != nil { + l.Infof("Stopping folder %s mid-scan due to folder error: %s", folder, iterError) + return iterError } - return true - }) - - if iterError != nil { - l.Infof("Stopping folder %s mid-scan due to folder error: %s", folder, iterError) - return iterError } if err := m.CheckFolderHealth(folder); err != nil {