From 194dce27bf91fbdb77dd935c2037dcaf9dec1073 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 8 May 2026 12:37:23 -0700 Subject: [PATCH] fix(mount): preserve user-set mtime through async/periodic flush (#9363) (#9370) * fix(mount): preserve user-set mtime through async/periodic flush (#9363) flushMetadataToFiler and flushFileMetadata both stamped time.Now() onto the entry before sending it to the filer, clobbering any mtime SetAttr had stored from utimes()/touch -m -d. The reproducer hit this ~1s after touch because the writebackCache deferred close from the prior write ran flushMetadataToFiler after the user's utimes call. Flush has no business inventing timestamps. Move the write-time stamp into Write (where it always belonged for POSIX correctness) and let flush persist whatever Write or SetAttr already put on the entry. * test(mount): tighten mtime regression test, drop tautological one - userMtime now has non-zero nanoseconds, so the *Ns assertions catch a regression that would zero the field. - Add CtimeNs assertion (was missing). - Drop TestWriteStampsEntryMtime: it duplicated the implementation it was supposed to test, so a regression in Write would not have failed it. Driving the real Write path needs a full PageWriter, which is out of scope for this fix; TestFlushFileMetadataPreservesUserMtime is the meaningful regression for #9363. --- weed/mount/weedfs_file_sync.go | 8 +-- weed/mount/weedfs_file_write.go | 9 +++ weed/mount/weedfs_metadata_flush.go | 10 +-- .../mount/weedfs_metadata_flush_mtime_test.go | 70 +++++++++++++++++++ 4 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 weed/mount/weedfs_metadata_flush_mtime_test.go diff --git a/weed/mount/weedfs_file_sync.go b/weed/mount/weedfs_file_sync.go index 4724a98c9..c59f0d4d7 100644 --- a/weed/mount/weedfs_file_sync.go +++ b/weed/mount/weedfs_file_sync.go @@ -201,11 +201,9 @@ func (wfs *WFS) flushMetadataToFiler(fh *FileHandle, dir, name string, uid, gid if entry.Attributes.Gid == 0 { entry.Attributes.Gid = gid } - flushNow := time.Now() - entry.Attributes.Mtime = flushNow.Unix() - entry.Attributes.MtimeNs = int32(flushNow.Nanosecond()) - entry.Attributes.Ctime = flushNow.Unix() - entry.Attributes.CtimeNs = int32(flushNow.Nanosecond()) + // Do not stamp mtime/ctime here. Write/SetAttr already maintain + // them on the entry; overwriting at flush time clobbered user-set + // mtime (utimes/touch -m -d) once the deferred flush ran. } glog.V(4).Infof("%s set chunks: %v", fileFullPath, len(entry.GetChunks())) diff --git a/weed/mount/weedfs_file_write.go b/weed/mount/weedfs_file_write.go index ea64cb622..a70932ff1 100644 --- a/weed/mount/weedfs_file_write.go +++ b/weed/mount/weedfs_file_write.go @@ -66,6 +66,15 @@ func (wfs *WFS) Write(cancel <-chan struct{}, in *fuse.WriteIn, data []byte) (wr newFileSize := max(offset+int64(len(data)), oldFileSize) entry.Attributes.FileSize = uint64(newFileSize) + // POSIX: writes update mtime and ctime. Stamp the entry now so the + // flush path persists the write time rather than overwriting any + // user-set mtime (e.g., utimes/touch -m -d) at flush time. + writeNow := time.Unix(0, tsNs) + entry.Attributes.Mtime = writeNow.Unix() + entry.Attributes.MtimeNs = int32(writeNow.Nanosecond()) + entry.Attributes.Ctime = writeNow.Unix() + entry.Attributes.CtimeNs = int32(writeNow.Nanosecond()) + // Track uncommitted bytes for real-time quota enforcement. // Only count the new bytes being added beyond the current file size. if newFileSize > oldFileSize { diff --git a/weed/mount/weedfs_metadata_flush.go b/weed/mount/weedfs_metadata_flush.go index 63c8a0847..8d9f83a2f 100644 --- a/weed/mount/weedfs_metadata_flush.go +++ b/weed/mount/weedfs_metadata_flush.go @@ -110,13 +110,9 @@ func (wfs *WFS) flushFileMetadata(fh *FileHandle) error { } entry.Name = name - if entry.Attributes != nil { - metaNow := time.Now() - entry.Attributes.Mtime = metaNow.Unix() - entry.Attributes.MtimeNs = int32(metaNow.Nanosecond()) - entry.Attributes.Ctime = metaNow.Unix() - entry.Attributes.CtimeNs = int32(metaNow.Nanosecond()) - } + // Do not stamp mtime/ctime here. Write/SetAttr already maintain + // them on the entry; overwriting at periodic-flush time clobbered + // user-set mtime (utimes/touch -m -d) once the timer fired. // Get current chunks - these include chunks that have been uploaded // but not yet persisted to filer metadata diff --git a/weed/mount/weedfs_metadata_flush_mtime_test.go b/weed/mount/weedfs_metadata_flush_mtime_test.go new file mode 100644 index 000000000..a2fc3554d --- /dev/null +++ b/weed/mount/weedfs_metadata_flush_mtime_test.go @@ -0,0 +1,70 @@ +package mount + +import ( + "testing" + "time" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/util" +) + +// TestFlushFileMetadataPreservesUserMtime is a regression test for issue #9363. +// +// Before the fix, flushFileMetadata stamped entry.Attributes.Mtime/Ctime with +// time.Now() on every flush, clobbering the value SetAttr stored on the entry +// when the user ran utimes()/touch -m -d while a file handle was still open. +// +// The flush has no business inventing timestamps: Write and SetAttr already +// maintain mtime/ctime on the entry, and the flush should just persist them. +// +// The test sets a user-chosen mtime far in the past, runs flushFileMetadata +// with no chunks (so it returns before the streamCreateEntry RPC), and asserts +// the entry's mtime is unchanged. +func TestFlushFileMetadataPreservesUserMtime(t *testing.T) { + wfs := &WFS{ + inodeToPath: NewInodeToPath(util.FullPath("/"), 0), + fhLockTable: util.NewLockTable[FileHandleId](), + option: &Option{}, + } + + const inode = uint64(42) + fullPath := util.FullPath("/dir/sample.txt") + wfs.inodeToPath.Lookup(fullPath, 1, false, false, inode, true) + + // Use a non-zero nanosecond so the *Ns assertions below catch a regression + // that zeroes the field instead of preserving it. + userMtime := time.Date(2020, 1, 15, 12, 34, 56, 123456789, time.UTC) + entry := &filer_pb.Entry{ + Name: "sample.txt", + Attributes: &filer_pb.FuseAttributes{ + Mtime: userMtime.Unix(), + MtimeNs: int32(userMtime.Nanosecond()), + Ctime: userMtime.Unix(), + CtimeNs: int32(userMtime.Nanosecond()), + }, + } + fh := &FileHandle{ + fh: FileHandleId(1), + inode: inode, + wfs: wfs, + entry: &LockedEntry{Entry: entry}, + dirtyMetadata: true, + } + + if err := wfs.flushFileMetadata(fh); err != nil { + t.Fatalf("flushFileMetadata returned error: %v", err) + } + + if got := entry.Attributes.Mtime; got != userMtime.Unix() { + t.Errorf("mtime sec changed: got %d, want %d", got, userMtime.Unix()) + } + if got := entry.Attributes.MtimeNs; got != int32(userMtime.Nanosecond()) { + t.Errorf("mtime ns changed: got %d, want %d", got, userMtime.Nanosecond()) + } + if got := entry.Attributes.Ctime; got != userMtime.Unix() { + t.Errorf("ctime sec changed: got %d, want %d", got, userMtime.Unix()) + } + if got := entry.Attributes.CtimeNs; got != int32(userMtime.Nanosecond()) { + t.Errorf("ctime ns changed: got %d, want %d", got, userMtime.Nanosecond()) + } +}