diff --git a/weed/command/filer_remote_sync_dir.go b/weed/command/filer_remote_sync_dir.go index 20489c3507..4a9c950d56 100644 --- a/weed/command/filer_remote_sync_dir.go +++ b/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 diff --git a/weed/command/filer_remote_sync_dir_test.go b/weed/command/filer_remote_sync_dir_test.go index 0392946a09..a57c8c48b8 100644 --- a/weed/command/filer_remote_sync_dir_test.go +++ b/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