vfs: fix uploads never starting while a dirty file keeps being read

Every close of a dirty cache item re-adds it to the writeback queue,
and WriteBack.Add reset the upload timer even when the file had not
been modified since it was last queued. A file which is opened and
closed more often than --vfs-write-back - for example a git object
that another process keeps reading - therefore never got uploaded
while the reads continued.

Before this change, with --vfs-write-back 2s, a file reread every
0.5s through a mount was still not uploaded after 12s of reads. After
it the file is uploaded after 2.6s, while the reads carry on.

This only moves the timer on when the item has been modified, which
was already the condition for cancelling an upload in progress. An
unmodified re-add now also leaves the retry backoff of a failed
upload alone.

vfs/vfscache/writeback/writeback.go

@@ -254,8 +254,8 @@ func (wb *WriteBack) SetID(pid *Handle) {
 //
 // Use SetID to create Handles in advance of calling Add.
 //
-// If modified is false then it doesn't cancel a pending upload if
-// there is one as there is no need.
+// If modified is false then it neither cancels an upload in progress
+// nor delays a pending one, as the data to upload hasn't changed.
 func (wb *WriteBack) Add(id Handle, name string, size int64, modified bool, putFn PutFn) Handle {
 	wb.mu.Lock()
 	defer wb.mu.Unlock()
@@ -263,8 +263,8 @@ func (wb *WriteBack) Add(id Handle, name string, size int64, modified bool, putF
 	wbItem, ok := wb.lookup[id]
 	if !ok {
 		wbItem = wb._newItem(id, name, size)
-	} else {
-		if wbItem.uploading && modified {
+	} else if modified {
+		if wbItem.uploading {
 			// We are uploading already so cancel the upload
 			wb._cancelUpload(wbItem)
 		}

vfs/vfscache/writeback/writeback_test.go

@@ -467,6 +467,28 @@ func TestWriteBackAddUpdateNotStarted(t *testing.T) {
 	assert.False(t, pi.called)
 }
 
+// Re-adding an unmodified item, as closing a dirty file which was only
+// read does, must not delay its upload, otherwise a file which is read
+// more often than the writeback delay never gets uploaded.
+func TestWriteBackAddNotModifiedKeepsExpiry(t *testing.T) {
+	wb, cancel := newTestWriteBack(t)
+	defer cancel()
+	wb.opt.WriteBack = fs.Duration(time.Hour)
+
+	pi := newPutItem(t)
+	id := wb.Add(0, "one", 10, true, pi.put)
+	wbItem := wb.lookup[id]
+	expiry := wbItem.expiry
+
+	time.Sleep(10 * time.Millisecond)
+	wb.Add(id, "one", 10, false, pi.put)
+	assert.Equal(t, expiry, wbItem.expiry, "unmodified add delayed the upload")
+
+	wb.Add(id, "one", 10, true, pi.put)
+	assert.True(t, wbItem.expiry.After(expiry), "modified add didn't delay the upload")
+	assert.False(t, pi.called)
+}
+
 func TestWriteBackGetStats(t *testing.T) {
 	wb, cancel := newTestWriteBack(t)
 	defer cancel()