From 3e679e925e7e731ca7c40283ed670a62e7857142 Mon Sep 17 00:00:00 2001 From: yi111 <153097222+Yi-111-a@users.noreply.github.com> Date: Sat, 3 Oct 2026 09:13:59 +0800 Subject: [PATCH] filer: keep generated inodes inside the positive signed 64-bit range (#11567) AsInode derives inodes from HashStringToLong, which is uniform over int64, so roughly half of the derived values land above math.MaxInt64 once they are converted to uint64. The Elasticsearch store indexes Entry.Attr.Inode as a signed long, so those values are rejected with HTTP 400 and the metadata entry is never written, which the filer then retries forever. Fold the sign bit off in one place, util.NormalizeInode, and route both derivation sites through it: FullPath.AsInode (path plus creation time) and the hard-link branch in ensureEntryInode (HardLinkId hash). Masking keeps the other 63 hash bits, so distinct paths still get distinct inodes, and it applies identically to the FUSE mount, which derives the same value. Co-authored-by: Yi-111-a <34116709+0-xiaosu@users.noreply.github.com> --- weed/filer/filer_inode.go | 2 +- weed/filer/filer_inode_test.go | 50 +++++++++++++++++++++++++++++++++- weed/util/fullpath.go | 18 +++++++++++- weed/util/fullpath_test.go | 33 ++++++++++++++++++++++ 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/weed/filer/filer_inode.go b/weed/filer/filer_inode.go index e5c4c5e26..d1a2eea81 100644 --- a/weed/filer/filer_inode.go +++ b/weed/filer/filer_inode.go @@ -19,7 +19,7 @@ func (f *Filer) ensureEntryInode(entry *Entry) { entry.Attr.Crtime = time.Now() } if len(entry.HardLinkId) > 0 { - entry.Attr.Inode = uint64(util.HashStringToLong(string(entry.HardLinkId))) + entry.Attr.Inode = util.NormalizeInode(uint64(util.HashStringToLong(string(entry.HardLinkId)))) return } entry.Attr.Inode = entry.FullPath.AsInode(entry.Attr.Crtime.Unix()) diff --git a/weed/filer/filer_inode_test.go b/weed/filer/filer_inode_test.go index 575332ca0..0c6b5db69 100644 --- a/weed/filer/filer_inode_test.go +++ b/weed/filer/filer_inode_test.go @@ -3,6 +3,8 @@ package filer import ( "context" "errors" + "fmt" + "math" "os" "testing" "time" @@ -51,10 +53,56 @@ func TestEnsureEntryInodeSharesAcrossHardLinks(t *testing.T) { // Every link to the same target resolves to one inode, independent of path // or creation time. - assert.Equal(t, uint64(util.HashStringToLong(string(hardLinkId))), a.Attr.Inode) + assert.Equal(t, util.NormalizeInode(uint64(util.HashStringToLong(string(hardLinkId)))), a.Attr.Inode) assert.Equal(t, a.Attr.Inode, b.Attr.Inode) } +// TestEnsureEntryInodeFitsSignedLong pins the invariant that every generated +// inode is storable: the filer hands Attr.Inode to the backing store verbatim, +// and the Elasticsearch store indexes it as a signed `long`. Roughly half of +// the unsigned hash space sits above math.MaxInt64, so a store that rejects +// those values used to fail the metadata write for half of all entries. +func TestEnsureEntryInodeFitsSignedLong(t *testing.T) { + f := &Filer{} + crtime := time.Unix(1700000000, 0) + + seen := make(map[uint64]string) + for i := 0; i < 5000; i++ { + fullPath := util.FullPath(fmt.Sprintf("/topics/.system/log/2026-10-02/entry-%d", i)) + entry := &Entry{FullPath: fullPath, Attr: Attr{Crtime: crtime}} + f.ensureEntryInode(entry) + + if entry.Attr.Inode > math.MaxInt64 { + t.Fatalf("ensureEntryInode(%q) = %d, above math.MaxInt64", fullPath, entry.Attr.Inode) + } + // Folding must not collapse distinct paths onto one inode. + if other, ok := seen[entry.Attr.Inode]; ok { + t.Fatalf("ensureEntryInode(%q) collided with %q on inode %d", fullPath, other, entry.Attr.Inode) + } + seen[entry.Attr.Inode] = string(fullPath) + } +} + +// TestEnsureEntryInodeHardLinkFitsSignedLong covers the hard-link branch, which +// hashes HardLinkId instead of the path and so has no path-derived crtime term. +func TestEnsureEntryInodeHardLinkFitsSignedLong(t *testing.T) { + f := &Filer{} + crtime := time.Unix(1700000000, 0) + + for i := 0; i < 5000; i++ { + entry := &Entry{ + FullPath: util.FullPath(fmt.Sprintf("/links/target-%d.txt", i)), + Attr: Attr{Crtime: crtime}, + HardLinkId: NewHardLinkId(), + } + f.ensureEntryInode(entry) + + if entry.Attr.Inode > math.MaxInt64 { + t.Fatalf("ensureEntryInode(%q) = %d, above math.MaxInt64", entry.FullPath, entry.Attr.Inode) + } + } +} + func newTestFilerWithStubStore() (*Filer, *stubFilerStore) { store := newStubFilerStore() f := NewFiler(pb.ServerDiscovery{}, nil, "", "", "", "", "", 255, nil) diff --git a/weed/util/fullpath.go b/weed/util/fullpath.go index 6e4a5f248..866592f95 100644 --- a/weed/util/fullpath.go +++ b/weed/util/fullpath.go @@ -1,6 +1,7 @@ package util import ( + "math" "path" "strings" "unicode/utf8" @@ -87,11 +88,26 @@ func (fp FullPath) Child(name string) FullPath { return FullPath(dir + "/" + noPrefix) } +// NormalizeInode folds a derived inode into the positive signed 64-bit range +// by dropping the sign bit. Every filer store has to persist the value as-is, +// and some of them serialize it as a signed 64-bit integer: the Elasticsearch +// store indexes Entry.Attr.Inode as a `long`, so a value above math.MaxInt64 +// is rejected with HTTP 400 and the whole entry is lost. HashStringToLong is +// uniform over int64, so roughly half of the derived inodes used to land above +// that limit and failed to be written. +// +// Masking keeps the remaining 63 hash bits, so distinct paths still derive +// distinct inodes; subtracting or folding into a smaller modulus would only +// spread the same collisions more thinly. +func NormalizeInode(inode uint64) uint64 { + return inode & math.MaxInt64 +} + // AsInode an in-memory only inode representation func (fp FullPath) AsInode(unixTime int64) uint64 { inode := uint64(HashStringToLong(string(fp))) inode = inode + uint64(unixTime)*37 - return inode + return NormalizeInode(inode) } // split, but skipping the root diff --git a/weed/util/fullpath_test.go b/weed/util/fullpath_test.go index 95856d532..908291019 100644 --- a/weed/util/fullpath_test.go +++ b/weed/util/fullpath_test.go @@ -1,6 +1,8 @@ package util import ( + "fmt" + "math" "strings" "testing" "unicode/utf8" @@ -173,3 +175,34 @@ func TestJoinPreservesBackslash(t *testing.T) { t.Fatalf("Join: got %q want %q", got, want) } } + +// TestAsInodeFitsSignedLong asserts the derived inode stays inside the positive +// signed 64-bit range for a batch of paths and creation times. The filer stores +// this value verbatim and the Elasticsearch store indexes it as a `long`, so an +// inode above math.MaxInt64 is rejected and the entry is lost. HashStringToLong +// is uniform over int64, so about half of the paths here would have exceeded +// the limit before NormalizeInode was introduced. +func TestAsInodeFitsSignedLong(t *testing.T) { + unixTimes := []int64{0, 1, 1700000000, 2000000000} + + seen := make(map[uint64]FullPath) + for _, unixTime := range unixTimes { + for i := 0; i < 2000; i++ { + fp := FullPath(fmt.Sprintf("/topics/.system/log/2026-10-02/entry-%d", i)) + inode := fp.AsInode(unixTime) + + if inode > math.MaxInt64 { + t.Fatalf("AsInode(%q, %d) = %d, above math.MaxInt64", fp, unixTime, inode) + } + if inode == 0 { + t.Fatalf("AsInode(%q, %d) = 0, which reads as \"unset\" to the filer", fp, unixTime) + } + // Distinct paths must still get distinct inodes: masking one bit + // off the hash keeps that property. + if other, ok := seen[inode]; ok && other != fp { + t.Fatalf("AsInode(%q) collided with %q on inode %d", fp, other, inode) + } + seen[inode] = fp + } + } +}