From 1826a5d222c522856d6977b1e9be9b47a322b641 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 16 Jun 2026 14:59:13 -0700 Subject: [PATCH] fix(mount): pin rebuild entries by their own inode, not inodeToPath (#9993) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isLocalOnlyEntry resolved the pinned-child check through inodeToPath. A kernel Forget drops the path→inode mapping once the lookup count reaches zero, but an async writeback flush — and the file handle, still in fhMap during the drain — is keyed by inode and outlives that mapping. Between Release dispatching the async flush and the flush reaching the filer, a Forget could unpin an in-flight create, so a concurrent directory rebuild would wipe it and the file would ENOENT until the flush lands and the cache refreshes. Key the check off the inode the store entry already carries (createFile stamps it into the placeholder), so the pin no longer depends on a mapping Forget can remove. --- weed/mount/meta_cache/meta_cache.go | 6 +++--- weed/mount/meta_cache/meta_cache_build_test.go | 16 ++++++++++++---- weed/mount/weedfs.go | 12 ++++++++---- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/weed/mount/meta_cache/meta_cache.go b/weed/mount/meta_cache/meta_cache.go index d866c13f5..29055daad 100644 --- a/weed/mount/meta_cache/meta_cache.go +++ b/weed/mount/meta_cache/meta_cache.go @@ -33,7 +33,7 @@ type MetaCache struct { isCachedFn func(fullpath util.FullPath) bool invalidateFunc func(fullpath util.FullPath, entry *filer_pb.Entry) onDirectoryUpdate func(dir util.FullPath) - pinnedChildFn func(util.FullPath) bool // a child a rebuild must not drop (local-only, not yet on the filer); nil disables + pinnedChildFn func(*filer.Entry) bool // a child a rebuild must not drop (local-only, not yet on the filer); nil disables visitGroup singleflight.Group // deduplicates concurrent EnsureVisited calls for the same path applyCh chan metadataApplyRequest applyDone chan struct{} @@ -342,7 +342,7 @@ func (mc *MetaCache) DeleteFolderChildren(ctx context.Context, fp util.FullPath) // SetPinnedChildFn installs a predicate reporting whether a child holds // local-only state a rebuild must not discard. See deleteFolderChildrenForRebuild. -func (mc *MetaCache) SetPinnedChildFn(fn func(util.FullPath) bool) { +func (mc *MetaCache) SetPinnedChildFn(fn func(*filer.Entry) bool) { mc.pinnedChildFn = fn } @@ -360,7 +360,7 @@ func (mc *MetaCache) deleteFolderChildrenForRebuild(ctx context.Context, dirPath } var pinned []*filer.Entry if _, err := mc.localStore.ListDirectoryEntries(ctx, dirPath, "", true, math.MaxInt64, func(entry *filer.Entry) (bool, error) { - if mc.pinnedChildFn(entry.FullPath) { + if mc.pinnedChildFn(entry) { pinned = append(pinned, entry) } return true, nil diff --git a/weed/mount/meta_cache/meta_cache_build_test.go b/weed/mount/meta_cache/meta_cache_build_test.go index e48f41b9f..ead21dae2 100644 --- a/weed/mount/meta_cache/meta_cache_build_test.go +++ b/weed/mount/meta_cache/meta_cache_build_test.go @@ -473,11 +473,19 @@ func TestEnsureVisitedPreservesLocalOnlyEntry(t *testing.T) { mc, _, _, _ := newTestMetaCache(t, map[util.FullPath]bool{"/": true}) defer mc.Shutdown() - // The mount pins the un-flushed create (open dirty handle / pending flush). - mc.SetPinnedChildFn(func(p util.FullPath) bool { return p == "/dir/pending.txt" }) + // The mount pins the un-flushed create (open dirty handle / pending flush), + // keyed off the inode the entry carries so a kernel Forget that dropped the + // path→inode mapping cannot unpin an in-flight create. + mc.SetPinnedChildFn(func(e *filer.Entry) bool { return e.Attr.Inode == 42 }) // A deferred local create lands before the rebuild; /dir is not yet cached. - insertCacheEntry(t, mc, "/dir/pending.txt") + // It carries its allocated inode, as createFile's placeholder does. + if err := mc.InsertEntry(context.Background(), &filer.Entry{ + FullPath: "/dir/pending.txt", + Attr: filer.Attr{Crtime: time.Unix(1, 0), Mtime: time.Unix(1, 0), Mode: 0100644, FileSize: 1, Inode: 42}, + }); err != nil { + t.Fatalf("insert pending entry: %v", err) + } // A concurrent rebuild lists the filer, whose snapshot pre-dates the // un-flushed create, so it returns only the already-persisted sibling. @@ -522,7 +530,7 @@ func TestEnsureVisitedDropsUnpinnedStaleEntry(t *testing.T) { mc, _, _, _ := newTestMetaCache(t, map[util.FullPath]bool{"/": true}) defer mc.Shutdown() - mc.SetPinnedChildFn(func(util.FullPath) bool { return false }) + mc.SetPinnedChildFn(func(*filer.Entry) bool { return false }) // A stale child sits in the cache; the filer no longer has it. insertCacheEntry(t, mc, "/dir/stale.txt") diff --git a/weed/mount/weedfs.go b/weed/mount/weedfs.go index bc4bc222f..9b7665183 100644 --- a/weed/mount/weedfs.go +++ b/weed/mount/weedfs.go @@ -550,16 +550,20 @@ func (wfs *WFS) maybeReadEntry(inode uint64) (path util.FullPath, fh *FileHandle return } -// isLocalOnlyEntry reports whether fullpath holds local-only state not yet on the +// isLocalOnlyEntry reports whether entry holds local-only state not yet on the // filer — an open handle with dirty metadata, or a pending async flush. A // directory rebuild refills from a filer listing that omits such an entry, so it // must be preserved across the wipe; this is the same signal lookupEntry trusts // over a filer ErrNotFound for deferred creates. -func (wfs *WFS) isLocalOnlyEntry(fullpath util.FullPath) bool { - inode, found := wfs.inodeToPath.GetInode(fullpath) - if !found { +// +// Keyed off the inode the entry carries, not inodeToPath: a kernel Forget can +// drop the path→inode mapping while an async writeback flush is still in flight, +// and the entry must stay pinned until that flush reaches the filer. +func (wfs *WFS) isLocalOnlyEntry(entry *filer.Entry) bool { + if entry == nil || entry.Attr.Inode == 0 { return false } + inode := entry.Attr.Inode if fh, fhFound := wfs.fhMap.FindFileHandle(inode); fhFound && fh.dirtyMetadata { return true }