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.
2026-09-19 06:31:54 +00:00 · 2026-08-25 10:23:00 -07:00
parent b77431c142
commit 70c3adb983
3 changed files with 424 additions and 27 deletions
@@ -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))
+}
@@ -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())
+	}
+}
@@ -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)
 }