From 3e2f615444c89cea61d0b20cd793936d39f7319b Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Sat, 29 Aug 2026 03:21:59 +0000 Subject: [PATCH 1/3] test: add synchronized file lock and deletion queue coverage Signed-off-by: Perfloop Agent --- Makefile | 45 ++++ storage/deletion/queue_bench_test.go | 264 +++++++++++++++++++++++ storage/deletion/queue_test.go | 11 +- storage/fd/filelockmanager_bench_test.go | 67 ++++++ 4 files changed, 381 insertions(+), 6 deletions(-) create mode 100644 storage/deletion/queue_bench_test.go create mode 100644 storage/fd/filelockmanager_bench_test.go diff --git a/Makefile b/Makefile index e667ab61..c16614af 100644 --- a/Makefile +++ b/Makefile @@ -267,6 +267,27 @@ build-bench-cache-service-delete: proto @test -n "$(PERFLOOP_BUILD_OUTPUT_DIR)" || { echo "PERFLOOP_BUILD_OUTPUT_DIR is required"; exit 1; } @mkdir -p "$(PERFLOOP_BUILD_OUTPUT_DIR)" @cd server && CGO_ENABLED=1 CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" go test $(LDFLAGS) -c -o "$(PERFLOOP_BUILD_OUTPUT_DIR)/cache-service-delete.test" ./service +# Compile the standalone file-lock benchmarks without linking the RocksDB-backed +# FD cache. Set FILELOCK_BENCH_BINARY to the desired output path. Resolve it +# before changing directories so relative paths are rooted at the repository. +FILELOCK_BENCH_BINARY ?= +FILELOCK_BENCH_OUTPUT := $(abspath $(FILELOCK_BENCH_BINARY)) +.PHONY: bench-filelock +bench-filelock: + @test -n "$(FILELOCK_BENCH_BINARY)" || { echo "FILELOCK_BENCH_BINARY is required"; exit 1; } + @mkdir -p "$(dir $(FILELOCK_BENCH_OUTPUT))" + @cd storage/fd && go test -c -o "$(FILELOCK_BENCH_OUTPUT)" filelockmanager.go filelockmanager_test.go filelockmanager_bench_test.go + +# Compile deletion-queue benchmarks with the same RocksDB Cgo flags as storage tests. +# Set DELETION_BENCH_BINARY to the desired output path. Resolve it before changing +# directories so relative paths are rooted at the repository. +DELETION_BENCH_BINARY ?= +DELETION_BENCH_OUTPUT := $(abspath $(DELETION_BENCH_BINARY)) +.PHONY: bench-deletion +bench-deletion: + @test -n "$(DELETION_BENCH_BINARY)" || { echo "DELETION_BENCH_BINARY is required"; exit 1; } + @mkdir -p "$(dir $(DELETION_BENCH_OUTPUT))" + @cd storage/deletion && CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" go test -c -o "$(DELETION_BENCH_OUTPUT)" . # Compile the CacheService ListWithValues benchmark once so paired runs do not # include Go compilation. PERFLOOP_BUILD_OUTPUT_DIR is supplied by the benchmark runner. @@ -322,6 +343,25 @@ test-storage-wire: proto .PHONY: test-storage-reconcile test-storage-reconcile: proto @cd storage && CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" go test $(LDFLAGS) -v -timeout 60s -run '^TestCleaner' . +# FileLockManager has no RocksDB dependency, so test it directly without +# compiling the rest of the FD cache package. +.PHONY: test-filelock +test-filelock: + @echo "Running file lock manager tests..." + $(if $(TEST)$(TESTRUN),@echo "Filter: $(if $(TEST),$(TEST),$(TESTRUN))",) + @cd storage/fd && go test -v -timeout 60s $(TESTFLAGS) filelockmanager.go filelockmanager_test.go filelockmanager_bench_test.go + +.PHONY: test-filelock-race +test-filelock-race: + @echo "Running file lock manager tests with race detector..." + $(if $(TEST)$(TESTRUN),@echo "Filter: $(if $(TEST),$(TEST),$(TESTRUN))",) + @cd storage/fd && go test -race -v -timeout 60s $(TESTFLAGS) filelockmanager.go filelockmanager_test.go filelockmanager_bench_test.go + +.PHONY: test-deletion +test-deletion: + @echo "Running deletion queue tests..." + $(if $(TEST)$(TESTRUN),@echo "Filter: $(if $(TEST),$(TEST),$(TESTRUN))",) + @cd storage/deletion && CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" go test -v -timeout 60s $(TESTFLAGS) . .PHONY: test-client test-client: proto @@ -595,6 +635,9 @@ help: @echo " test - Run unit tests (server and client)" @echo " test-all - Run all tests (unit and integration)" @echo " test-server - Run server tests only" + @echo " test-filelock - Run file lock manager tests" + @echo " test-filelock-race - Run file lock manager tests with the race detector" + @echo " test-deletion - Run deletion queue tests" @echo " test-client - Run client tests only" @echo " test-coordinator - Run coordinator tests only" @echo " test-race - Run tests with race detector" @@ -618,6 +661,8 @@ help: @echo " build-bench-cache-topology-rpc - Compile the CacheService topology RPC benchmark (set PERFLOOP_BUILD_OUTPUT_DIR)" @echo " build-bench-cache-service-delete - Compile the CacheService delete benchmark (set PERFLOOP_BUILD_OUTPUT_DIR)" @echo " build-bench-cache-service-list-with-values - Compile the CacheService ListWithValues benchmark (set PERFLOOP_BUILD_OUTPUT_DIR)" + @echo " bench-filelock - Compile file lock benchmarks (set FILELOCK_BENCH_BINARY)" + @echo " bench-deletion - Compile deletion queue benchmarks (set DELETION_BENCH_BINARY)" @echo "" @echo " To run specific tests, use TEST or TESTRUN variable:" @echo " make test TEST=TestMyFunction - Run exact test name" diff --git a/storage/deletion/queue_bench_test.go b/storage/deletion/queue_bench_test.go new file mode 100644 index 00000000..e3d26e2b --- /dev/null +++ b/storage/deletion/queue_bench_test.go @@ -0,0 +1,264 @@ +// Copyright 2026 Tigris Data, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package deletion + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/rs/zerolog" + "github.com/tigrisdata/ocache/storage/fd" + "github.com/tigrisdata/ocache/storage/keys" + "github.com/tigrisdata/ocache/storage/metadata" +) + +const ( + deletionBenchmarkBatchSize = 1000 + deletionBenchmarkProcessInterval = time.Second + deletionBenchmarkRetryDelay = 30 * time.Second + + deletionBenchmarkCompletionPollInterval = 10 * time.Millisecond + deletionBenchmarkCompletionTimeout = 5 * time.Second +) + +func newLockedRetryQueue(b *testing.B) (*Queue, []string, func()) { + b.Helper() + + meta, err := metadata.NewMetaDB(b.TempDir(), 0, nil, nil) + if err != nil { + b.Fatal(err) + } + + queue := NewQueue(meta, Config{ + BatchSize: deletionBenchmarkBatchSize, + ProcessInterval: deletionBenchmarkProcessInterval, + PruneAge: time.Hour, + RetryDelay: deletionBenchmarkRetryDelay, + }) + + paths := make([]string, deletionBenchmarkBatchSize) + locks := make([]*sync.RWMutex, deletionBenchmarkBatchSize) + lockManager := fd.GetFileLockManager() + tmpDir := b.TempDir() + for i := range paths { + path := filepath.Join(tmpDir, fmt.Sprintf("locked-%04d", i)) + if err := os.WriteFile(path, nil, 0o644); err != nil { + b.Fatal(err) + } + + lock := lockManager.GetFileLock(path) + lock.RLock() + paths[i] = path + locks[i] = lock + } + + return queue, paths, func() { + for _, lock := range locks { + lock.RUnlock() + } + for _, path := range paths { + lockManager.RemoveFileLock(path) + } + meta.Close() + } +} + +func newUnlockedDeletionQueue(b *testing.B) (*Queue, []string, func()) { + b.Helper() + + meta, err := metadata.NewMetaDB(b.TempDir(), 0, nil, nil) + if err != nil { + b.Fatal(err) + } + + queue := NewQueue(meta, Config{ + BatchSize: deletionBenchmarkBatchSize, + ProcessInterval: deletionBenchmarkProcessInterval, + PruneAge: time.Hour, + RetryDelay: deletionBenchmarkRetryDelay, + }) + + paths := make([]string, deletionBenchmarkBatchSize) + tmpDir := b.TempDir() + for i := range paths { + path := filepath.Join(tmpDir, fmt.Sprintf("unlocked-%04d", i)) + if err := os.WriteFile(path, nil, 0o644); err != nil { + b.Fatal(err) + } + paths[i] = path + } + + return queue, paths, func() { + meta.Close() + } +} + +func enqueueDeletionBatch(b *testing.B, queue *Queue, paths []string) { + b.Helper() + + for _, path := range paths { + if err := queue.Add(path); err != nil { + b.Fatal(err) + } + } +} + +func queueEntryKey(b *testing.B, queue *Queue) []byte { + b.Helper() + + ro := metadata.CreateReadOptions(true, false) + defer ro.Destroy() + + it := queue.meta.Handle().NewIterator(ro) + defer it.Close() + + prefix := []byte(keys.DeletionQueuePrefix) + it.Seek(prefix) + if !it.ValidForPrefix(prefix) { + b.Fatal("queued deletion entry not found") + } + + key := it.Key() + defer key.Free() + value := it.Value() + defer value.Free() + return bytes.Clone(key.Data()) +} + +func queueEntryRemoved(queue *Queue, key []byte) (bool, error) { + ro := metadata.CreateReadOptions(false, false) + defer ro.Destroy() + + value, err := queue.meta.Handle().Get(ro, key) + if err != nil { + return false, err + } + defer value.Free() + return !value.Exists(), nil +} + +func waitForQueueEntryRemoval(b *testing.B, queue *Queue, key []byte) { + b.Helper() + + deadline := time.NewTimer(deletionBenchmarkCompletionTimeout) + defer deadline.Stop() + poll := time.NewTicker(deletionBenchmarkCompletionPollInterval) + defer poll.Stop() + + for { + removed, err := queueEntryRemoved(queue, key) + if err != nil { + b.Fatalf("check queued deletion entry: %v", err) + } + if removed { + return + } + + select { + case <-deadline.C: + b.Fatal("timed out waiting for queue processing loop") + case <-poll.C: + } + } +} + +func processQueueBatchThroughLoop(b *testing.B, queue *Queue, key []byte) { + b.Helper() + + queue.Start() + defer queue.Stop() + waitForQueueEntryRemoval(b, queue, key) +} + +func benchmarkQueueProcessingLoopLockedRetryBacklog(b *testing.B) { + b.Helper() + + b.StopTimer() + queue, paths, cleanup := newLockedRetryQueue(b) + defer cleanup() + enqueueDeletionBatch(b, queue, paths) + key := queueEntryKey(b, queue) + + b.StartTimer() + processQueueBatchThroughLoop(b, queue, key) + b.StopTimer() + + if queue.processed != 0 || queue.failed != deletionBenchmarkBatchSize { + b.Fatalf("processingLoop processed=%d failed=%d, want processed=0 failed=%d", queue.processed, queue.failed, deletionBenchmarkBatchSize) + } + if depth := queue.GetQueueDepth(); depth != deletionBenchmarkBatchSize { + b.Fatalf("processingLoop queue depth=%d, want %d", depth, deletionBenchmarkBatchSize) + } + for _, path := range paths { + if _, err := os.Stat(path); err != nil { + b.Fatalf("locked file removed: %v", err) + } + } +} + +func benchmarkQueueProcessingLoopUnlockedDeletionBacklog(b *testing.B) { + b.Helper() + + b.StopTimer() + queue, paths, cleanup := newUnlockedDeletionQueue(b) + defer cleanup() + enqueueDeletionBatch(b, queue, paths) + key := queueEntryKey(b, queue) + + b.StartTimer() + processQueueBatchThroughLoop(b, queue, key) + b.StopTimer() + + if queue.processed != deletionBenchmarkBatchSize || queue.failed != 0 { + b.Fatalf("processingLoop processed=%d failed=%d, want processed=%d failed=0", queue.processed, queue.failed, deletionBenchmarkBatchSize) + } + if depth := queue.GetQueueDepth(); depth != 0 { + b.Fatalf("processingLoop queue depth=%d, want 0", depth) + } + for _, path := range paths { + if _, err := os.Stat(path); !os.IsNotExist(err) { + b.Fatalf("file remained after processingLoop: %v", err) + } + } +} + +// BenchmarkQueueProcessingLoopLockedRetryBacklog measures a full timer-driven +// batch of 1,000 deletions that must be retried. Each path already has a read +// lock, so tryDelete reaches an existing GetFileLock entry before re-enqueuing +// it after 30 seconds. The benchmark waits for removal of an initial queue key, +// which happens in ProcessBatch's final atomic write after the entire batch has +// run, rather than sleeping for a fixed number of ticker intervals. +func BenchmarkQueueProcessingLoopLockedRetryBacklog(b *testing.B) { + level := zerolog.GlobalLevel() + zerolog.SetGlobalLevel(zerolog.Disabled) + defer zerolog.SetGlobalLevel(level) + + b.ReportAllocs() + for b.Loop() { + benchmarkQueueProcessingLoopLockedRetryBacklog(b) + b.StartTimer() + } +} + +// BenchmarkQueueProcessingLoopUnlockedDeletionBacklog measures one full +// timer-driven batch of 1,000 ordinary deletions. It uses the same one-second +// cadence, batch size, and retry delay as the locked-retry benchmark. Removal +// of the initial queue key synchronizes with ProcessBatch's final atomic write +// instead of adding a fixed-duration wait to the measured operation. +func BenchmarkQueueProcessingLoopUnlockedDeletionBacklog(b *testing.B) { + level := zerolog.GlobalLevel() + zerolog.SetGlobalLevel(zerolog.Disabled) + defer zerolog.SetGlobalLevel(level) + + b.ReportAllocs() + for b.Loop() { + benchmarkQueueProcessingLoopUnlockedDeletionBacklog(b) + b.StartTimer() + } +} diff --git a/storage/deletion/queue_test.go b/storage/deletion/queue_test.go index 90b5ba91..c6230f5a 100644 --- a/storage/deletion/queue_test.go +++ b/storage/deletion/queue_test.go @@ -174,12 +174,11 @@ func TestQueue_BackgroundProcessing(t *testing.T) { err = queue.Add(testFile) require.NoError(t, err) - // Wait for background processing - time.Sleep(200 * time.Millisecond) - - // Verify file is deleted - _, err = os.Stat(testFile) - require.True(t, os.IsNotExist(err), "file should be deleted by background processing") + // Wait for background processing to delete the file. + require.Eventually(t, func() bool { + _, err := os.Stat(testFile) + return os.IsNotExist(err) + }, 500*time.Millisecond, 10*time.Millisecond, "file should be deleted by background processing") } func TestQueue_LockedFile(t *testing.T) { diff --git a/storage/fd/filelockmanager_bench_test.go b/storage/fd/filelockmanager_bench_test.go new file mode 100644 index 00000000..15a58394 --- /dev/null +++ b/storage/fd/filelockmanager_bench_test.go @@ -0,0 +1,67 @@ +// Copyright 2026 Tigris Data, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package fd + +import ( + "sync/atomic" + "testing" +) + +// BenchmarkFileLockManager_GetFileLockHit measures a lookup for a path whose +// lock was registered by an earlier file operation. +func BenchmarkFileLockManager_GetFileLockHit(b *testing.B) { + const path = "/test/file.txt" + + manager := &FileLockManager{} + want := manager.GetFileLock(path) + + b.ReportAllocs() + for b.Loop() { + if got := manager.GetFileLock(path); got != want { + b.Fatal("GetFileLock returned a different lock for an existing path") + } + } +} + +// BenchmarkFileLockManager_GetFileLockMiss measures only the first lookup for +// a path. Resetting the manager while timing is stopped keeps every lookup on +// the miss path without charging RemoveFileLock to the benchmark. +func BenchmarkFileLockManager_GetFileLockMiss(b *testing.B) { + const path = "/test/file.txt" + + manager := &FileLockManager{} + + b.ReportAllocs() + for b.Loop() { + b.StopTimer() + manager = &FileLockManager{} + b.StartTimer() + + if lock := manager.GetFileLock(path); lock == nil { + b.Fatal("GetFileLock returned nil") + } + } +} + +func BenchmarkFileLockManager_GetFileLockHitParallel(b *testing.B) { + const path = "/test/file.txt" + + manager := &FileLockManager{} + want := manager.GetFileLock(path) + var differentLock atomic.Bool + + b.ReportAllocs() + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + if got := manager.GetFileLock(path); got != want { + differentLock.Store(true) + return + } + } + }) + if differentLock.Load() { + b.Fatal("GetFileLock returned a different lock for an existing path") + } +} From 71065f38367c462bf1f9d7e943fe9ca611c9bbcd Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Sat, 29 Aug 2026 03:22:04 +0000 Subject: [PATCH 2/3] perf: avoid mutex allocation on existing file-lock lookups Signed-off-by: Perfloop Agent --- storage/fd/filelockmanager.go | 10 ++++++++- storage/fd/filelockmanager_test.go | 35 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/storage/fd/filelockmanager.go b/storage/fd/filelockmanager.go index eeda6b9b..f2b7ad30 100644 --- a/storage/fd/filelockmanager.go +++ b/storage/fd/filelockmanager.go @@ -46,7 +46,15 @@ func GetFileLockManager() *FileLockManager { // access to writers (Write/Delete), which is exactly the behaviour we need for // raw files. func (flm *FileLockManager) GetFileLock(path string) *sync.RWMutex { - lock, _ := flm.fileLocks.LoadOrStore(path, &sync.RWMutex{}) + if lock, ok := flm.fileLocks.Load(path); ok { + return lock.(*sync.RWMutex) + } + + newLock := &sync.RWMutex{} + lock, loaded := flm.fileLocks.LoadOrStore(path, newLock) + if !loaded { + return newLock + } return lock.(*sync.RWMutex) } diff --git a/storage/fd/filelockmanager_test.go b/storage/fd/filelockmanager_test.go index bb7b87c4..97b0f45c 100644 --- a/storage/fd/filelockmanager_test.go +++ b/storage/fd/filelockmanager_test.go @@ -55,6 +55,41 @@ func TestFileLockManager_GetFileLock(t *testing.T) { } } +func TestFileLockManager_ConcurrentMissUsesOneLock(t *testing.T) { + const ( + path = "/test/file.txt" + workers = 100 + ) + + manager := &FileLockManager{} + locks := make([]*sync.RWMutex, workers) + start := make(chan struct{}) + + var wg sync.WaitGroup + wg.Add(workers) + for i := range workers { + go func() { + defer wg.Done() + <-start + locks[i] = manager.GetFileLock(path) + }() + } + close(start) + wg.Wait() + + if locks[0] == nil { + t.Fatal("GetFileLock returned nil") + } + if !manager.HasLock(path) { + t.Fatal("GetFileLock did not retain the lock") + } + for i, lock := range locks { + if lock != locks[0] { + t.Fatalf("GetFileLock returned a different lock to worker %d", i) + } + } +} + func TestFileLockManager_RemoveFileLock(t *testing.T) { // Reset singleton for testing lockManagerOnce = sync.Once{} From 215aa38ba6b1ec4af444777ab1ed9f11e8094673 Mon Sep 17 00:00:00 2001 From: Ovais Tariq Date: Wed, 9 Sep 2026 15:44:58 -0700 Subject: [PATCH 3/3] chore(fd): trim the benchmark scaffolding from the file-lock lookup change Keep the eight-line Load-then-LoadOrStore change, its concurrent-miss test, and the deletion-queue test's polling wait; drop the two benchmark files and the Makefile targets that existed only to run them. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01YCoN4Him5ZY4hZovVp37ra Signed-off-by: Ovais Tariq --- Makefile | 45 ---- storage/deletion/queue_bench_test.go | 264 ----------------------- storage/fd/filelockmanager_bench_test.go | 67 ------ 3 files changed, 376 deletions(-) delete mode 100644 storage/deletion/queue_bench_test.go delete mode 100644 storage/fd/filelockmanager_bench_test.go diff --git a/Makefile b/Makefile index c16614af..e667ab61 100644 --- a/Makefile +++ b/Makefile @@ -267,27 +267,6 @@ build-bench-cache-service-delete: proto @test -n "$(PERFLOOP_BUILD_OUTPUT_DIR)" || { echo "PERFLOOP_BUILD_OUTPUT_DIR is required"; exit 1; } @mkdir -p "$(PERFLOOP_BUILD_OUTPUT_DIR)" @cd server && CGO_ENABLED=1 CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" go test $(LDFLAGS) -c -o "$(PERFLOOP_BUILD_OUTPUT_DIR)/cache-service-delete.test" ./service -# Compile the standalone file-lock benchmarks without linking the RocksDB-backed -# FD cache. Set FILELOCK_BENCH_BINARY to the desired output path. Resolve it -# before changing directories so relative paths are rooted at the repository. -FILELOCK_BENCH_BINARY ?= -FILELOCK_BENCH_OUTPUT := $(abspath $(FILELOCK_BENCH_BINARY)) -.PHONY: bench-filelock -bench-filelock: - @test -n "$(FILELOCK_BENCH_BINARY)" || { echo "FILELOCK_BENCH_BINARY is required"; exit 1; } - @mkdir -p "$(dir $(FILELOCK_BENCH_OUTPUT))" - @cd storage/fd && go test -c -o "$(FILELOCK_BENCH_OUTPUT)" filelockmanager.go filelockmanager_test.go filelockmanager_bench_test.go - -# Compile deletion-queue benchmarks with the same RocksDB Cgo flags as storage tests. -# Set DELETION_BENCH_BINARY to the desired output path. Resolve it before changing -# directories so relative paths are rooted at the repository. -DELETION_BENCH_BINARY ?= -DELETION_BENCH_OUTPUT := $(abspath $(DELETION_BENCH_BINARY)) -.PHONY: bench-deletion -bench-deletion: - @test -n "$(DELETION_BENCH_BINARY)" || { echo "DELETION_BENCH_BINARY is required"; exit 1; } - @mkdir -p "$(dir $(DELETION_BENCH_OUTPUT))" - @cd storage/deletion && CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" go test -c -o "$(DELETION_BENCH_OUTPUT)" . # Compile the CacheService ListWithValues benchmark once so paired runs do not # include Go compilation. PERFLOOP_BUILD_OUTPUT_DIR is supplied by the benchmark runner. @@ -343,25 +322,6 @@ test-storage-wire: proto .PHONY: test-storage-reconcile test-storage-reconcile: proto @cd storage && CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" go test $(LDFLAGS) -v -timeout 60s -run '^TestCleaner' . -# FileLockManager has no RocksDB dependency, so test it directly without -# compiling the rest of the FD cache package. -.PHONY: test-filelock -test-filelock: - @echo "Running file lock manager tests..." - $(if $(TEST)$(TESTRUN),@echo "Filter: $(if $(TEST),$(TEST),$(TESTRUN))",) - @cd storage/fd && go test -v -timeout 60s $(TESTFLAGS) filelockmanager.go filelockmanager_test.go filelockmanager_bench_test.go - -.PHONY: test-filelock-race -test-filelock-race: - @echo "Running file lock manager tests with race detector..." - $(if $(TEST)$(TESTRUN),@echo "Filter: $(if $(TEST),$(TEST),$(TESTRUN))",) - @cd storage/fd && go test -race -v -timeout 60s $(TESTFLAGS) filelockmanager.go filelockmanager_test.go filelockmanager_bench_test.go - -.PHONY: test-deletion -test-deletion: - @echo "Running deletion queue tests..." - $(if $(TEST)$(TESTRUN),@echo "Filter: $(if $(TEST),$(TEST),$(TESTRUN))",) - @cd storage/deletion && CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" go test -v -timeout 60s $(TESTFLAGS) . .PHONY: test-client test-client: proto @@ -635,9 +595,6 @@ help: @echo " test - Run unit tests (server and client)" @echo " test-all - Run all tests (unit and integration)" @echo " test-server - Run server tests only" - @echo " test-filelock - Run file lock manager tests" - @echo " test-filelock-race - Run file lock manager tests with the race detector" - @echo " test-deletion - Run deletion queue tests" @echo " test-client - Run client tests only" @echo " test-coordinator - Run coordinator tests only" @echo " test-race - Run tests with race detector" @@ -661,8 +618,6 @@ help: @echo " build-bench-cache-topology-rpc - Compile the CacheService topology RPC benchmark (set PERFLOOP_BUILD_OUTPUT_DIR)" @echo " build-bench-cache-service-delete - Compile the CacheService delete benchmark (set PERFLOOP_BUILD_OUTPUT_DIR)" @echo " build-bench-cache-service-list-with-values - Compile the CacheService ListWithValues benchmark (set PERFLOOP_BUILD_OUTPUT_DIR)" - @echo " bench-filelock - Compile file lock benchmarks (set FILELOCK_BENCH_BINARY)" - @echo " bench-deletion - Compile deletion queue benchmarks (set DELETION_BENCH_BINARY)" @echo "" @echo " To run specific tests, use TEST or TESTRUN variable:" @echo " make test TEST=TestMyFunction - Run exact test name" diff --git a/storage/deletion/queue_bench_test.go b/storage/deletion/queue_bench_test.go deleted file mode 100644 index e3d26e2b..00000000 --- a/storage/deletion/queue_bench_test.go +++ /dev/null @@ -1,264 +0,0 @@ -// Copyright 2026 Tigris Data, Inc. -// SPDX-License-Identifier: Apache-2.0 - -package deletion - -import ( - "bytes" - "fmt" - "os" - "path/filepath" - "sync" - "testing" - "time" - - "github.com/rs/zerolog" - "github.com/tigrisdata/ocache/storage/fd" - "github.com/tigrisdata/ocache/storage/keys" - "github.com/tigrisdata/ocache/storage/metadata" -) - -const ( - deletionBenchmarkBatchSize = 1000 - deletionBenchmarkProcessInterval = time.Second - deletionBenchmarkRetryDelay = 30 * time.Second - - deletionBenchmarkCompletionPollInterval = 10 * time.Millisecond - deletionBenchmarkCompletionTimeout = 5 * time.Second -) - -func newLockedRetryQueue(b *testing.B) (*Queue, []string, func()) { - b.Helper() - - meta, err := metadata.NewMetaDB(b.TempDir(), 0, nil, nil) - if err != nil { - b.Fatal(err) - } - - queue := NewQueue(meta, Config{ - BatchSize: deletionBenchmarkBatchSize, - ProcessInterval: deletionBenchmarkProcessInterval, - PruneAge: time.Hour, - RetryDelay: deletionBenchmarkRetryDelay, - }) - - paths := make([]string, deletionBenchmarkBatchSize) - locks := make([]*sync.RWMutex, deletionBenchmarkBatchSize) - lockManager := fd.GetFileLockManager() - tmpDir := b.TempDir() - for i := range paths { - path := filepath.Join(tmpDir, fmt.Sprintf("locked-%04d", i)) - if err := os.WriteFile(path, nil, 0o644); err != nil { - b.Fatal(err) - } - - lock := lockManager.GetFileLock(path) - lock.RLock() - paths[i] = path - locks[i] = lock - } - - return queue, paths, func() { - for _, lock := range locks { - lock.RUnlock() - } - for _, path := range paths { - lockManager.RemoveFileLock(path) - } - meta.Close() - } -} - -func newUnlockedDeletionQueue(b *testing.B) (*Queue, []string, func()) { - b.Helper() - - meta, err := metadata.NewMetaDB(b.TempDir(), 0, nil, nil) - if err != nil { - b.Fatal(err) - } - - queue := NewQueue(meta, Config{ - BatchSize: deletionBenchmarkBatchSize, - ProcessInterval: deletionBenchmarkProcessInterval, - PruneAge: time.Hour, - RetryDelay: deletionBenchmarkRetryDelay, - }) - - paths := make([]string, deletionBenchmarkBatchSize) - tmpDir := b.TempDir() - for i := range paths { - path := filepath.Join(tmpDir, fmt.Sprintf("unlocked-%04d", i)) - if err := os.WriteFile(path, nil, 0o644); err != nil { - b.Fatal(err) - } - paths[i] = path - } - - return queue, paths, func() { - meta.Close() - } -} - -func enqueueDeletionBatch(b *testing.B, queue *Queue, paths []string) { - b.Helper() - - for _, path := range paths { - if err := queue.Add(path); err != nil { - b.Fatal(err) - } - } -} - -func queueEntryKey(b *testing.B, queue *Queue) []byte { - b.Helper() - - ro := metadata.CreateReadOptions(true, false) - defer ro.Destroy() - - it := queue.meta.Handle().NewIterator(ro) - defer it.Close() - - prefix := []byte(keys.DeletionQueuePrefix) - it.Seek(prefix) - if !it.ValidForPrefix(prefix) { - b.Fatal("queued deletion entry not found") - } - - key := it.Key() - defer key.Free() - value := it.Value() - defer value.Free() - return bytes.Clone(key.Data()) -} - -func queueEntryRemoved(queue *Queue, key []byte) (bool, error) { - ro := metadata.CreateReadOptions(false, false) - defer ro.Destroy() - - value, err := queue.meta.Handle().Get(ro, key) - if err != nil { - return false, err - } - defer value.Free() - return !value.Exists(), nil -} - -func waitForQueueEntryRemoval(b *testing.B, queue *Queue, key []byte) { - b.Helper() - - deadline := time.NewTimer(deletionBenchmarkCompletionTimeout) - defer deadline.Stop() - poll := time.NewTicker(deletionBenchmarkCompletionPollInterval) - defer poll.Stop() - - for { - removed, err := queueEntryRemoved(queue, key) - if err != nil { - b.Fatalf("check queued deletion entry: %v", err) - } - if removed { - return - } - - select { - case <-deadline.C: - b.Fatal("timed out waiting for queue processing loop") - case <-poll.C: - } - } -} - -func processQueueBatchThroughLoop(b *testing.B, queue *Queue, key []byte) { - b.Helper() - - queue.Start() - defer queue.Stop() - waitForQueueEntryRemoval(b, queue, key) -} - -func benchmarkQueueProcessingLoopLockedRetryBacklog(b *testing.B) { - b.Helper() - - b.StopTimer() - queue, paths, cleanup := newLockedRetryQueue(b) - defer cleanup() - enqueueDeletionBatch(b, queue, paths) - key := queueEntryKey(b, queue) - - b.StartTimer() - processQueueBatchThroughLoop(b, queue, key) - b.StopTimer() - - if queue.processed != 0 || queue.failed != deletionBenchmarkBatchSize { - b.Fatalf("processingLoop processed=%d failed=%d, want processed=0 failed=%d", queue.processed, queue.failed, deletionBenchmarkBatchSize) - } - if depth := queue.GetQueueDepth(); depth != deletionBenchmarkBatchSize { - b.Fatalf("processingLoop queue depth=%d, want %d", depth, deletionBenchmarkBatchSize) - } - for _, path := range paths { - if _, err := os.Stat(path); err != nil { - b.Fatalf("locked file removed: %v", err) - } - } -} - -func benchmarkQueueProcessingLoopUnlockedDeletionBacklog(b *testing.B) { - b.Helper() - - b.StopTimer() - queue, paths, cleanup := newUnlockedDeletionQueue(b) - defer cleanup() - enqueueDeletionBatch(b, queue, paths) - key := queueEntryKey(b, queue) - - b.StartTimer() - processQueueBatchThroughLoop(b, queue, key) - b.StopTimer() - - if queue.processed != deletionBenchmarkBatchSize || queue.failed != 0 { - b.Fatalf("processingLoop processed=%d failed=%d, want processed=%d failed=0", queue.processed, queue.failed, deletionBenchmarkBatchSize) - } - if depth := queue.GetQueueDepth(); depth != 0 { - b.Fatalf("processingLoop queue depth=%d, want 0", depth) - } - for _, path := range paths { - if _, err := os.Stat(path); !os.IsNotExist(err) { - b.Fatalf("file remained after processingLoop: %v", err) - } - } -} - -// BenchmarkQueueProcessingLoopLockedRetryBacklog measures a full timer-driven -// batch of 1,000 deletions that must be retried. Each path already has a read -// lock, so tryDelete reaches an existing GetFileLock entry before re-enqueuing -// it after 30 seconds. The benchmark waits for removal of an initial queue key, -// which happens in ProcessBatch's final atomic write after the entire batch has -// run, rather than sleeping for a fixed number of ticker intervals. -func BenchmarkQueueProcessingLoopLockedRetryBacklog(b *testing.B) { - level := zerolog.GlobalLevel() - zerolog.SetGlobalLevel(zerolog.Disabled) - defer zerolog.SetGlobalLevel(level) - - b.ReportAllocs() - for b.Loop() { - benchmarkQueueProcessingLoopLockedRetryBacklog(b) - b.StartTimer() - } -} - -// BenchmarkQueueProcessingLoopUnlockedDeletionBacklog measures one full -// timer-driven batch of 1,000 ordinary deletions. It uses the same one-second -// cadence, batch size, and retry delay as the locked-retry benchmark. Removal -// of the initial queue key synchronizes with ProcessBatch's final atomic write -// instead of adding a fixed-duration wait to the measured operation. -func BenchmarkQueueProcessingLoopUnlockedDeletionBacklog(b *testing.B) { - level := zerolog.GlobalLevel() - zerolog.SetGlobalLevel(zerolog.Disabled) - defer zerolog.SetGlobalLevel(level) - - b.ReportAllocs() - for b.Loop() { - benchmarkQueueProcessingLoopUnlockedDeletionBacklog(b) - b.StartTimer() - } -} diff --git a/storage/fd/filelockmanager_bench_test.go b/storage/fd/filelockmanager_bench_test.go deleted file mode 100644 index 15a58394..00000000 --- a/storage/fd/filelockmanager_bench_test.go +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2026 Tigris Data, Inc. -// SPDX-License-Identifier: Apache-2.0 - -package fd - -import ( - "sync/atomic" - "testing" -) - -// BenchmarkFileLockManager_GetFileLockHit measures a lookup for a path whose -// lock was registered by an earlier file operation. -func BenchmarkFileLockManager_GetFileLockHit(b *testing.B) { - const path = "/test/file.txt" - - manager := &FileLockManager{} - want := manager.GetFileLock(path) - - b.ReportAllocs() - for b.Loop() { - if got := manager.GetFileLock(path); got != want { - b.Fatal("GetFileLock returned a different lock for an existing path") - } - } -} - -// BenchmarkFileLockManager_GetFileLockMiss measures only the first lookup for -// a path. Resetting the manager while timing is stopped keeps every lookup on -// the miss path without charging RemoveFileLock to the benchmark. -func BenchmarkFileLockManager_GetFileLockMiss(b *testing.B) { - const path = "/test/file.txt" - - manager := &FileLockManager{} - - b.ReportAllocs() - for b.Loop() { - b.StopTimer() - manager = &FileLockManager{} - b.StartTimer() - - if lock := manager.GetFileLock(path); lock == nil { - b.Fatal("GetFileLock returned nil") - } - } -} - -func BenchmarkFileLockManager_GetFileLockHitParallel(b *testing.B) { - const path = "/test/file.txt" - - manager := &FileLockManager{} - want := manager.GetFileLock(path) - var differentLock atomic.Bool - - b.ReportAllocs() - b.ResetTimer() - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - if got := manager.GetFileLock(path); got != want { - differentLock.Store(true) - return - } - } - }) - if differentLock.Load() { - b.Fatal("GetFileLock returned a different lock for an existing path") - } -}