From 184459ba8fcfd7c790738d8b04cd9329cc87c226 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sun, 14 Jan 2024 17:46:25 +0000 Subject: [PATCH] vfs: fix stale data when using --vfs-cache-mode full Before this change the VFS cache could get into a state where when an object was updated remotely, the fingerprint of the item was correct for the new object but the data in the VFS cache was for the old object. This fixes the problem by updating the fingerprint of the item at the point we remove the stale data. The empty cache item now represents the new item even though it has no data in. This stops the fallback code for an empty fingerprint running (used when we are writing items to the cache instead of reading them) which was causing the problem. Fixes #6053 See: https://forum.rclone.org/t/cached-webdav-mount-fingerprints-get-nuked-on-ls/43974/ --- vfs/read_write_test.go | 44 +++++++++++++++++++++++++++++++++++++++ vfs/vfscache/item.go | 1 + vfs/vfscache/item_test.go | 6 ++++++ 3 files changed, 51 insertions(+) diff --git a/vfs/read_write_test.go b/vfs/read_write_test.go index 7a49c9ab5..bc3389151 100644 --- a/vfs/read_write_test.go +++ b/vfs/read_write_test.go @@ -714,3 +714,47 @@ func TestRWCacheRename(t *testing.T) { assert.False(t, vfs.cache.Exists("rename_me")) assert.True(t, vfs.cache.Exists("i_was_renamed")) } + +// Test the cache reading a file that is updated externally +// +// See: https://github.com/rclone/rclone/issues/6053 +func TestRWCacheUpdate(t *testing.T) { + opt := vfscommon.DefaultOpt + opt.CacheMode = vfscommon.CacheModeFull + opt.WriteBack = writeBackDelay + opt.DirCacheTime = 100 * time.Millisecond + r, vfs := newTestVFSOpt(t, &opt) + + if r.Fremote.Precision() == fs.ModTimeNotSupported { + t.Skip("skip as modtime not supported") + } + + const filename = "TestRWCacheUpdate" + + modTime := time.Now().Add(-time.Hour) + for i := 0; i < 10; i++ { + modTime = modTime.Add(time.Minute) + // Refresh test file + contents := fmt.Sprintf("TestRWCacheUpdate%03d", i) + // Increase the size for second half of test + for j := 5; j < i; j++ { + contents += "*" + } + file1 := r.WriteObject(context.Background(), filename, contents, modTime) + r.CheckRemoteItems(t, file1) + + // Wait for directory cache to expire + time.Sleep(2 * opt.DirCacheTime) + + // Check the file is OK via the VFS + data, err := vfs.ReadFile(filename) + require.NoError(t, err) + require.Equal(t, contents, string(data)) + + // Check Stat + fi, err := vfs.Stat(filename) + require.NoError(t, err) + assert.Equal(t, int64(len(contents)), fi.Size()) + fstest.AssertTimeEqualWithPrecision(t, filename, modTime, fi.ModTime(), r.Fremote.Precision()) + } +} diff --git a/vfs/vfscache/item.go b/vfs/vfscache/item.go index 06a7454a5..34962a503 100644 --- a/vfs/vfscache/item.go +++ b/vfs/vfscache/item.go @@ -810,6 +810,7 @@ func (item *Item) _checkObject(o fs.Object) error { if !item.info.Dirty { fs.Debugf(item.name, "vfs cache: removing cached entry as stale (remote fingerprint %q != cached fingerprint %q)", remoteFingerprint, item.info.Fingerprint) item._remove("stale (remote is different)") + item.info.Fingerprint = remoteFingerprint } else { fs.Debugf(item.name, "vfs cache: remote object has changed but local object modified - keeping it (remote fingerprint %q != cached fingerprint %q)", remoteFingerprint, item.info.Fingerprint) } diff --git a/vfs/vfscache/item_test.go b/vfs/vfscache/item_test.go index 27a40cbbd..fc3eeddad 100644 --- a/vfs/vfscache/item_test.go +++ b/vfs/vfscache/item_test.go @@ -409,8 +409,14 @@ func TestItemReloadCacheStale(t *testing.T) { assert.NotEqual(t, contents, contents2) // Re-open with updated object + oldFingerprint := item.info.Fingerprint + assert.NotEqual(t, "", oldFingerprint) require.NoError(t, item.Open(obj)) + // Make sure fingerprint was updated + assert.NotEqual(t, oldFingerprint, item.info.Fingerprint) + assert.NotEqual(t, "", item.info.Fingerprint) + // Check size is now 110 size, err = item.GetSize() require.NoError(t, err)