vfs: fix change notify discarding directory cache for changes the VFS made

On a backend which supports polling for changes, every write made
through the VFS came back as a change notification which invalidated
the listing of the directory the file was in and (for backends which
report a file change as a change to the containing directory) the
listing of that directory's parent too. Where rclone is the writer
this threw away the cache it was about to need, so a mount or serve
writing into a large directory re-listed it on every file written.

Each directory now records when the VFS last changed its contents, and
a notification which is explained by one of those changes no longer
invalidates the parent listing. The directory the notification names is
still invalidated, so changes made elsewhere within it are picked up as
before.

Fixes #9856

vfs/dir.go

@@ -35,6 +35,7 @@ type Dir struct {
 	path    string
 	entry   fs.Directory
 	read    time.Time         // time directory entry last read
+	changed time.Time         // time the VFS last changed the contents of this directory
 	items   map[string]Node   // directory entries - can be empty but not nil
 	virtual map[string]vState // virtual directory entries - may be nil
 
@@ -273,16 +274,50 @@ func (d *Dir) invalidateDir(absPath string) {
 	}
 }
 
+// changedRecently returns true if this VFS changed the contents of the
+// directory at absPath recently enough that a change notification arriving
+// now is likely to be describing that change.
+//
+// A notification can take up to the poll interval to arrive, so allow a
+// generous margin on top of that.
+func (d *Dir) changedRecently(absPath string) bool {
+	dir, ok := d.vfs.root.cachedNode(absPath).(*Dir)
+	if !ok {
+		return false
+	}
+	window := max(2*time.Duration(d.vfs.Opt.PollInterval), time.Minute)
+	dir.mu.RLock()
+	defer dir.mu.RUnlock()
+	return !dir.changed.IsZero() && time.Since(dir.changed) < window
+}
+
 // changeNotify invalidates the directory cache for the relativePath
 // passed in.
 //
 // if entryType is a directory it invalidates the parent of the directory too.
+//
+// Notifications describing changes this VFS made itself are ignored where
+// they can be identified, as the directory cache was updated when the
+// change was made.
 func (d *Dir) changeNotify(relativePath string, entryType fs.EntryType) {
 	defer log.Trace(d.path, "relativePath=%q, type=%v", relativePath, entryType)("")
 	d.mu.RLock()
 	absPath := path.Join(d.path, relativePath)
 	d.mu.RUnlock()
-	d.invalidateDir(vfscommon.FindParent(absPath))
+	parent := vfscommon.FindParent(absPath)
+	// The parent listing only changes if absPath itself was created,
+	// removed or renamed. A notification for a directory may instead be
+	// reporting a change to that directory's contents, in which case the
+	// parent listing is still correct.
+	ours := d.changedRecently(parent)
+	if !ours && entryType == fs.EntryDirectory {
+		ours = d.changedRecently(absPath)
+	}
+	if ours {
+		fs.Debugf(parent, "Not invalidating directory cache as this change was made by the VFS")
+	} else {
+		d.invalidateDir(parent)
+	}
 	if entryType == fs.EntryDirectory {
 		d.invalidateDir(absPath)
 	}
@@ -436,6 +471,7 @@ func (d *Dir) addObject(node Node) {
 	d.mu.Lock()
 	leaf := node.Name()
 	d.items[leaf] = node
+	d.changed = time.Now()
 	if d.virtual == nil {
 		d.virtual = make(map[string]vState)
 	}
@@ -496,6 +532,7 @@ func (d *Dir) AddVirtual(leaf string, size int64, isDir bool) {
 func (d *Dir) delObject(leaf string) {
 	d.mu.Lock()
 	delete(d.items, leaf)
+	d.changed = time.Now()
 	if d.virtual == nil {
 		d.virtual = make(map[string]vState)
 	}

vfs/dir_test.go

@@ -141,6 +141,65 @@ func TestDirForgetPath(t *testing.T) {
 	assert.Equal(t, 0, len(dir.items))
 }
 
+// Check that change notifications describing changes the VFS made
+// itself don't invalidate the parent directory listing
+func TestDirChangeNotify(t *testing.T) {
+	_, vfs, dir, file1 := dirCreate(t)
+
+	root, err := vfs.Root()
+	require.NoError(t, err)
+
+	// Read the directories in so they are in the cache
+	readDirs := func() {
+		_, err := vfs.Stat(file1.Path)
+		require.NoError(t, err)
+		require.False(t, root.read.IsZero())
+		require.False(t, dir.read.IsZero())
+	}
+
+	// Forget the changes the VFS has made
+	forgetChanges := func() {
+		for _, d := range []*Dir{root, dir} {
+			d.mu.Lock()
+			d.changed = time.Time{}
+			d.mu.Unlock()
+		}
+	}
+
+	// A file changed by someone else invalidates the directory it is in
+	readDirs()
+	forgetChanges()
+	root.changeNotify("dir/file2", fs.EntryObject)
+	assert.False(t, root.read.IsZero())
+	assert.True(t, dir.read.IsZero())
+
+	// A directory changed by someone else invalidates the directory and its parent
+	readDirs()
+	forgetChanges()
+	root.changeNotify("dir", fs.EntryDirectory)
+	assert.True(t, root.read.IsZero())
+	assert.True(t, dir.read.IsZero())
+
+	// Write a file into dir with the VFS
+	fd, err := vfs.Create("dir/file2")
+	require.NoError(t, err)
+	require.NoError(t, fd.Close())
+
+	// A notification for that file doesn't invalidate dir
+	readDirs()
+	root.changeNotify("dir/file2", fs.EntryObject)
+	assert.False(t, root.read.IsZero())
+	assert.False(t, dir.read.IsZero())
+
+	// Nor does a notification for dir, which is how some backends
+	// report a change to the contents of a directory - dir itself is
+	// still invalidated as it may have changed in other ways
+	readDirs()
+	root.changeNotify("dir", fs.EntryDirectory)
+	assert.False(t, root.read.IsZero())
+	assert.True(t, dir.read.IsZero())
+}
+
 func TestDirWalk(t *testing.T) {
 	r, vfs, _, file1 := dirCreate(t)
 

vfs/vfs.md

@@ -30,6 +30,12 @@ the directory cache expires if the backend configured does not support
 polling for changes. If the backend supports polling, changes will be
 picked up within the polling interval.
 
+Changes that this rclone made itself are recognised as such and don't
+invalidate the directory cache, as it was updated when the change was
+made. A side effect of this is that a change made elsewhere to a
+directory this rclone has just written to may not be noticed until the
+directory cache expires.
+
 You can send a `SIGHUP` signal to rclone for it to flush all
 directory caches, regardless of how old they are.  Assuming only one
 rclone instance is running, you can reset the cache like this: