diff --git a/weed/server/filer_grpc_server_rename.go b/weed/server/filer_grpc_server_rename.go index 7fd6e95c3..a4bd917a0 100644 --- a/weed/server/filer_grpc_server_rename.go +++ b/weed/server/filer_grpc_server_rename.go @@ -2,6 +2,7 @@ package weed_server import ( "context" + "errors" "fmt" "maps" "path/filepath" @@ -159,11 +160,18 @@ func (event metadataEvent) notify(f *filer.Filer, ctx context.Context, signature if event.deleteOldRemote { // a child path is not locked by its directory's rename, so another // rename may have put a remote-only entry at the old key meanwhile; - // the store is asked directly, a lazy fetch would revive the old key - if current, err := f.Store.FindEntry(ctx, event.oldEntry.FullPath); err == nil && current.IsInRemoteOnly() { + // the store is asked directly, a lazy fetch would revive the old key. + // When in doubt the object stays: an extra object loses nothing. + current, findErr := f.Store.FindEntry(ctx, event.oldEntry.FullPath) + switch { + case findErr != nil && !errors.Is(findErr, filer_pb.ErrNotFound): + glog.WarningfCtx(ctx, "keep remote object of renamed %s: look up its old path: %v", event.oldEntry.FullPath, findErr) + case findErr == nil && current.IsInRemoteOnly(): glog.V(0).InfofCtx(ctx, "keep remote object of renamed %s: a remote-only entry has taken its path", event.oldEntry.FullPath) - } else if err := f.DeleteFromRemote(ctx, event.oldEntry); err != nil { - glog.WarningfCtx(ctx, "delete remote object of renamed %s: %v", event.oldEntry.FullPath, err) + default: + if err := f.DeleteFromRemote(ctx, event.oldEntry); err != nil { + glog.WarningfCtx(ctx, "delete remote object of renamed %s: %v", event.oldEntry.FullPath, err) + } } } f.NotifyUpdateEvent(ctx, event.oldEntry, event.newEntry, event.deleteChunks, false, signatures) diff --git a/weed/server/filer_grpc_server_rename_remote_test.go b/weed/server/filer_grpc_server_rename_remote_test.go index 970ae980d..7c325cceb 100644 --- a/weed/server/filer_grpc_server_rename_remote_test.go +++ b/weed/server/filer_grpc_server_rename_remote_test.go @@ -304,7 +304,13 @@ func TestDeferredOldKeyDeleteKeepsAKeyRetakenSince(t *testing.T) { event.notify(server.filer, context.Background(), nil) assert.Empty(t, client.recorded()) + // a lookup that fails says nothing about the path delete(store.entries, "/buckets/b/src/a.jpg") + store.findErr = errors.New("store unavailable") + event.notify(server.filer, context.Background(), nil) + assert.Empty(t, client.recorded()) + + store.findErr = nil event.notify(server.filer, context.Background(), nil) assert.Equal(t, []string{"delete origin/src/a.jpg"}, client.recorded()) } diff --git a/weed/server/filer_grpc_server_rename_test.go b/weed/server/filer_grpc_server_rename_test.go index 2c36c9785..71fc92e01 100644 --- a/weed/server/filer_grpc_server_rename_test.go +++ b/weed/server/filer_grpc_server_rename_test.go @@ -31,6 +31,7 @@ type renameTestStore struct { deleteErr error listErr error // simulates a transient store/RPC failure from a directory listing findDelay time.Duration // optional: widen check-then-act windows in tests + findErr error // simulates a failing entry lookup } func newRenameTestStore() *renameTestStore { @@ -77,6 +78,9 @@ func (s *renameTestStore) FindEntry(_ context.Context, p util.FullPath) (*filer. s.mu.Lock() defer s.mu.Unlock() s.findCalls[string(p)]++ + if s.findErr != nil { + return nil, s.findErr + } entry, found := s.entries[string(p)] if !found { return nil, filer_pb.ErrNotFound