filer.remote.sync: stop a touched file from being restamped forever (#12419)

* filer.remote.sync: stop a touched file from being restamped forever

A metadata-only update that moves the mtime past RemoteMtime (touch, or
an O_TRUNC open of an empty lock file) goes through UpdateFileMetadata,
which changes nothing on the remote, and the entry is stamped with the
same RemoteEntry and a new LastLocalSyncTsNs. That stamp is an update
event of its own. shouldSendToRemote only looks at LastLocalSyncTsNs
when RemoteMtime and the mtime share a second, so it still reads the
entry as dirty, and the event is stamped again. Nothing is uploaded,
but the daemon keeps rewriting the entry as fast as it reads its own
events back.

Skip the restamp when the backend left the object alone and the event's
entry already carries a stamp taken after its mtime. The touch itself is
still stamped once.

* filer.remote.sync: log the skipped restamp

The other skips in processUpdateEvent log at V(2); this one did not, so
with -v=2 a touched file showed an "update meta" line with nothing after
it.

* filer.remote.sync: skip the restamp of a stamp's own event, whatever the clocks

The skip compared the entry's stamp with its mtime, so an mtime ahead of
the sync's clock (a skewed writer, touch -d) stayed past every stamp and
kept the loop going until the clock caught up. An event that changed only
the RemoteEntry is the sync's own stamp, which needs no other.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* filer.remote.sync: ignore the filer's commit timestamp when spotting a stamp

The filer gives every stored update a new CommitTsNs, so a stamp's event
never compared equal outside the RemoteEntry and a touched file kept being
restamped against a real filer. The test now assigns one per update.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

weed/command/filer_remote_sync_dir.go

@@ -507,6 +507,11 @@ func processUpdateEvent(
 			// the object the update produced when the backend reports it
 			if updated != nil {
 				remoteEntry = updated
+			} else if isStampOnlyUpdate(message) {
+				// restamping a stamp's own event logs the next one, and an mtime
+				// past RemoteMtime (a touch) keeps reading dirty, forever
+				glog.V(2).Infof("skipping restamp of synced entry: %s/%s ts_ns=%d", filer_pb.MetadataEventTargetDirectory(resp), message.GetNewEntry().GetName(), resp.TsNs)
+				return nil
 			}
 			return updateLocalEntry(filerClient, message.NewParentPath, message.NewEntry, remoteEntry)
 		}
@@ -1098,6 +1103,20 @@ func isMetadataOnlyUpdate(dir string, message *filer_pb.EventNotification) bool
 	return filer.IsSameData(message.OldEntry, message.NewEntry)
 }
 
+// isStampOnlyUpdate reports an update that changed the entry's RemoteEntry
+// and nothing else, as a sync stamp does. CommitTsNs is the filer's, new on
+// every stored update.
+func isStampOnlyUpdate(message *filer_pb.EventNotification) bool {
+	if proto.Equal(message.OldEntry.GetRemoteEntry(), message.NewEntry.GetRemoteEntry()) {
+		return false
+	}
+	oldEntry := proto.Clone(message.OldEntry).(*filer_pb.Entry)
+	newEntry := proto.Clone(message.NewEntry).(*filer_pb.Entry)
+	oldEntry.RemoteEntry, newEntry.RemoteEntry = nil, nil
+	oldEntry.CommitTsNs, newEntry.CommitTsNs = 0, 0
+	return proto.Equal(oldEntry, newEntry)
+}
+
 // liveRemoteEntry returns the RemoteEntry showing the entry's object is on the
 // remote, or nil when it never got there. The event's own is not enough: a
 // chmod right after a write is logged before the sync has uploaded the write

weed/command/filer_remote_sync_dir_test.go

@@ -2206,6 +2206,74 @@ func TestMetadataUpdateKeepsTheStoredStampOnNilResult(t *testing.T) {
 	}
 }
 
+// TestTouchedEntryIsStampedOnce: a touch, or an open with O_TRUNC of an
+// empty file as lock files get, moves the mtime past RemoteMtime and leaves
+// the content alone. The metadata-only update stamps the entry with the same
+// RemoteEntry, and the stamp is an update event of its own, which
+// shouldSendToRemote still reads as dirty because the mtime is past
+// RemoteMtime. Each stamp was processed into the next one, forever. An mtime
+// ahead of the sync's clock stays past every stamp it takes.
+func TestTouchedEntryIsStampedOnce(t *testing.T) {
+	const uploaded = 1786096669
+	for _, tc := range []struct {
+		name  string
+		mtime int64
+	}{
+		{name: "touched", mtime: uploaded + 3600},
+		{name: "touched into the future", mtime: time.Now().Add(24 * time.Hour).Unix()},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			const mountedDir = "/buckets"
+			const dir = "/buckets/b"
+			mountLoc := &remote_pb.RemoteStorageLocation{Name: "s3", Bucket: "bucket", Path: "/"}
+			synced := &filer_pb.RemoteEntry{StorageName: "s3", RemoteETag: `"d41d8cd98f00b204e9800998ecf8427e"`, RemoteMtime: uploaded, LastLocalSyncTsNs: uploaded*1e9 + 500}
+			before := &filer_pb.Entry{Name: "app.lock", Attributes: &filer_pb.FuseAttributes{Mtime: uploaded}, RemoteEntry: synced, CommitTsNs: uploaded * 1e9}
+			touched := proto.Clone(before).(*filer_pb.Entry)
+			touched.Attributes.Mtime = tc.mtime
+			touched.CommitTsNs++
+
+			remote := &recordingRemote{}
+			filerClient := &stubFilerClient{}
+			event := &filer_pb.SubscribeMetadataResponse{
+				Directory:         dir,
+				EventNotification: &filer_pb.EventNotification{OldEntry: before, NewParentPath: dir, NewEntry: touched},
+			}
+			stamps := 0
+			for round := 0; round < 5; round++ {
+				filerClient.updated = nil
+				if err := processUpdateEvent(filerClient, filerClient, "", remote, mountedDir, mountLoc, event); err != nil {
+					t.Fatal(err)
+				}
+				if filerClient.updated == nil {
+					break
+				}
+				stamps++
+				stamped := proto.Clone(filerClient.updated.Entry).(*filer_pb.Entry)
+				// the filer gives every stored update a new commit timestamp
+				stamped.CommitTsNs++
+				if !shouldSendToRemote(stamped) {
+					t.Fatal("precondition: the stamped entry still reads dirty, its mtime is past RemoteMtime")
+				}
+				// the filer logs the stamp as the next update of the entry
+				event = &filer_pb.SubscribeMetadataResponse{
+					Directory: dir,
+					EventNotification: &filer_pb.EventNotification{
+						OldEntry:      proto.Clone(filerClient.updated.Condition.Clauses[0].ExpectedEntry).(*filer_pb.Entry),
+						NewParentPath: dir,
+						NewEntry:      stamped,
+					},
+				}
+			}
+			if stamps != 1 {
+				t.Errorf("stamped the entry %d times, want 1: the touch is stamped, the stamp's own event is not", stamps)
+			}
+			if len(remote.writes) != 0 {
+				t.Errorf("writes = %+v, want none: the content did not change", remote.writes)
+			}
+		})
+	}
+}
+
 // TestReshapeCrossMountRename covers an update event with a path outside
 // the mount. The subscription delivers a rename when either path is under
 // the mount, and an update of the /etc/remote directory entry itself through