From 8e87924a95e332a577d5f38ca9411724db372a96 Mon Sep 17 00:00:00 2001 From: Peter Date: Thu, 5 Mar 2026 17:16:26 +0000 Subject: [PATCH] filer: propagate lazy metadata deletes to remote mounts Delete operations now call the remote backend for mounted remote-only entries before removing filer metadata, keeping remote state aligned and preserving retry semantics on remote failures. Made-with: Cursor --- weed/filer/filer_delete_entry.go | 4 + weed/filer/filer_lazy_remote.go | 42 ++++++ weed/filer/filer_lazy_remote_test.go | 202 ++++++++++++++++++++++++++- 3 files changed, 246 insertions(+), 2 deletions(-) diff --git a/weed/filer/filer_delete_entry.go b/weed/filer/filer_delete_entry.go index 77d9e34a7..c688be017 100644 --- a/weed/filer/filer_delete_entry.go +++ b/weed/filer/filer_delete_entry.go @@ -130,6 +130,10 @@ func (f *Filer) doDeleteEntryMetaAndData(ctx context.Context, entry *Entry, shou glog.V(3).InfofCtx(ctx, "deleting entry %v, delete chunks: %v", entry.FullPath, shouldDeleteChunks) + if remoteDeletionErr := f.maybeDeleteFromRemote(ctx, entry); remoteDeletionErr != nil { + return remoteDeletionErr + } + if storeDeletionErr := f.Store.DeleteOneEntry(ctx, entry); storeDeletionErr != nil { return fmt.Errorf("filer store delete: %w", storeDeletionErr) } diff --git a/weed/filer/filer_lazy_remote.go b/weed/filer/filer_lazy_remote.go index 76f48a6fd..87fb9adfc 100644 --- a/weed/filer/filer_lazy_remote.go +++ b/weed/filer/filer_lazy_remote.go @@ -114,3 +114,45 @@ func (f *Filer) maybeLazyFetchFromRemote(ctx context.Context, p util.FullPath) ( } return result.entry, nil } + +func (f *Filer) maybeDeleteFromRemote(ctx context.Context, entry *Entry) error { + if entry == nil || f.RemoteStorage == nil { + return nil + } + + mountDir, remoteLoc := f.RemoteStorage.FindMountDirectory(entry.FullPath) + if remoteLoc == nil { + return nil + } + + client, _, found := f.RemoteStorage.FindRemoteStorageClient(entry.FullPath) + if !found { + return nil + } + + objectLoc := MapFullPathToRemoteStorageLocation(mountDir, remoteLoc, entry.FullPath) + + if entry.IsDirectory() { + if err := client.RemoveDirectory(objectLoc); err != nil { + if errors.Is(err, remote_storage.ErrRemoteObjectNotFound) { + return nil + } + return fmt.Errorf("remove remote directory %s: %w", entry.FullPath, err) + } + return nil + } + + if !entry.IsInRemoteOnly() { + return nil + } + + if err := client.DeleteFile(objectLoc); err != nil { + if errors.Is(err, remote_storage.ErrRemoteObjectNotFound) { + return nil + } + return fmt.Errorf("delete remote file %s: %w", entry.FullPath, err) + } + + glog.V(3).InfofCtx(ctx, "maybeDeleteFromRemote: deleted %s from remote", entry.FullPath) + return nil +} diff --git a/weed/filer/filer_lazy_remote_test.go b/weed/filer/filer_lazy_remote_test.go index 2c5dcd60f..a316ad0bf 100644 --- a/weed/filer/filer_lazy_remote_test.go +++ b/weed/filer/filer_lazy_remote_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "os" "sync" "testing" "time" @@ -94,6 +95,11 @@ func (s *stubFilerStore) DeleteEntry(_ context.Context, p util.FullPath) error { type stubRemoteClient struct { statResult *filer_pb.RemoteEntry statErr error + deleteErr error + removeErr error + + deleteCalls []*remote_pb.RemoteStorageLocation + removeCalls []*remote_pb.RemoteStorageLocation } func (c *stubRemoteClient) StatFile(*remote_pb.RemoteStorageLocation) (*filer_pb.RemoteEntry, error) { @@ -108,14 +114,28 @@ func (c *stubRemoteClient) ReadFile(*remote_pb.RemoteStorageLocation, int64, int func (c *stubRemoteClient) WriteDirectory(*remote_pb.RemoteStorageLocation, *filer_pb.Entry) error { return nil } -func (c *stubRemoteClient) RemoveDirectory(*remote_pb.RemoteStorageLocation) error { return nil } +func (c *stubRemoteClient) RemoveDirectory(loc *remote_pb.RemoteStorageLocation) error { + c.removeCalls = append(c.removeCalls, &remote_pb.RemoteStorageLocation{ + Name: loc.Name, + Bucket: loc.Bucket, + Path: loc.Path, + }) + return c.removeErr +} func (c *stubRemoteClient) WriteFile(*remote_pb.RemoteStorageLocation, *filer_pb.Entry, io.Reader) (*filer_pb.RemoteEntry, error) { return nil, nil } func (c *stubRemoteClient) UpdateFileMetadata(*remote_pb.RemoteStorageLocation, *filer_pb.Entry, *filer_pb.Entry) error { return nil } -func (c *stubRemoteClient) DeleteFile(*remote_pb.RemoteStorageLocation) error { return nil } +func (c *stubRemoteClient) DeleteFile(loc *remote_pb.RemoteStorageLocation) error { + c.deleteCalls = append(c.deleteCalls, &remote_pb.RemoteStorageLocation{ + Name: loc.Name, + Bucket: loc.Bucket, + Path: loc.Path, + }) + return c.deleteErr +} func (c *stubRemoteClient) ListBuckets() ([]*remote_storage.Bucket, error) { return nil, nil } func (c *stubRemoteClient) CreateBucket(string) error { return nil } func (c *stubRemoteClient) DeleteBucket(string) error { return nil } @@ -368,3 +388,181 @@ func TestFindEntry_LazyFetchOnMiss(t *testing.T) { require.NotNil(t, entry2) assert.Equal(t, uint64(999), entry2.FileSize) } + +func TestDeleteEntryMetaAndData_RemoteOnlyFileDeletesRemoteAndMetadata(t *testing.T) { + const storageType = "stub_lazy_delete_file" + stub := &stubRemoteClient{} + defer registerStubMaker(t, storageType, stub)() + + conf := &remote_pb.RemoteConf{Name: "deletestore", Type: storageType} + rs := NewFilerRemoteStorage() + rs.storageNameToConf[conf.Name] = conf + rs.mapDirectoryToRemoteStorage("/buckets/mybucket", &remote_pb.RemoteStorageLocation{ + Name: "deletestore", + Bucket: "mybucket", + Path: "/", + }) + + store := newStubFilerStore() + filePath := util.FullPath("/buckets/mybucket/file.txt") + store.entries[string(filePath)] = &Entry{ + FullPath: filePath, + Attr: Attr{ + Mtime: time.Unix(1700000000, 0), + Crtime: time.Unix(1700000000, 0), + Mode: 0644, + FileSize: 64, + }, + Remote: &filer_pb.RemoteEntry{RemoteMtime: 1700000000, RemoteSize: 64}, + } + f := newTestFiler(t, store, rs) + + err := f.DeleteEntryMetaAndData(context.Background(), filePath, false, false, false, false, nil, 0) + require.NoError(t, err) + + _, findErr := store.FindEntry(context.Background(), filePath) + require.ErrorIs(t, findErr, filer_pb.ErrNotFound) + require.Len(t, stub.deleteCalls, 1) + assert.Equal(t, "deletestore", stub.deleteCalls[0].Name) + assert.Equal(t, "mybucket", stub.deleteCalls[0].Bucket) + assert.Equal(t, "/file.txt", stub.deleteCalls[0].Path) +} + +func TestDeleteEntryMetaAndData_RemoteOnlyFileNotUnderMountSkipsRemoteDelete(t *testing.T) { + const storageType = "stub_lazy_delete_not_under_mount" + stub := &stubRemoteClient{} + defer registerStubMaker(t, storageType, stub)() + + conf := &remote_pb.RemoteConf{Name: "notundermount", Type: storageType} + rs := NewFilerRemoteStorage() + rs.storageNameToConf[conf.Name] = conf + rs.mapDirectoryToRemoteStorage("/buckets/mybucket", &remote_pb.RemoteStorageLocation{ + Name: "notundermount", + Bucket: "mybucket", + Path: "/", + }) + + store := newStubFilerStore() + filePath := util.FullPath("/no/mount/file.txt") + store.entries[string(filePath)] = &Entry{ + FullPath: filePath, + Attr: Attr{ + Mtime: time.Unix(1700000000, 0), + Crtime: time.Unix(1700000000, 0), + Mode: 0644, + FileSize: 99, + }, + Remote: &filer_pb.RemoteEntry{RemoteMtime: 1700000000, RemoteSize: 99}, + } + f := newTestFiler(t, store, rs) + + err := f.DeleteEntryMetaAndData(context.Background(), filePath, false, false, false, false, nil, 0) + require.NoError(t, err) + require.Len(t, stub.deleteCalls, 0) +} + +func TestDeleteEntryMetaAndData_RemoteDeleteNotFoundStillDeletesMetadata(t *testing.T) { + const storageType = "stub_lazy_delete_notfound" + stub := &stubRemoteClient{deleteErr: remote_storage.ErrRemoteObjectNotFound} + defer registerStubMaker(t, storageType, stub)() + + conf := &remote_pb.RemoteConf{Name: "deletenotfound", Type: storageType} + rs := NewFilerRemoteStorage() + rs.storageNameToConf[conf.Name] = conf + rs.mapDirectoryToRemoteStorage("/buckets/mybucket", &remote_pb.RemoteStorageLocation{ + Name: "deletenotfound", + Bucket: "mybucket", + Path: "/", + }) + + store := newStubFilerStore() + filePath := util.FullPath("/buckets/mybucket/notfound.txt") + store.entries[string(filePath)] = &Entry{ + FullPath: filePath, + Attr: Attr{ + Mtime: time.Unix(1700000000, 0), + Crtime: time.Unix(1700000000, 0), + Mode: 0644, + FileSize: 23, + }, + Remote: &filer_pb.RemoteEntry{RemoteMtime: 1700000000, RemoteSize: 23}, + } + f := newTestFiler(t, store, rs) + + err := f.DeleteEntryMetaAndData(context.Background(), filePath, false, false, false, false, nil, 0) + require.NoError(t, err) + _, findErr := store.FindEntry(context.Background(), filePath) + require.ErrorIs(t, findErr, filer_pb.ErrNotFound) +} + +func TestDeleteEntryMetaAndData_RemoteDeleteErrorKeepsMetadata(t *testing.T) { + const storageType = "stub_lazy_delete_error" + stub := &stubRemoteClient{deleteErr: errors.New("remote delete failed")} + defer registerStubMaker(t, storageType, stub)() + + conf := &remote_pb.RemoteConf{Name: "deleteerr", Type: storageType} + rs := NewFilerRemoteStorage() + rs.storageNameToConf[conf.Name] = conf + rs.mapDirectoryToRemoteStorage("/buckets/mybucket", &remote_pb.RemoteStorageLocation{ + Name: "deleteerr", + Bucket: "mybucket", + Path: "/", + }) + + store := newStubFilerStore() + filePath := util.FullPath("/buckets/mybucket/error.txt") + store.entries[string(filePath)] = &Entry{ + FullPath: filePath, + Attr: Attr{ + Mtime: time.Unix(1700000000, 0), + Crtime: time.Unix(1700000000, 0), + Mode: 0644, + FileSize: 77, + }, + Remote: &filer_pb.RemoteEntry{RemoteMtime: 1700000000, RemoteSize: 77}, + } + f := newTestFiler(t, store, rs) + + err := f.DeleteEntryMetaAndData(context.Background(), filePath, false, false, false, false, nil, 0) + require.Error(t, err) + stored, findErr := store.FindEntry(context.Background(), filePath) + require.NoError(t, findErr) + require.NotNil(t, stored) +} + +func TestDeleteEntryMetaAndData_DirectoryUnderMountDeletesRemoteDirectory(t *testing.T) { + const storageType = "stub_lazy_delete_dir" + stub := &stubRemoteClient{} + defer registerStubMaker(t, storageType, stub)() + + conf := &remote_pb.RemoteConf{Name: "dirstore", Type: storageType} + rs := NewFilerRemoteStorage() + rs.storageNameToConf[conf.Name] = conf + rs.mapDirectoryToRemoteStorage("/buckets/mybucket", &remote_pb.RemoteStorageLocation{ + Name: "dirstore", + Bucket: "mybucket", + Path: "/", + }) + + store := newStubFilerStore() + dirPath := util.FullPath("/buckets/mybucket/dir") + store.entries[string(dirPath)] = &Entry{ + FullPath: dirPath, + Attr: Attr{ + Mtime: time.Unix(1700000000, 0), + Crtime: time.Unix(1700000000, 0), + Mode: os.ModeDir | 0755, + }, + } + f := newTestFiler(t, store, rs) + + err := f.doDeleteEntryMetaAndData(context.Background(), store.entries[string(dirPath)], false, false, nil) + require.NoError(t, err) + + require.Len(t, stub.removeCalls, 1) + assert.Equal(t, "dirstore", stub.removeCalls[0].Name) + assert.Equal(t, "mybucket", stub.removeCalls[0].Bucket) + assert.Equal(t, "/dir", stub.removeCalls[0].Path) + _, findErr := store.FindEntry(context.Background(), dirPath) + require.ErrorIs(t, findErr, filer_pb.ErrNotFound) +}