From c3d13c60e91d797efe29b4c8a3d6d8e5359fc39c Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sat, 10 Oct 2026 22:32:23 +0800 Subject: [PATCH] filer: fail writes when the existing-entry lookup fails (#11699) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * filer: fail writes when the existing-entry lookup fails CreateEntry only returned a FindEntry error for exclusive creates; a regular write treated a transient store error as "entry not found" and, on upsert stores, replaced the entry without loading its old chunks — leaving them out of the cleanup process. Return any lookup error except ErrNotFound for both paths. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: test that a lookup failure fails a regular write Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/filer/filer.go | 6 +++--- weed/filer/filer_inode_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/weed/filer/filer.go b/weed/filer/filer.go index a78d90691..bee16acdf 100644 --- a/weed/filer/filer.go +++ b/weed/filer/filer.go @@ -339,9 +339,9 @@ func (f *Filer) CreateEntry(ctx context.Context, entry *Entry, existing *Entry, if oldEntry == nil { var findErr error oldEntry, findErr = f.FindEntry(ctx, entry.FullPath) - if o_excl && findErr != nil && !errors.Is(findErr, filer_pb.ErrNotFound) { - // An exclusive create cannot decide whether the path exists when - // the lookup itself failed; proceeding would upsert over it. + if findErr != nil && !errors.Is(findErr, filer_pb.ErrNotFound) { + // A create cannot decide whether the path exists when the lookup + // itself failed; proceeding would upsert over it. return fmt.Errorf("find entry %s: %w", entry.FullPath, findErr) } } diff --git a/weed/filer/filer_inode_test.go b/weed/filer/filer_inode_test.go index 0c6b5db69..b00c1ec7e 100644 --- a/weed/filer/filer_inode_test.go +++ b/weed/filer/filer_inode_test.go @@ -265,3 +265,28 @@ func TestUpdateEntryBackfillsMissingLegacyInode(t *testing.T) { assert.NotZero(t, stored.Attr.Inode) assert.NotEqual(t, uint64(1), stored.Attr.Inode) } + +// A transient store error must fail the write instead of being mistaken for a +// missing entry: on upsert stores proceeding would replace the entry without +// loading the old chunks, orphaning them from cleanup. +func TestCreateEntryFailsWhenLookupErrors(t *testing.T) { + f, store := newTestFilerWithStubStore() + path := util.FullPath("/dir/file.txt") + require.NoError(t, store.InsertEntry(context.Background(), &Entry{ + FullPath: path, + Content: []byte("existing"), + })) + store.findErr = errors.New("transient store failure") + + entry := &Entry{ + FullPath: path, + Attr: Attr{Mode: 0o644}, + Content: []byte("overwrite"), + } + require.Error(t, f.CreateEntry(context.Background(), entry, nil, false, false, nil, false, f.MaxFilenameLength)) + + store.findErr = nil + stored, findErr := store.FindEntry(context.Background(), path) + require.NoError(t, findErr) + assert.Equal(t, "existing", string(stored.Content)) +}