From 70c3adb9831db5f136e948d487a95c8767687109 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 25 Aug 2026 10:23:00 -0700 Subject: [PATCH] volume: stop read-only volumes from pinning .idx and .sdx (#10950) A read-only or cloud-tiered volume loads a SortedFileNeedleMap, which held both its .idx and its .sdx open for the life of the process. On a server with ~600K tiered volumes that is 1.2M descriptors before a single read, enough to exhaust the fd limit and take the listeners down. The .dat is not the problem: a tiered volume serves it from the remote backend. Neither index file is needed except while a lookup is in flight, so borrow them from a bounded process-wide pool instead. An idle volume now holds zero descriptors; a busy one keeps its handles hot rather than paying an open() per needle. Reads borrow O_RDONLY, so a volume on a read-only mount answers lookups that previously failed at load. Sync tracks whether a tombstone was appended, which also drops the fsync-per-volume storm at shutdown. --- weed/storage/needle_map_file_pool.go | 112 +++++++++++++ weed/storage/needle_map_file_pool_test.go | 188 ++++++++++++++++++++++ weed/storage/needle_map_sorted_file.go | 151 +++++++++++++---- 3 files changed, 424 insertions(+), 27 deletions(-) create mode 100644 weed/storage/needle_map_file_pool.go create mode 100644 weed/storage/needle_map_file_pool_test.go diff --git a/weed/storage/needle_map_file_pool.go b/weed/storage/needle_map_file_pool.go new file mode 100644 index 000000000..a82808644 --- /dev/null +++ b/weed/storage/needle_map_file_pool.go @@ -0,0 +1,112 @@ +package storage + +import ( + "os" + "sync" + + "github.com/hashicorp/golang-lru/v2/simplelru" +) + +// Read-only volumes — cloud-tiered ones above all — outnumber writable ones by +// orders of magnitude on a large server, and each one used to pin its .idx and +// .sdx descriptors for the life of the process. At ~600K volumes per server +// that alone exhausts any fd limit. Neither file is needed except while a +// lookup is in flight, so SortedFileNeedleMap borrows them from this bounded +// pool: an idle volume holds no descriptor at all, while a busy one keeps its +// handles hot instead of paying an open() per needle. +const maxPooledIndexFiles = 1024 + +type pooledFile struct { + file *os.File + refs int + dropped bool // left the pool; close once the last borrower is done +} + +type indexFilePool struct { + sync.Mutex + lru *simplelru.LRU[string, *pooledFile] +} + +var pooledIndexFiles = newIndexFilePool(maxPooledIndexFiles) + +func newIndexFilePool(size int) *indexFilePool { + p := &indexFilePool{} + // simplelru is not thread safe on its own; every access below holds + // p.Mutex, and this eviction callback runs inline under it. + p.lru, _ = simplelru.NewLRU(size, func(_ string, f *pooledFile) { + f.dropped = true + f.closeIfUnused() + }) + return p +} + +// closeIfUnused closes a handle that has left the pool once no borrower is +// still reading through it. Callers hold indexFilePool.Mutex. +func (f *pooledFile) closeIfUnused() { + if f.dropped && f.refs == 0 && f.file != nil { + f.file.Close() + f.file = nil + } +} + +// Writable and read-only handles for the same path are pooled separately so a +// read never depends on the .idx being openable for write — a volume served off +// a read-only mount still answers lookups. +func poolKey(name string, writable bool) string { + if writable { + return name + "\x00rw" + } + return name +} + +// borrow hands out an open handle for name, reusing the pooled one when there +// is one. The caller must release it exactly once. +func (p *indexFilePool) borrow(name string, writable bool) (*pooledFile, error) { + key := poolKey(name, writable) + + p.Lock() + if f, found := p.lru.Get(key); found { + f.refs++ + p.Unlock() + return f, nil + } + p.Unlock() + + flag := os.O_RDONLY + if writable { + flag = os.O_RDWR + } + // Opened outside the lock: a cold open blocks on disk, and holding a + // process-wide mutex across it would serialize every volume's lookups. + file, err := os.OpenFile(name, flag, 0644) + if err != nil { + return nil, err + } + + p.Lock() + defer p.Unlock() + if f, found := p.lru.Get(key); found { // another borrower won the race + f.refs++ + file.Close() + return f, nil + } + f := &pooledFile{file: file, refs: 1} + p.lru.Add(key, f) + return f, nil +} + +func (p *indexFilePool) release(f *pooledFile) { + p.Lock() + defer p.Unlock() + f.refs-- + f.closeIfUnused() +} + +// discard forgets the pooled handles for name, so a later rename or delete of +// that path cannot be served from a descriptor on the old inode. +func (p *indexFilePool) discard(name string) { + p.Lock() + defer p.Unlock() + p.lru.Remove(poolKey(name, false)) + p.lru.Remove(poolKey(name, true)) +} diff --git a/weed/storage/needle_map_file_pool_test.go b/weed/storage/needle_map_file_pool_test.go new file mode 100644 index 000000000..9f1f15e35 --- /dev/null +++ b/weed/storage/needle_map_file_pool_test.go @@ -0,0 +1,188 @@ +package storage + +import ( + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/storage/needle" + "github.com/seaweedfs/seaweedfs/weed/storage/super_block" + . "github.com/seaweedfs/seaweedfs/weed/storage/types" +) + +// openIndexFilesUnder counts the process's descriptors on .idx/.sdx files under +// dir, reading /proc/self/fd where it exists and falling back to lsof. Returns +// false when neither is available, so the caller can skip. +func openIndexFilesUnder(t *testing.T, dir string) (int, bool) { + t.Helper() + resolved, err := filepath.EvalSymlinks(dir) + if err != nil { + resolved = dir + } + prefix := resolved + string(os.PathSeparator) + + if entries, err := os.ReadDir("/proc/self/fd"); err == nil { + count := 0 + for _, e := range entries { + target, err := os.Readlink(filepath.Join("/proc/self/fd", e.Name())) + if err != nil { + continue // raced with a close + } + if isIndexFileUnder(target, prefix) { + count++ + } + } + return count, true + } + + out, err := exec.Command("lsof", "-p", strconv.Itoa(os.Getpid()), "-F", "n").Output() + if err != nil { + return 0, false + } + count := 0 + for _, line := range strings.Split(string(out), "\n") { + if strings.HasPrefix(line, "n") && isIndexFileUnder(line[1:], prefix) { + count++ + } + } + return count, true +} + +func isIndexFileUnder(target, prefix string) bool { + return strings.HasPrefix(target, prefix) && + (strings.HasSuffix(target, ".idx") || strings.HasSuffix(target, ".sdx")) +} + +// TestSortedFileNeedleMap_HoldsNoDescriptors is the regression guard for +// issue #10937: a volume server with hundreds of thousands of read-only or +// cloud-tiered volumes ran out of descriptors because every one of them pinned +// its .idx and .sdx for the life of the process. An idle read-only volume must +// hold neither. +func TestSortedFileNeedleMap_HoldsNoDescriptors(t *testing.T) { + if _, ok := openIndexFilesUnder(t, t.TempDir()); !ok { + t.Skip("cannot enumerate open descriptors on this platform") + } + + t.Run("readonly", func(t *testing.T) { + dir := t.TempDir() + v, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0) + if err != nil { + t.Fatalf("new volume: %v", err) + } + for i := 1; i <= 8; i++ { + if _, _, _, err := v.writeNeedle2(newRandomNeedle(uint64(i)), true, false, false); err != nil { + t.Fatalf("write needle %d: %v", i, err) + } + } + v.PersistReadOnly(true, true) + v.Close() + + v, err = NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0) + if err != nil { + t.Fatalf("reload volume: %v", err) + } + defer v.Close() + nm, isSorted := v.nm.(*SortedFileNeedleMap) + if !isSorted { + t.Fatalf("read-only volume should load a SortedFileNeedleMap, got %T", v.nm) + } + assertNoIndexFds(t, dir, "after load") + + // A lookup borrows a handle and hands it straight back; only the pool's + // bounded cache keeps it, and dropping that leaves nothing behind. + if _, found := nm.Get(Uint64ToNeedleId(3)); !found { + t.Fatal("needle 3 not found after reload") + } + pooledIndexFiles.discard(nm.dbFileName) + assertNoIndexFds(t, dir, "after a lookup") + }) + + t.Run("remote", func(t *testing.T) { + b := newLocalDirBackend(t) + registerTestBackend(t, b) + dir := t.TempDir() + const vid = needle.VolumeId(9) + tierUpVolume(t, dir, vid, b) + + v := reloadVolume(t, dir, vid) + defer v.Close() + if !v.HasRemoteFile() { + t.Fatal("reloaded volume is not tiered to remote") + } + assertNoIndexFds(t, dir, "after load") + }) +} + +func assertNoIndexFds(t *testing.T, dir, when string) { + t.Helper() + if got, _ := openIndexFilesUnder(t, dir); got != 0 { + t.Fatalf("%s the volume holds %d .idx/.sdx descriptors under %s, want 0", when, got, dir) + } +} + +// TestIndexFilePool_EvictWhileBorrowed locks in that an eviction does not pull +// a descriptor out from under an in-flight reader. +func TestIndexFilePool_EvictWhileBorrowed(t *testing.T) { + dir := t.TempDir() + first := filepath.Join(dir, "first") + second := filepath.Join(dir, "second") + for _, name := range []string{first, second} { + if err := os.WriteFile(name, []byte(filepath.Base(name)), 0644); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + + pool := newIndexFilePool(1) + borrowed, err := pool.borrow(first, false) + if err != nil { + t.Fatalf("borrow first: %v", err) + } + + // Pushes the single pool slot over, evicting the entry still in use. + other, err := pool.borrow(second, false) + if err != nil { + t.Fatalf("borrow second: %v", err) + } + pool.release(other) + + buf := make([]byte, len("first")) + if _, err := borrowed.file.ReadAt(buf, 0); err != nil { + t.Fatalf("read through evicted-but-borrowed handle: %v", err) + } + if string(buf) != "first" { + t.Fatalf("read %q, want %q", buf, "first") + } + + pool.release(borrowed) + if borrowed.file != nil { + t.Fatal("evicted handle was not closed once the last borrower released it") + } +} + +// TestIndexFilePool_DiscardClosesIdle covers the path Close/Destroy rely on: +// once a volume is unmounted nothing may keep serving reads from its old inode. +func TestIndexFilePool_DiscardClosesIdle(t *testing.T) { + dir := t.TempDir() + name := filepath.Join(dir, "idx") + if err := os.WriteFile(name, []byte("x"), 0644); err != nil { + t.Fatalf("write: %v", err) + } + + pool := newIndexFilePool(4) + f, err := pool.borrow(name, false) + if err != nil { + t.Fatalf("borrow: %v", err) + } + pool.release(f) + + pool.discard(name) + if f.file != nil { + t.Fatal("discard left the pooled handle open") + } + if pool.lru.Len() != 0 { + t.Fatalf("discard left %d entries in the pool", pool.lru.Len()) + } +} diff --git a/weed/storage/needle_map_sorted_file.go b/weed/storage/needle_map_sorted_file.go index 8945dd46a..ebf7d78fe 100644 --- a/weed/storage/needle_map_sorted_file.go +++ b/weed/storage/needle_map_sorted_file.go @@ -2,54 +2,68 @@ package storage import ( "fmt" + "io" "os" + "sync" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding" + "github.com/seaweedfs/seaweedfs/weed/storage/idx" "github.com/seaweedfs/seaweedfs/weed/storage/needle" "github.com/seaweedfs/seaweedfs/weed/storage/needle_map" . "github.com/seaweedfs/seaweedfs/weed/storage/types" ) +// SortedFileNeedleMap backs every read-only volume, which on a tiered cluster +// is nearly every volume. It deliberately keeps no *os.File of its own: +// .idx and .sdx are borrowed from pooledIndexFiles per operation, so a volume +// nobody is reading costs zero descriptors. See needle_map_file_pool.go. type SortedFileNeedleMap struct { - baseNeedleMapper - baseFileName string - dbFile *os.File - dbFileSize int64 + mapMetric + baseFileName string + indexFileName string + dbFileName string + dbFileSize int64 + + indexFileAccessLock sync.Mutex + indexFileOffset int64 + indexNeedsSync bool } func NewSortedFileNeedleMap(indexBaseFileName string, indexFile *os.File, version needle.Version) (m *SortedFileNeedleMap, err error) { - m = &SortedFileNeedleMap{baseFileName: indexBaseFileName} - m.indexFile = indexFile - fileName := indexBaseFileName + ".sdx" - if !isSortedFileFresh(fileName, indexFile) { - glog.V(0).Infof("Start to Generate %s from %s", fileName, indexFile.Name()) + m = &SortedFileNeedleMap{ + baseFileName: indexBaseFileName, + indexFileName: indexFile.Name(), + dbFileName: indexBaseFileName + ".sdx", + } + if !isSortedFileFresh(m.dbFileName, indexFile) { + glog.V(0).Infof("Start to Generate %s from %s", m.dbFileName, indexFile.Name()) erasure_coding.WriteSortedFileFromIdx(indexBaseFileName, ".sdx") - glog.V(0).Infof("Finished Generating %s from %s", fileName, indexFile.Name()) + glog.V(0).Infof("Finished Generating %s from %s", m.dbFileName, indexFile.Name()) } - glog.V(1).Infof("Opening %s...", fileName) - if m.dbFile, err = os.OpenFile(indexBaseFileName+".sdx", os.O_RDWR, 0); err != nil { - return + dbStat, err := os.Stat(m.dbFileName) + if err != nil { + return nil, fmt.Errorf("stat %s: %v", m.dbFileName, err) } - dbStat, _ := m.dbFile.Stat() m.dbFileSize = dbStat.Size() // Seed indexFileOffset so Delete() appends tombstones to the tail of // .idx instead of overwriting from offset 0 and clobbering existing // records with tombstones for unrelated keys. indexStat, statErr := indexFile.Stat() if statErr != nil { - _ = m.dbFile.Close() return nil, fmt.Errorf("stat %s: %v", indexFile.Name(), statErr) } m.indexFileOffset = indexStat.Size() glog.V(1).Infof("Loading %s...", indexFile.Name()) mm, indexLoadError := newNeedleMapMetricFromIndexFile(indexFile, version) if indexLoadError != nil { - _ = m.dbFile.Close() return nil, indexLoadError } m.mapMetric = *mm + // Everything past the load walk goes through the pool, so hand the + // caller's descriptor back instead of holding it for the volume's life. + indexFile.Close() return } @@ -71,7 +85,13 @@ func isSortedFileFresh(dbFileName string, indexFile *os.File) bool { } func (m *SortedFileNeedleMap) Get(key NeedleId) (element *needle_map.NeedleValue, ok bool) { - offset, size, err := erasure_coding.SearchNeedleFromSortedIndex(m.dbFile, m.dbFileSize, key, nil) + f, err := pooledIndexFiles.borrow(m.dbFileName, false) + if err != nil { + glog.Warningf("open %s: %v", m.dbFileName, err) + return &needle_map.NeedleValue{Key: key}, false + } + offset, size, err := erasure_coding.SearchNeedleFromSortedIndex(f.file, m.dbFileSize, key, nil) + pooledIndexFiles.release(f) ok = err == nil return &needle_map.NeedleValue{Key: key, Offset: offset, Size: size}, ok @@ -83,7 +103,13 @@ func (m *SortedFileNeedleMap) Put(key NeedleId, offset Offset, size Size) error func (m *SortedFileNeedleMap) Delete(key NeedleId, offset Offset) error { - _, size, err := erasure_coding.SearchNeedleFromSortedIndex(m.dbFile, m.dbFileSize, key, nil) + f, err := pooledIndexFiles.borrow(m.dbFileName, true) + if err != nil { + return err + } + defer pooledIndexFiles.release(f) + + _, size, err := erasure_coding.SearchNeedleFromSortedIndex(f.file, m.dbFileSize, key, nil) if err != nil { if err == erasure_coding.NotFoundError { @@ -100,25 +126,96 @@ func (m *SortedFileNeedleMap) Delete(key NeedleId, offset Offset) error { if err := m.appendToIndexFile(key, offset, TombstoneFileSize); err != nil { return err } - _, _, err = erasure_coding.SearchNeedleFromSortedIndex(m.dbFile, m.dbFileSize, key, erasure_coding.MarkNeedleDeleted) + _, _, err = erasure_coding.SearchNeedleFromSortedIndex(f.file, m.dbFileSize, key, erasure_coding.MarkNeedleDeleted) return err } +func (m *SortedFileNeedleMap) appendToIndexFile(key NeedleId, offset Offset, size Size) error { + f, err := pooledIndexFiles.borrow(m.indexFileName, true) + if err != nil { + return err + } + defer pooledIndexFiles.release(f) + + bytes := needle_map.ToBytes(key, offset, size) + + m.indexFileAccessLock.Lock() + defer m.indexFileAccessLock.Unlock() + written, err := f.file.WriteAt(bytes, m.indexFileOffset) + if err == nil { + m.indexFileOffset += int64(written) + m.indexNeedsSync = true + } + return err +} + +// IndexFileSize answers from the offset the appends maintain rather than a +// stat: the heartbeat asks every volume for this on every beat, and a +// read-only volume's .idx only ever grows through appendToIndexFile. +func (m *SortedFileNeedleMap) IndexFileSize() uint64 { + m.indexFileAccessLock.Lock() + defer m.indexFileAccessLock.Unlock() + return uint64(m.indexFileOffset) +} + +// Sync flushes tombstones appended by Delete. A read-only volume that has never +// been deleted from — the overwhelming majority — opens nothing here, so +// shutting down a server holding hundreds of thousands of them costs no fsyncs. +func (m *SortedFileNeedleMap) Sync() error { + m.indexFileAccessLock.Lock() + defer m.indexFileAccessLock.Unlock() + if !m.indexNeedsSync { + return nil + } + f, err := pooledIndexFiles.borrow(m.indexFileName, true) + if err != nil { + return err + } + defer pooledIndexFiles.release(f) + if err := f.file.Sync(); err != nil { + return err + } + m.indexNeedsSync = false + return nil +} + +func (m *SortedFileNeedleMap) ReadIndexEntry(n int64) (key NeedleId, offset Offset, size Size, err error) { + var f *pooledFile + if f, err = pooledIndexFiles.borrow(m.indexFileName, false); err != nil { + return + } + defer pooledIndexFiles.release(f) + + bytes := make([]byte, NeedleMapEntrySize) + var readCount int + if readCount, err = f.file.ReadAt(bytes, n*NeedleMapEntrySize); err != nil { + if err == io.EOF { + if readCount == NeedleMapEntrySize { + err = nil + } + } + if err != nil { + return + } + } + key, offset, size = idx.IdxFileEntry(bytes) + return +} + func (m *SortedFileNeedleMap) Close() { if m == nil { return } - if m.indexFile != nil { - m.indexFile.Close() - } - if m.dbFile != nil { - m.dbFile.Close() - } + // Drop the pooled handles too: the caller may be about to rename or remove + // these paths, and a descriptor left behind would keep answering reads from + // the old inode. + pooledIndexFiles.discard(m.indexFileName) + pooledIndexFiles.discard(m.dbFileName) } func (m *SortedFileNeedleMap) Destroy() error { m.Close() - os.Remove(m.indexFile.Name()) - return os.Remove(m.baseFileName + ".sdx") + os.Remove(m.indexFileName) + return os.Remove(m.dbFileName) }