diff --git a/weed/command/filer_remote_sync_dir.go b/weed/command/filer_remote_sync_dir.go index 8c660f70a..f7b1b0e39 100644 --- a/weed/command/filer_remote_sync_dir.go +++ b/weed/command/filer_remote_sync_dir.go @@ -247,9 +247,11 @@ func processDeleteEvent( // The object at the key may have been replaced since the delete: a // rename copies another entry's content onto it, a rewrite uploads // over it. Match the recorded etag before deleting so only the - // object the delete meant goes away. A changed object with no entry - // at the path is an upload that finished after the delete, and goes. - if oldETag := message.OldEntry.GetRemoteEntry().GetRemoteETag(); oldETag != "" && current != nil { + // object the delete meant goes away. That holds with no entry at the + // path too: a rename copies before its new entry becomes visible. An + // upload that finished after the delete is kept the same way, which + // leaks that object rather than risking the only copy of a rename. + if oldETag := message.OldEntry.GetRemoteEntry().GetRemoteETag(); oldETag != "" { if live, err := client.StatFile(dest); err == nil { if live.RemoteETag != "" && live.RemoteETag != oldETag { glog.V(0).Infof("keep %s: the object changed since the delete", remote_storage.FormatLocation(dest)) diff --git a/weed/command/filer_remote_sync_dir_test.go b/weed/command/filer_remote_sync_dir_test.go index ba1e09122..ae9d7f7e5 100644 --- a/weed/command/filer_remote_sync_dir_test.go +++ b/weed/command/filer_remote_sync_dir_test.go @@ -1053,17 +1053,17 @@ func TestDeleteEventAbsentRemoteObjectIsSuccess(t *testing.T) { } }) - t.Run("a changed object with no entry at the path still goes", func(t *testing.T) { - // an upload that finished after the delete leaves a newer object that - // no entry describes; keeping it would leave it on the remote forever + t.Run("a changed object with no entry at the path yet is kept", func(t *testing.T) { + // a directory rename copies a child onto the key before its new entry + // becomes visible; the copy may be the content's only remaining home deleted := proto.Clone(resp).(*filer_pb.SubscribeMetadataResponse) deleted.EventNotification.OldEntry.RemoteEntry = &filer_pb.RemoteEntry{RemoteETag: `"orig"`, RemoteSize: 10} - remote := &recordingRemote{objects: map[string][]byte{"/b/vol/state.db.tmp": []byte("late")}, statETag: `"late"`} + remote := &recordingRemote{objects: map[string][]byte{"/b/vol/state.db.tmp": []byte("copied")}, statETag: `"copy"`} if err := processDeleteEvent(&stubFilerClient{}, remote, mountedDir, mountLoc, deleted); err != nil { t.Fatalf("err = %v", err) } - if len(remote.deletes) != 1 { - t.Errorf("deletes = %+v, want one", remote.deletes) + if len(remote.deletes) != 0 { + t.Errorf("deletes = %+v, want none", remote.deletes) } })