extern/rclone
1
0
Fork 0
mirror of https://github.com/rclone/rclone.git synced 2025-03-06 03:11:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

bisync: fix listings missing concurrent modifications - fixes #8359

Browse Source 
Before this change, there was a bug affecting listing files when:

- a given bisync run had changes in the 2to1 direction
AND
- the run had NO changes in the 1to2 direction
AND
- at least one of the changed files changed AGAIN during the run
(specifically, after the initial march and before the transfers.)

In this situation, the listings on one side would still retain the prior version
of the changed file, potentially causing conflicts or errors.

This change fixes the issue by making sure that if we're updating the listings
on one side, we must also update the other. (We previously tried to skip it for
efficiency, but this failed to account for the possibility that a changed file
could change again during the run.)
This commit is contained in:
nielash 2025-02-09 21:13:55 -05:00 committed by Nick Craig-Wood
parent c0515a51a5
commit 3b49fd24d4
31 changed files with 181 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
cmd/bisync/bisync_test.go
View File

@ -746,6 +746,16 @@ func (b *bisyncTest) runTestStep(ctx context.Context, line string) (err error) {
case "test-func":
b.TestFn = testFunc
return
case "concurrent-func":
b.TestFn = func() {
src := filepath.Join(b.dataDir, "file7.txt")
dst := "file1.txt"
err := b.copyFile(ctx, src, b.replaceHex(b.path2), dst)
if err != nil {
fs.Errorf(src, "error copying file: %v", err)
}
}
return
case "fix-names":
// in case the local os converted any filenames
ci.NoUnicodeNormalization = true

16
cmd/bisync/deltas.go
View File

@ -286,7 +286,7 @@ func (b *bisyncRun) findDeltas(fctx context.Context, f fs.Fs, oldListing string,
}
// applyDeltas
func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (changes1, changes2 bool, results2to1, results1to2 []Results, queues queues, err error) {
func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (results2to1, results1to2 []Results, queues queues, err error) {
path1 := bilib.FsPath(b.fs1)
path2 := bilib.FsPath(b.fs2)
@ -367,7 +367,7 @@ func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (change
}
}
//if there are potential conflicts to check, check them all here (outside the loop) in one fell swoop
// if there are potential conflicts to check, check them all here (outside the loop) in one fell swoop
matches, err := b.checkconflicts(ctxCheck, filterCheck, b.fs1, b.fs2)
for _, file := range ds1.sort() {
@ -392,7 +392,7 @@ func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (change
} else if d2.is(deltaOther) {
b.indent("!WARNING", file, "New or changed in both paths")
//if files are identical, leave them alone instead of renaming
// if files are identical, leave them alone instead of renaming
if (dirs1.has(file) || dirs1.has(alias)) && (dirs2.has(file) || dirs2.has(alias)) {
fs.Infof(nil, "This is a directory, not a file. Skipping equality check and will not rename: %s", file)
ls1.getPut(file, skippedDirs1)
@ -486,7 +486,6 @@ func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (change
// Do the batch operation
if copy2to1.NotEmpty() && !b.InGracefulShutdown {
changes1 = true
b.indent("Path2", "Path1", "Do queued copies to")
ctx = b.setBackupDir(ctx, 1)
results2to1, err = b.fastCopy(ctx, b.fs2, b.fs1, copy2to1, "copy2to1")
@ -498,12 +497,11 @@ func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (change
return
}
//copy empty dirs from path2 to path1 (if --create-empty-src-dirs)
// copy empty dirs from path2 to path1 (if --create-empty-src-dirs)
b.syncEmptyDirs(ctx, b.fs1, copy2to1, dirs2, &results2to1, "make")
}
if copy1to2.NotEmpty() && !b.InGracefulShutdown {
changes2 = true
b.indent("Path1", "Path2", "Do queued copies to")
ctx = b.setBackupDir(ctx, 2)
results1to2, err = b.fastCopy(ctx, b.fs1, b.fs2, copy1to2, "copy1to2")
@ -515,7 +513,7 @@ func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (change
return
}
//copy empty dirs from path1 to path2 (if --create-empty-src-dirs)
// copy empty dirs from path1 to path2 (if --create-empty-src-dirs)
b.syncEmptyDirs(ctx, b.fs2, copy1to2, dirs1, &results1to2, "make")
}
@ -523,7 +521,7 @@ func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (change
if err = b.saveQueue(delete1, "delete1"); err != nil {
return
}
//propagate deletions of empty dirs from path2 to path1 (if --create-empty-src-dirs)
// propagate deletions of empty dirs from path2 to path1 (if --create-empty-src-dirs)
b.syncEmptyDirs(ctx, b.fs1, delete1, dirs1, &results2to1, "remove")
}
@ -531,7 +529,7 @@ func (b *bisyncRun) applyDeltas(ctx context.Context, ds1, ds2 *deltaSet) (change
if err = b.saveQueue(delete2, "delete2"); err != nil {
return
}
//propagate deletions of empty dirs from path1 to path2 (if --create-empty-src-dirs)
// propagate deletions of empty dirs from path1 to path2 (if --create-empty-src-dirs)
b.syncEmptyDirs(ctx, b.fs2, delete2, dirs2, &results1to2, "remove")
}

18
cmd/bisync/operations.go
View File

@ -359,8 +359,6 @@ func (b *bisyncRun) runLocked(octx context.Context) (err error) {
// Determine and apply changes to Path1 and Path2
noChanges := ds1.empty() && ds2.empty()
changes1 := false // 2to1
changes2 := false // 1to2
results2to1 := []Results{}
results1to2 := []Results{}
@ -370,7 +368,7 @@ func (b *bisyncRun) runLocked(octx context.Context) (err error) {
fs.Infof(nil, "No changes found")
} else {
fs.Infof(nil, "Applying changes")
changes1, changes2, results2to1, results1to2, queues, err = b.applyDeltas(octx, ds1, ds2)
results2to1, results1to2, queues, err = b.applyDeltas(octx, ds1, ds2)
if err != nil {
if b.InGracefulShutdown && (err == context.Canceled || err == accounting.ErrorMaxTransferLimitReachedGraceful || strings.Contains(err.Error(), "context canceled")) {
fs.Infof(nil, "Ignoring sync error due to Graceful Shutdown: %v", err)
@ -395,21 +393,11 @@ func (b *bisyncRun) runLocked(octx context.Context) (err error) {
}
b.saveOldListings()
// save new listings
// NOTE: "changes" in this case does not mean this run vs. last run, it means start of this run vs. end of this run.
// i.e. whether we can use the March lst-new as this side's lst without modifying it.
if noChanges {
b.replaceCurrentListings()
} else {
if changes1 || b.InGracefulShutdown { // 2to1
err1 = b.modifyListing(fctx, b.fs2, b.fs1, results2to1, queues, false)
} else {
err1 = bilib.CopyFileIfExists(b.newListing1, b.listing1)
}
if changes2 || b.InGracefulShutdown { // 1to2
err2 = b.modifyListing(fctx, b.fs1, b.fs2, results1to2, queues, true)
} else {
err2 = bilib.CopyFileIfExists(b.newListing2, b.listing2)
}
err1 = b.modifyListing(fctx, b.fs2, b.fs1, results2to1, queues, false) // 2to1
err2 = b.modifyListing(fctx, b.fs1, b.fs2, results1to2, queues, true) // 1to2
}
if b.DebugName != "" {
l1, _ := b.loadListing(b.listing1)

1
cmd/bisync/testdata/test_concurrent/golden/_testdir_path1.._testdir_path2.copy2to1.que vendored Normal file
View File

@ -0,0 +1 @@
"file1.txt"

10
cmd/bisync/testdata/test_concurrent/golden/_testdir_path1.._testdir_path2.path1.lst vendored Normal file
View File

@ -0,0 +1,10 @@
# bisync listing v1 from test
- 109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
- 19 - - 2023-08-26T00:00:00.000000000+0000 "file1.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file2.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file3.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file4.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file5.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file6.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file7.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file8.txt"

10
cmd/bisync/testdata/test_concurrent/golden/_testdir_path1.._testdir_path2.path1.lst-new vendored Normal file
View File

@ -0,0 +1,10 @@
# bisync listing v1 from test
- 109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
- 19 - - 2023-08-26T00:00:00.000000000+0000 "file1.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file2.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file3.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file4.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file5.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file6.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file7.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file8.txt"

10
cmd/bisync/testdata/test_concurrent/golden/_testdir_path1.._testdir_path2.path1.lst-old vendored Normal file
View File

@ -0,0 +1,10 @@
# bisync listing v1 from test
- 109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
- 19 - - 2023-08-26T00:00:00.000000000+0000 "file1.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file2.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file3.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file4.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file5.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file6.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file7.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file8.txt"

10
cmd/bisync/testdata/test_concurrent/golden/_testdir_path1.._testdir_path2.path2.lst vendored Normal file
View File

@ -0,0 +1,10 @@
# bisync listing v1 from test
- 109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
- 19 - - 2023-08-26T00:00:00.000000000+0000 "file1.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file2.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file3.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file4.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file5.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file6.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file7.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file8.txt"

10
cmd/bisync/testdata/test_concurrent/golden/_testdir_path1.._testdir_path2.path2.lst-new vendored Normal file
View File

@ -0,0 +1,10 @@
# bisync listing v1 from test
- 109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
- 19 - - 2023-08-26T00:00:00.000000000+0000 "file1.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file2.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file3.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file4.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file5.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file6.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file7.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file8.txt"

10
cmd/bisync/testdata/test_concurrent/golden/_testdir_path1.._testdir_path2.path2.lst-old vendored Normal file
View File

@ -0,0 +1,10 @@
# bisync listing v1 from test
- 109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
- 19 - - 2023-08-26T00:00:00.000000000+0000 "file1.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file2.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file3.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file4.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file5.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file6.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file7.txt"
- 0 - - 2000-01-01T00:00:00.000000000+0000 "file8.txt"

73
cmd/bisync/testdata/test_concurrent/golden/test.log vendored Normal file
View File

@ -0,0 +1,73 @@
(01) : test concurrent
(02) : test initial bisync
(03) : bisync resync
INFO : Setting --ignore-listing-checksum as neither --checksum nor --compare checksum are set.
INFO : Bisyncing with Comparison Settings:
{
"Modtime": true,
"Size": true,
"Checksum": false,
"NoSlowHash": false,
"SlowHashSyncOnly": false,
"DownloadHash": false
}
INFO : Synching Path1 "{path1/}" with Path2 "{path2/}"
INFO : Copying Path2 files to Path1
INFO : - Path2 Resync is copying files to - Path1
INFO : - Path1 Resync is copying files to - Path2
INFO : Resync updating listings
INFO : Validating listings for Path1 "{path1/}" vs Path2 "{path2/}"
INFO : Bisync successful
(04) : test changed on one path - file1
(05) : touch-glob 2001-01-02 {datadir/} file5R.txt
(06) : touch-glob 2023-08-26 {datadir/} file7.txt
(07) : copy-as {datadir/}file5R.txt {path2/} file1.txt
(08) : test bisync with file changed during
(09) : concurrent-func
(10) : bisync
INFO : Setting --ignore-listing-checksum as neither --checksum nor --compare checksum are set.
INFO : Bisyncing with Comparison Settings:
{
"Modtime": true,
"Size": true,
"Checksum": false,
"NoSlowHash": false,
"SlowHashSyncOnly": false,
"DownloadHash": false
}
INFO : Synching Path1 "{path1/}" with Path2 "{path2/}"
INFO : Building Path1 and Path2 listings
INFO : Path1 checking for diffs
INFO : Path2 checking for diffs
INFO : - Path2 File changed: size (larger), time (newer) - file1.txt
INFO : Path2: 1 changes:  0 new,  1 modified,  0 deleted
INFO : (Modified:  1 newer,  0 older,  1 larger,  0 smaller)
INFO : Applying changes
INFO : - Path2 Queue copy to Path1 - {path1/}file1.txt
INFO : - Path2 Do queued copies to - Path1
INFO : Updating listings
INFO : Validating listings for Path1 "{path1/}" vs Path2 "{path2/}"
INFO : Bisync successful
(11) : bisync
INFO : Setting --ignore-listing-checksum as neither --checksum nor --compare checksum are set.
INFO : Bisyncing with Comparison Settings:
{
"Modtime": true,
"Size": true,
"Checksum": false,
"NoSlowHash": false,
"SlowHashSyncOnly": false,
"DownloadHash": false
}
INFO : Synching Path1 "{path1/}" with Path2 "{path2/}"
INFO : Building Path1 and Path2 listings
INFO : Path1 checking for diffs
INFO : Path2 checking for diffs
INFO : No changes found
INFO : Updating listings
INFO : Validating listings for Path1 "{path1/}" vs Path2 "{path2/}"
INFO : Bisync successful

1
cmd/bisync/testdata/test_concurrent/initial/RCLONE_TEST vendored Normal file
View File

@ -0,0 +1 @@
This file is used for testing the health of rclone accesses to the local/remote file system. Do not delete.

0
cmd/bisync/testdata/test_concurrent/initial/file1.txt vendored Normal file
View File

0
cmd/bisync/testdata/test_concurrent/initial/file2.txt vendored Normal file
View File

0
cmd/bisync/testdata/test_concurrent/initial/file3.txt vendored Normal file
View File

0
cmd/bisync/testdata/test_concurrent/initial/file4.txt vendored Normal file
View File

0
cmd/bisync/testdata/test_concurrent/initial/file5.txt vendored Normal file
View File

0
cmd/bisync/testdata/test_concurrent/initial/file6.txt vendored Normal file
View File

0
cmd/bisync/testdata/test_concurrent/initial/file7.txt vendored Normal file
View File

0
cmd/bisync/testdata/test_concurrent/initial/file8.txt vendored Normal file
View File

0
cmd/bisync/testdata/test_concurrent/modfiles/dummy.txt vendored Normal file
View File

1
cmd/bisync/testdata/test_concurrent/modfiles/file1.txt vendored Normal file
View File

@ -0,0 +1 @@
This file is newer

1
cmd/bisync/testdata/test_concurrent/modfiles/file10.txt vendored Normal file
View File

@ -0,0 +1 @@
This file is newer

1
cmd/bisync/testdata/test_concurrent/modfiles/file11.txt vendored Normal file
View File

@ -0,0 +1 @@
This file is newer

1
cmd/bisync/testdata/test_concurrent/modfiles/file2.txt vendored Normal file
View File

@ -0,0 +1 @@
Newer version

1
cmd/bisync/testdata/test_concurrent/modfiles/file5L.txt vendored Normal file
View File

@ -0,0 +1 @@
This file is newer and not equal to 5R

1
cmd/bisync/testdata/test_concurrent/modfiles/file5R.txt vendored Normal file
View File

@ -0,0 +1 @@
This file is newer and not equal to 5L

1
cmd/bisync/testdata/test_concurrent/modfiles/file6.txt vendored Normal file
View File

@ -0,0 +1 @@
This file is newer

1
cmd/bisync/testdata/test_concurrent/modfiles/file7.txt vendored Normal file
View File

@ -0,0 +1 @@
This file is newer

15
cmd/bisync/testdata/test_concurrent/scenario.txt vendored Normal file
View File

@ -0,0 +1,15 @@
test concurrent
test initial bisync
bisync resync
test changed on one path - file1
touch-glob 2001-01-02 {datadir/} file5R.txt
touch-glob 2023-08-26 {datadir/} file7.txt
copy-as {datadir/}file5R.txt {path2/} file1.txt
test bisync with file changed during
concurrent-func
bisync
bisync

3
docs/content/bisync.md
View File

@ -1815,6 +1815,9 @@ about _Unison_ and synchronization in general.
## Changelog
### `v1.69.1`
* Fixed an issue causing listings to not capture concurrent modifications under certain conditions
### `v1.68`
* Fixed an issue affecting backends that round modtimes to a lower precision.