backend/local: skip entries removed concurrently with List() (#5297)

This change fixes the bug described below:
if a file is removed while the local backend List() runs,
the call will flag an accounting error.
The bug manifests itself if local backend is the Sync target
due to intrinsic concurrency.
The odds to hit this bug depend on --checkers and --transfers.
Chunker over local backend is affected even more because
updating a composite object with a smaller size content
translates into removing chunks on the underlying file system
and involves a number of List() calls.
This commit is contained in:
Ivan Andreev 2021-07-07 16:50:19 +03:00 committed by GitHub
parent fb305b5976
commit 4680c0776d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 56 additions and 3 deletions

View File

@ -467,6 +467,10 @@ func (f *Fs) List(ctx context.Context, dir string) (entries fs.DirEntries, err e
for _, name := range names {
namepath := filepath.Join(fsDirPath, name)
fi, fierr := os.Lstat(namepath)
if os.IsNotExist(fierr) {
// skip entry removed by a concurrent goroutine
continue
}
if fierr != nil {
err = errors.Wrapf(err, "failed to read directory %q", namepath)
fs.Errorf(dir, "%v", fierr)

View File

@ -624,9 +624,7 @@ func (s *syncCopyMove) srcParentDirCheck(entry fs.DirEntry) {
if parentDir == "." {
parentDir = ""
}
if _, ok := s.srcEmptyDirs[parentDir]; ok {
delete(s.srcEmptyDirs, parentDir)
}
delete(s.srcEmptyDirs, parentDir)
}
// parseTrackRenamesStrategy turns a config string into a trackRenamesStrategy

View File

@ -1973,3 +1973,54 @@ func TestMaxTransfer(t *testing.T) {
t.Run("Soft", func(t *testing.T) { test(t, fs.CutoffModeSoft) })
t.Run("Cautious", func(t *testing.T) { test(t, fs.CutoffModeCautious) })
}
func testSyncConcurrent(t *testing.T, subtest string) {
const (
NFILES = 20
NCHECKERS = 4
NTRANSFERS = 4
)
ctx, ci := fs.AddConfig(context.Background())
ci.Checkers = NCHECKERS
ci.Transfers = NTRANSFERS
r := fstest.NewRun(t)
defer r.Finalise()
stats := accounting.GlobalStats()
itemsBefore := []fstest.Item{}
itemsAfter := []fstest.Item{}
for i := 0; i < NFILES; i++ {
nameBoth := fmt.Sprintf("both%d", i)
nameOnly := fmt.Sprintf("only%d", i)
switch subtest {
case "delete":
fileBoth := r.WriteBoth(ctx, nameBoth, "potato", t1)
fileOnly := r.WriteObject(ctx, nameOnly, "potato", t1)
itemsBefore = append(itemsBefore, fileBoth, fileOnly)
itemsAfter = append(itemsAfter, fileBoth)
case "truncate":
fileBoth := r.WriteBoth(ctx, nameBoth, "potato", t1)
fileFull := r.WriteObject(ctx, nameOnly, "potato", t1)
fileEmpty := r.WriteFile(nameOnly, "", t1)
itemsBefore = append(itemsBefore, fileBoth, fileFull)
itemsAfter = append(itemsAfter, fileBoth, fileEmpty)
}
}
fstest.CheckItems(t, r.Fremote, itemsBefore...)
stats.ResetErrors()
err := Sync(ctx, r.Fremote, r.Flocal, false)
assert.NoError(t, err, "Sync must not return a error")
assert.False(t, stats.Errored(), "Low level errors must not have happened")
fstest.CheckItems(t, r.Fremote, itemsAfter...)
}
func TestSyncConcurrentDelete(t *testing.T) {
testSyncConcurrent(t, "delete")
}
func TestSyncConcurrentTruncate(t *testing.T) {
testSyncConcurrent(t, "truncate")
}