From afa813ff883d16814227291a5d86537ff6f5e05a Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 12 Nov 2026 17:57:58 -0800 Subject: [PATCH] mount: keep a dirty handle's base when a same-content file replaces it A replacement with the base's chunks but another inode counted as a metadata-only change, so the dirty handle adopted it as its base and the next conditional flush overwrote the new file. Co-Authored-By: Claude Opus 5.5 --- weed/mount/weedfs.go | 4 +- .../weedfs_invalidate_open_handle_test.go | 65 ++++++++++--------- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/weed/mount/weedfs.go b/weed/mount/weedfs.go index b0671d79dd..14d0424fe5 100644 --- a/weed/mount/weedfs.go +++ b/weed/mount/weedfs.go @@ -1049,10 +1049,10 @@ func (wfs *WFS) invalidateOpenFileHandle(invalidation meta_cache.EntryInvalidati // against an approximate base) leaves them valid. A dirty handle likewise // keeps its diverged entry unless foreign content supersedes it. contentChanged := base == nil || !sameEntryContent(candidate, base) - if contentChanged && fh.dirtyMetadata && base != nil { + if fh.dirtyMetadata && base != nil && (contentChanged || !sameContentFile(candidate, base)) { // An unmergeable change or a replaced file keeps the writes; the // flush reports the conflict. - if sameContentFile(candidate, base) && fh.mergePeerContent(candidate, base) { + if contentChanged && sameContentFile(candidate, base) && fh.mergePeerContent(candidate, base) { fh.adoptPeerBase(proto.Clone(candidate).(*filer_pb.Entry)) fh.advanceEntryVersion(candidateTsNs, 0) } diff --git a/weed/mount/weedfs_invalidate_open_handle_test.go b/weed/mount/weedfs_invalidate_open_handle_test.go index 5fb2be5263..03a2daec55 100644 --- a/weed/mount/weedfs_invalidate_open_handle_test.go +++ b/weed/mount/weedfs_invalidate_open_handle_test.go @@ -2235,37 +2235,44 @@ func TestPeerTruncateWaitsForBufferedWrites(t *testing.T) { // A peer replacing the file at the handle's path (another inode) leaves the // dirty handle's writes and base alone instead of installing the newcomer. func TestPeerReplacementKeepsDirtyHandle(t *testing.T) { - wfs := newInvalidateTestWFS(t) + for _, name := range []string{"new-content", "same-content"} { + t.Run(name, func(t *testing.T) { + wfs := newInvalidateTestWFS(t) - inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) - fh, _, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, flushConditionEntry(1), 0, 0) - fh.AddChunks([]*filer_pb.FileChunk{{FileId: "1,0300000001", Offset: 2, Size: 1, ModifiedTsNs: 3}}) - fh.dirtyMetadata = true - pagesBefore := fh.dirtyPages + inode := wfs.inodeToPath.Lookup(util.FullPath("/dir/file"), time.Now().Unix(), false, false, 0, false) + fh, _, _, _ := wfs.fhMap.AcquireFileHandle(wfs, inode, flushConditionEntry(1), 0, 0) + fh.AddChunks([]*filer_pb.FileChunk{{FileId: "1,0300000001", Offset: 2, Size: 1, ModifiedTsNs: 3}}) + fh.dirtyMetadata = true + pagesBefore := fh.dirtyPages - replacement := flushConditionEntry(7) - replacement.Attributes.Inode = 99 - resp := &filer_pb.SubscribeMetadataResponse{ - Directory: "/dir", - TsNs: 1500, - EventNotification: &filer_pb.EventNotification{ - OldEntry: flushConditionEntry(1), - NewEntry: replacement, - NewParentPath: "/dir", - }, - } - if err := wfs.metaCache.ApplyMetadataResponse(context.Background(), resp, meta_cache.SubscriberMetadataResponseApplyOptions); err != nil { - t.Fatalf("apply: %v", err) - } - wfs.metaCache.WaitForEntryInvalidations() + replacement := flushConditionEntry(7) + if name == "same-content" { + replacement = flushConditionEntry(1) + } + replacement.Attributes.Inode = 99 + resp := &filer_pb.SubscribeMetadataResponse{ + Directory: "/dir", + TsNs: 1500, + EventNotification: &filer_pb.EventNotification{ + OldEntry: flushConditionEntry(1), + NewEntry: replacement, + NewParentPath: "/dir", + }, + } + if err := wfs.metaCache.ApplyMetadataResponse(context.Background(), resp, meta_cache.SubscriberMetadataResponseApplyOptions); err != nil { + t.Fatalf("apply: %v", err) + } + wfs.metaCache.WaitForEntryInvalidations() - if fh.dirtyPages != pagesBefore { - t.Fatal("dirty pages destroyed by a replacement") - } - if got := chunkFileKeys(fh.GetEntry().GetEntry()); len(got) != 2 || got[0] != 1 { - t.Fatalf("handle chunks %v, want its own", got) - } - if base := fh.baseEntry.Load(); base.GetAttributes().GetInode() != 42 { - t.Fatalf("base %v moved to the replacement", base) + if fh.dirtyPages != pagesBefore { + t.Fatal("dirty pages destroyed by a replacement") + } + if got := chunkFileKeys(fh.GetEntry().GetEntry()); len(got) != 2 || got[0] != 1 { + t.Fatalf("handle chunks %v, want its own", got) + } + if base := fh.baseEntry.Load(); base.GetAttributes().GetInode() != 42 { + t.Fatalf("base %v moved to the replacement", base) + } + }) } }