From d9b8991fef8910cdf7ea8a221f42ac4faff2437f Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Sun, 27 Sep 2026 03:45:06 +0000 Subject: [PATCH 1/9] test(bench): add embedded block range benchmark --- cmd/tag/block_cache_range_benchmark_test.go | 436 ++++++++++++++++++++ 1 file changed, 436 insertions(+) create mode 100644 cmd/tag/block_cache_range_benchmark_test.go diff --git a/cmd/tag/block_cache_range_benchmark_test.go b/cmd/tag/block_cache_range_benchmark_test.go new file mode 100644 index 00000000..b9e7c2d8 --- /dev/null +++ b/cmd/tag/block_cache_range_benchmark_test.go @@ -0,0 +1,436 @@ +package main + +import ( + "bytes" + "context" + "fmt" + "net" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" + "github.com/tigrisdata/ocache/embedded" + pb "github.com/tigrisdata/ocache/proto" + "github.com/tigrisdata/tag/auth" + "github.com/tigrisdata/tag/cache" + "github.com/tigrisdata/tag/config" + "github.com/tigrisdata/tag/handlers" + "github.com/tigrisdata/tag/proxy" + "google.golang.org/grpc" + "google.golang.org/grpc/stats" +) + +const ( + embeddedBlockRangeBenchmarkBlockSize = config.DefaultCacheBlockSize + benchmarkBlockPresenceMethod = "/tag.cache.v1.BlockPresence/Check" +) + +type embeddedBlockRangeBenchmarkFixture struct { + handler http.Handler + body []byte + key string + stats *embeddedBlockRangeRPCStats + requestSigner *auth.RequestSigner + accessKey string + secretKey string + region string +} + +type benchmark206Writer struct { + *httptest.ResponseRecorder + benchmark *testing.B + stopped bool +} + +func (w *benchmark206Writer) stopTimer() { + if w.benchmark != nil && !w.stopped { + w.stopped = true + w.benchmark.StopTimer() + } +} + +func (w *benchmark206Writer) WriteHeader(status int) { + w.ResponseRecorder.WriteHeader(status) + w.stopTimer() +} + +func (w *benchmark206Writer) Write(p []byte) (int, error) { + n, err := w.ResponseRecorder.Write(p) + w.stopTimer() + return n, err +} + +func (w *benchmark206Writer) Flush() { + w.ResponseRecorder.Flush() + w.stopTimer() +} + +func (w *benchmark206Writer) Unwrap() http.ResponseWriter { return w.ResponseRecorder } + +type embeddedBlockRangeRPCStats struct { + presenceRPCs atomic.Int64 + presenceReplyBytes atomic.Int64 + probeGetRPCs atomic.Int64 + probePayloadBytes atomic.Int64 +} + +type embeddedBlockRangeRPCKey struct{} + +type embeddedBlockRangeRPCRecord struct { + method string + probe atomic.Bool +} + +func (s *embeddedBlockRangeRPCStats) TagRPC(ctx context.Context, info *stats.RPCTagInfo) context.Context { + return context.WithValue(ctx, embeddedBlockRangeRPCKey{}, &embeddedBlockRangeRPCRecord{method: info.FullMethodName}) +} + +func (s *embeddedBlockRangeRPCStats) HandleRPC(ctx context.Context, event stats.RPCStats) { + record, _ := ctx.Value(embeddedBlockRangeRPCKey{}).(*embeddedBlockRangeRPCRecord) + if record == nil { + return + } + switch event := event.(type) { + case *stats.InPayload: + request, ok := event.Payload.(*pb.GetRequest) + if ok && record.method == "/cache.CacheService/Get" { + record.probe.Store(strings.HasPrefix(request.Key, "blk|") && request.Start == 0 && request.End == 1) + } + case *stats.OutPayload: + if record.method == benchmarkBlockPresenceMethod { + s.presenceReplyBytes.Add(int64(event.Length)) + } else if record.probe.Load() { + if response, ok := event.Payload.(*pb.GetResponse); ok { + s.probePayloadBytes.Add(int64(len(response.Data))) + } + } + case *stats.End: + if record.method == benchmarkBlockPresenceMethod { + s.presenceRPCs.Add(1) + } + if record.probe.Load() { + s.probeGetRPCs.Add(1) + } + } +} + +func (*embeddedBlockRangeRPCStats) TagConn(ctx context.Context, _ *stats.ConnTagInfo) context.Context { + return ctx +} + +func (*embeddedBlockRangeRPCStats) HandleConn(context.Context, stats.ConnStats) {} + +func (s *embeddedBlockRangeRPCStats) reset() { + s.presenceRPCs.Store(0) + s.presenceReplyBytes.Store(0) + s.probeGetRPCs.Store(0) + s.probePayloadBytes.Store(0) +} + +func newEmbeddedBlockRangeBenchmarkFixture(tb testing.TB, blockCount int, withRPCStats ...bool) *embeddedBlockRangeBenchmarkFixture { + return newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb, blockCount, false, withRPCStats...) +} + +func newEmbeddedBlockRangeTransparentBenchmarkFixture(tb testing.TB, blockCount int) *embeddedBlockRangeBenchmarkFixture { + return newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb, blockCount, true) +} + +func newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb testing.TB, blockCount int, transparent bool, withRPCStats ...bool) *embeddedBlockRangeBenchmarkFixture { + tb.Helper() + if blockCount < 2 || blockCount > 32 { + tb.Fatalf("benchmark block count %d outside 2..32", blockCount) + } + + oldLogger := log.Logger + log.Logger = log.Logger.Level(zerolog.WarnLevel) + tb.Cleanup(func() { log.Logger = oldLogger }) + + gossipAddrs := make([]string, 2) + grpcAddrs := make([]string, 2) + for i := range gossipAddrs { + gossipAddrs[i] = embeddedBlockRangeFreeAddress(tb) + grpcAddrs[i] = embeddedBlockRangeFreeAddress(tb) + } + + var blockCacheClient *embeddedBlockCacheClient + var nodes []*embedded.Client + var stats *embeddedBlockRangeRPCStats + if len(withRPCStats) > 0 && withRPCStats[0] { + stats = &embeddedBlockRangeRPCStats{} + } + const ( + accessKey = "range-benchmark-access" + secretKey = "range-benchmark-secret" + proxyAccessKey = "range-benchmark-proxy" + proxySecretKey = "range-benchmark-proxy-secret" + ) + grpcToken := auth.DeriveGRPCAuthToken(proxyAccessKey, proxySecretKey) + cfg := config.NewDefault() + cfg.Cache.SetBlockCachingEnabled(true) + cfg.Cache.BlockSize = embeddedBlockRangeBenchmarkBlockSize + for i := range gossipAddrs { + embeddedCfg := &embedded.Config{ + DiskPath: tb.TempDir(), + TTL: cfg.Cache.TTL, + NodeID: fmt.Sprintf("range-bench-%d", i), + ClusterAddr: gossipAddrs[i], + GRPCAddr: grpcAddrs[i], + AdvertiseAddr: grpcAddrs[i], + Registerer: prometheus.NewRegistry(), + } + if cfg.Cache.IsGRPCAuthEnabled() { + embeddedCfg.GRPCServerOptions = auth.GRPCServerOptions(grpcToken) + embeddedCfg.GRPCDialOptions = auth.GRPCDialOptions(grpcToken) + } + if stats != nil { + embeddedCfg.GRPCServerOptions = append(embeddedCfg.GRPCServerOptions, grpc.StatsHandler(stats)) + } + if i > 0 { + embeddedCfg.SeedNodes = []string{gossipAddrs[0]} + } + client, err := embedded.New(embeddedCfg) + if err != nil { + tb.Fatalf("create embedded range node %d: %v", i, err) + } + nodes = append(nodes, client) + if i == 0 { + blockCacheClient = newEmbeddedBlockCacheClient(client) + tb.Cleanup(func() { _ = blockCacheClient.Close() }) + } else { + tb.Cleanup(func() { _ = client.Close() }) + } + if err := client.StartGRPCServer(); err != nil { + tb.Fatalf("start embedded range node %d: %v", i, err) + } + } + readyCtx, readyCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer readyCancel() + for _, client := range nodes { + if err := client.WaitReady(readyCtx); err != nil { + tb.Fatalf("embedded range node not ready: %v", err) + } + } + for { + ready := true + for _, client := range nodes { + if len(client.Coordinator().GetRing().GetActiveNodes()) != len(nodes) { + ready = false + break + } + } + if ready { + break + } + select { + case <-readyCtx.Done(): + tb.Fatalf("embedded range cluster did not converge: %v", readyCtx.Err()) + case <-time.After(20 * time.Millisecond): + } + } + + body := make([]byte, blockCount*embeddedBlockRangeBenchmarkBlockSize) + for i := range body { + body[i] = byte(i*31 + i/257) + } + const ( + bucket = "benchmark" + etag = `"block-range"` + ) + key := embeddedBlockRangeObjectKey(tb, nodes[0], blockCount, bucket, etag) + store := cache.NewCacheWithClient(blockCacheClient, &cfg.Cache) + for i := 0; i < blockCount; i++ { + start := i * embeddedBlockRangeBenchmarkBlockSize + end := start + embeddedBlockRangeBenchmarkBlockSize + if err := store.PutBlock(context.Background(), bucket, key, etag, embeddedBlockRangeBenchmarkBlockSize, int64(i), body[start:end], 60); err != nil { + tb.Fatalf("seed embedded block %d: %v", i, err) + } + } + meta := &cache.CachedObjectMeta{ + Bucket: bucket, + Key: key, + ETag: etag, + ContentType: "application/octet-stream", + ContentLength: int64(len(body)), + StatusCode: http.StatusOK, + BlockSize: embeddedBlockRangeBenchmarkBlockSize, + } + if wrote, err := store.PutMetaIfVersion(context.Background(), bucket, key, meta, 60, cache.VersionAny); err != nil || !wrote { + tb.Fatalf("seed range metadata = (wrote=%t, err=%v)", wrote, err) + } + + upstreamEndpoint := "http://127.0.0.1:1" // a warmed hit must never reach upstream + cfg.Upstream.Endpoint = upstreamEndpoint + var forwarder proxy.RequestForwarder + if transparent { + cfg.Mode = config.ModeTransparent + clientCredentials := auth.NewCredentialStore() + clientCredentials.AddCredential(accessKey, secretKey) + derivedKeyStore := auth.NewDerivedKeyStore(auth.DefaultDerivedKeyTTL) + now := time.Now().UTC() + for dayOffset := 0; dayOffset <= 1; dayOffset++ { + date := now.Add(time.Duration(dayOffset) * 24 * time.Hour).Format("20060102") + signingKey, err := clientCredentials.GetSigningKey(accessKey, date, cfg.Upstream.Region) + if err != nil { + tb.Fatalf("derive transparent benchmark key: %v", err) + } + derivedKeyStore.Store(accessKey, date, cfg.Upstream.Region, signingKey) + } + keyUnwrapper, err := auth.NewKeyUnwrapper(proxySecretKey) + if err != nil { + tb.Fatalf("create transparent benchmark key unwrapper: %v", err) + } + authzCache := auth.NewAuthzCache(auth.DefaultAuthzCacheTTL) + authzCache.Grant(accessKey, bucket) + localAuth := &proxy.LocalAuthConfig{ + DerivedKeyStore: derivedKeyStore, + Validator: auth.NewRequestValidator(derivedKeyStore), + KeyUnwrapper: keyUnwrapper, + AuthzCache: authzCache, + } + proxySigner := auth.NewProxySigner(proxyAccessKey, proxySecretKey) + forwarder = proxy.NewForwarder(nil, upstreamEndpoint, cfg.Upstream.Region, cfg.Upstream.MaxIdleConnsPerHost, proxySigner, localAuth) + } else { + cfg.Mode = config.ModeSigning + credentialStore := auth.NewCredentialStore() + credentialStore.AddCredential(accessKey, secretKey) + forwarder = proxy.NewForwarder(credentialStore, upstreamEndpoint, cfg.Upstream.Region, cfg.Upstream.MaxIdleConnsPerHost, nil, nil) + } + requestSigner := auth.NewRequestSigner("http://range-benchmark.invalid", cfg.Upstream.Region) + service := proxy.NewService(forwarder, store, cfg) + server := handlers.NewServer(service, "127.0.0.1", 0, false, cfg.Server.MaxInflightRequests) + handler := server.Router() + return &embeddedBlockRangeBenchmarkFixture{ + handler: handler, body: body, key: key, stats: stats, + requestSigner: requestSigner, accessKey: accessKey, secretKey: secretKey, region: cfg.Upstream.Region, + } +} + +// embeddedBlockRangeObjectKey holds the owner mix constant across fresh randomized rings: +// the first block is local, the second remote, and half of the full range is remote. +func embeddedBlockRangeObjectKey(tb testing.TB, client *embedded.Client, blockCount int, bucket, etag string) string { + tb.Helper() + localID := client.Coordinator().GetLocalNodeID() + wantRemote := blockCount / 2 + for candidate := 0; candidate < 65536; candidate++ { + key := fmt.Sprintf("block-range-%05d", candidate) + remote := 0 + remoteFirst, remoteSecond := false, false + for index := 0; index < blockCount; index++ { + blockKey := cache.MakeBlockKey(bucket, key, etag, embeddedBlockRangeBenchmarkBlockSize, int64(index)) + node, err := client.Coordinator().GetNodeForKey(blockKey) + if err != nil { + tb.Fatalf("resolve benchmark block owner: %v", err) + } + isRemote := node.ID != localID + if isRemote { + remote++ + } + if index == 0 { + remoteFirst = isRemote + } else if index == 1 { + remoteSecond = isRemote + } + } + if remote == wantRemote && !remoteFirst && remoteSecond { + return key + } + } + tb.Fatalf("could not find a %d-block object with %d remote owners", blockCount, wantRemote) + return "" +} + +func embeddedBlockRangeFreeAddress(tb testing.TB) string { + tb.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + tb.Fatal(err) + } + addr := listener.Addr().String() + if err := listener.Close(); err != nil { + tb.Fatal(err) + } + return addr +} + +func (f *embeddedBlockRangeBenchmarkFixture) doRange(tb testing.TB, benchmark *testing.B) *httptest.ResponseRecorder { + tb.Helper() + if benchmark != nil { + benchmark.StopTimer() + } + unsigned := httptest.NewRequest(http.MethodGet, "/benchmark/"+f.key, nil) + unsigned.Header.Set("Range", fmt.Sprintf("bytes=0-%d", len(f.body)-1)) + req, err := f.requestSigner.SignRequest(unsigned.Context(), unsigned.Method, unsigned.URL.RequestURI(), unsigned.Body, "", f.accessKey, f.secretKey, unsigned.Header) + if err != nil { + tb.Fatalf("sign benchmark request: %v", err) + } + req.RemoteAddr = unsigned.RemoteAddr + response := httptest.NewRecorder() + writer := &benchmark206Writer{ResponseRecorder: response, benchmark: benchmark} + if benchmark != nil { + benchmark.StartTimer() + } + f.handler.ServeHTTP(writer, req) + writer.stopTimer() + return response +} + +func (f *embeddedBlockRangeBenchmarkFixture) verifyResponse(tb testing.TB, response *httptest.ResponseRecorder) { + tb.Helper() + body := response.Body.Bytes() + if response.Code != http.StatusPartialContent || response.Header().Get(proxy.XCacheHeader) != proxy.XCacheHit || !bytes.Equal(body, f.body) { + tb.Fatalf("range response = (status=%d, cache=%q, bytes=%d, exact=%t), want (206, HIT, %d exact bytes)", response.Code, response.Header().Get(proxy.XCacheHeader), len(body), bytes.Equal(body, f.body), len(f.body)) + } + if got, want := response.Header().Get("Content-Range"), fmt.Sprintf("bytes 0-%d/%d", len(f.body)-1, len(f.body)); got != want { + tb.Fatalf("Content-Range=%q, want %q", got, want) + } + if got, want := response.Header().Get("Content-Length"), fmt.Sprint(len(f.body)); got != want { + tb.Fatalf("Content-Length=%q, want %q", got, want) + } + if got := response.Header().Get("ETag"); got != `"block-range"` { + tb.Fatalf("ETag=%q, want %q", got, `"block-range"`) + } +} + +func BenchmarkEmbeddedBlockRangePresence(b *testing.B) { + oldLogger := log.Logger + log.Logger = log.Logger.Level(zerolog.WarnLevel) + b.Cleanup(func() { log.Logger = oldLogger }) + + for _, blockCount := range []int{2, 4, 8, 32} { + b.Run(fmt.Sprintf("%d_blocks", blockCount), func(b *testing.B) { + benchmarkEmbeddedBlockRangePresence(b, blockCount, false) + }) + } +} + +func BenchmarkEmbeddedBlockRangePresenceTransparent(b *testing.B) { + oldLogger := log.Logger + log.Logger = log.Logger.Level(zerolog.WarnLevel) + b.Cleanup(func() { log.Logger = oldLogger }) + + b.Run("32_blocks", func(b *testing.B) { + benchmarkEmbeddedBlockRangePresence(b, 32, true) + }) +} + +func benchmarkEmbeddedBlockRangePresence(b *testing.B, blockCount int, transparent bool) { + b.StopTimer() + var fixture *embeddedBlockRangeBenchmarkFixture + if transparent { + fixture = newEmbeddedBlockRangeTransparentBenchmarkFixture(b, blockCount) + } else { + fixture = newEmbeddedBlockRangeBenchmarkFixture(b, blockCount) + } + fixture.verifyResponse(b, fixture.doRange(b, nil)) // warm auth, cache, and peer connections + b.ResetTimer() + for i := 0; i < b.N; i++ { + fixture.verifyResponse(b, fixture.doRange(b, b)) + } + b.StopTimer() +} From c47131774b4d92a76ede3a719d550f5efb88809f Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Sun, 27 Sep 2026 09:47:48 +0000 Subject: [PATCH 2/9] test(bench): stabilize embedded range placement --- cmd/tag/block_cache_range_benchmark_test.go | 74 +++++++++++++++++++-- 1 file changed, 69 insertions(+), 5 deletions(-) diff --git a/cmd/tag/block_cache_range_benchmark_test.go b/cmd/tag/block_cache_range_benchmark_test.go index b9e7c2d8..5954be8f 100644 --- a/cmd/tag/block_cache_range_benchmark_test.go +++ b/cmd/tag/block_cache_range_benchmark_test.go @@ -3,10 +3,13 @@ package main import ( "bytes" "context" + "encoding/json" "fmt" "net" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "sync/atomic" "testing" @@ -175,8 +178,10 @@ func newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb testing.TB, blockCount int cfg.Cache.SetBlockCachingEnabled(true) cfg.Cache.BlockSize = embeddedBlockRangeBenchmarkBlockSize for i := range gossipAddrs { + diskPath := tb.TempDir() + writeEmbeddedBlockRangeRingTokens(tb, diskPath, i) embeddedCfg := &embedded.Config{ - DiskPath: tb.TempDir(), + DiskPath: diskPath, TTL: cfg.Cache.TTL, NodeID: fmt.Sprintf("range-bench-%d", i), ClusterAddr: gossipAddrs[i], @@ -311,16 +316,51 @@ func newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb testing.TB, blockCount int } } -// embeddedBlockRangeObjectKey holds the owner mix constant across fresh randomized rings: -// the first block is local, the second remote, and half of the full range is remote. +// writeEmbeddedBlockRangeRingTokens gives both benchmark arms the same ownership layout. +// OCache otherwise generates fresh tokens for every temporary DiskPath, which can change +// metadata locality and the local/remote order of a measured range between arms. +func writeEmbeddedBlockRangeRingTokens(tb testing.TB, diskPath string, nodeIndex int) { + tb.Helper() + const tokenCount = 512 + if nodeIndex < 0 || nodeIndex > 1 { + tb.Fatalf("benchmark ring node index %d outside 0..1", nodeIndex) + } + state := uint32(0x6d2b79f5) ^ uint32(nodeIndex+1)*0x9e3779b9 + tokens := make([]uint32, tokenCount) + for i := range tokens { + state ^= state << 13 + state ^= state >> 17 + state ^= state << 5 + tokens[i] = state + } + contents, err := json.Marshal(struct { + Tokens []uint32 `json:"tokens"` + }{Tokens: tokens}) + if err != nil { + tb.Fatalf("marshal benchmark ring tokens: %v", err) + } + tokensPath := filepath.Join(diskPath, "coordinator", "ring-tokens") + if err := os.MkdirAll(filepath.Dir(tokensPath), 0o755); err != nil { + tb.Fatalf("create benchmark ring-token directory: %v", err) + } + if err := os.WriteFile(tokensPath, contents, 0o600); err != nil { + tb.Fatalf("write benchmark ring tokens: %v", err) + } +} + +// embeddedBlockRangeObjectKey keeps metadata local and holds the block-owner mix +// constant across both benchmark arms: block 0 is local, block 1 is remote, and +// half of the full range is remote. func embeddedBlockRangeObjectKey(tb testing.TB, client *embedded.Client, blockCount int, bucket, etag string) string { tb.Helper() localID := client.Coordinator().GetLocalNodeID() wantRemote := blockCount / 2 + wantPattern := expectedEmbeddedBlockRangeOwnerPattern(blockCount) for candidate := 0; candidate < 65536; candidate++ { key := fmt.Sprintf("block-range-%05d", candidate) remote := 0 remoteFirst, remoteSecond := false, false + ownerPattern := make([]byte, blockCount) for index := 0; index < blockCount; index++ { blockKey := cache.MakeBlockKey(bucket, key, etag, embeddedBlockRangeBenchmarkBlockSize, int64(index)) node, err := client.Coordinator().GetNodeForKey(blockKey) @@ -330,6 +370,9 @@ func embeddedBlockRangeObjectKey(tb testing.TB, client *embedded.Client, blockCo isRemote := node.ID != localID if isRemote { remote++ + ownerPattern[index] = '1' + } else { + ownerPattern[index] = '0' } if index == 0 { remoteFirst = isRemote @@ -337,14 +380,35 @@ func embeddedBlockRangeObjectKey(tb testing.TB, client *embedded.Client, blockCo remoteSecond = isRemote } } - if remote == wantRemote && !remoteFirst && remoteSecond { + metaOwner, err := client.Coordinator().GetNodeForKey(cache.MakeMetaKey(bucket, key)) + if err != nil { + tb.Fatalf("resolve benchmark metadata owner: %v", err) + } + if remote == wantRemote && !remoteFirst && remoteSecond && metaOwner.ID == localID && + (wantPattern == "" || string(ownerPattern) == wantPattern) { return key } } - tb.Fatalf("could not find a %d-block object with %d remote owners", blockCount, wantRemote) + tb.Fatalf("could not find a %d-block object with %d remote owners and pattern %q", blockCount, wantRemote, wantPattern) return "" } +// expectedEmbeddedBlockRangeOwnerPattern guards the exact measured owner order. +func expectedEmbeddedBlockRangeOwnerPattern(blockCount int) string { + switch blockCount { + case 2: + return "01" + case 4: + return "0110" + case 8: + return "01111000" + case 32: + return "01000011001100011011010011100111" + default: + return "" + } +} + func embeddedBlockRangeFreeAddress(tb testing.TB) string { tb.Helper() listener, err := net.Listen("tcp", "127.0.0.1:0") From 51edfb3ec341efd560901acb8d8d9b9cfa6a54ff Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Sun, 27 Sep 2026 10:03:18 +0000 Subject: [PATCH 3/9] perf(cache): batch range block presence checks --- cache/block_presence.go | 83 +++ cache/block_presence_test.go | 192 +++++ cmd/tag/block_cache_client.go | 295 +++++++- cmd/tag/block_cache_range_benchmark_test.go | 58 +- cmd/tag/block_presence.go | 270 +++++++ cmd/tag/block_presence_test.go | 774 ++++++++++++++++++++ cmd/tag/main.go | 9 +- docs/rfcs/0001-block-aligned-caching.md | 2 +- proxy/block_cache.go | 71 +- proxy/block_cache_integration_test.go | 327 ++++++++- 10 files changed, 2031 insertions(+), 50 deletions(-) create mode 100644 cache/block_presence.go create mode 100644 cache/block_presence_test.go create mode 100644 cmd/tag/block_presence.go create mode 100644 cmd/tag/block_presence_test.go diff --git a/cache/block_presence.go b/cache/block_presence.go new file mode 100644 index 00000000..a4a1bc86 --- /dev/null +++ b/cache/block_presence.go @@ -0,0 +1,83 @@ +package cache + +import ( + "context" + "errors" + "fmt" +) + +// ErrBlockPresenceUnsupported signals that a peer does not implement the optional +// block-presence exchange. Cache falls back to its single-key probe for that page. +var ErrBlockPresenceUnsupported = errors.New("block presence exchange unsupported") + +// ErrBlockPresenceTopologyChanged signals that the owner or ring view changed +// during a batched check. Cache discards the whole page and retries it through the +// normal per-key routing path. +var ErrBlockPresenceTopologyChanged = errors.New("block presence topology changed") + +const maxBlockPresenceBatchSize = 32 + +// blockPresenceClient is an optional CacheClient capability. The result order +// must match keys; errors are never absence. It deliberately does not extend the +// required ocache CacheClient interface. +type blockPresenceClient interface { + BlockPresence(ctx context.Context, keys []string) ([]bool, error) +} + +// BlockExistsBatchErr reports presence for blockIdxs in their original order, +// processing at most maxBlockPresenceBatchSize keys per optional exchange. +// Clients without the optional batch capability, and peers that explicitly do +// not support it, use BlockExistsErr one key at a time. A real batch error is +// returned unchanged so a routing or storage failure cannot become a miss. +func (c *Cache) BlockExistsBatchErr(ctx context.Context, bucket, key, etag string, blockSize int64, blockIdxs []int64) ([]bool, error) { + present := make([]bool, len(blockIdxs)) + if len(blockIdxs) == 0 || !c.IsEnabled() || etag == "" { + return present, nil + } + + for start := 0; start < len(blockIdxs); start += maxBlockPresenceBatchSize { + end := min(start+maxBlockPresenceBatchSize, len(blockIdxs)) + page, err := c.blockExistsPageErr(ctx, bucket, key, etag, blockSize, blockIdxs[start:end]) + if err != nil { + return nil, err + } + copy(present[start:end], page) + } + return present, nil +} + +func (c *Cache) blockExistsPageErr(ctx context.Context, bucket, key, etag string, blockSize int64, blockIdxs []int64) ([]bool, error) { + present := make([]bool, len(blockIdxs)) + keys := make([]string, len(blockIdxs)) + for i, idx := range blockIdxs { + keys[i] = MakeBlockKey(bucket, key, etag, blockSize, idx) + } + + if batcher, ok := c.client.(blockPresenceClient); ok { + batch, err := batcher.BlockPresence(ctx, keys) + if err == nil { + if len(batch) != len(keys) { + return nil, fmt.Errorf("block presence returned %d results for %d keys", len(batch), len(keys)) + } + for i, found := range batch { + present[i] = found + if found { + c.recordServeLocality(keys[i]) + } + } + return present, nil + } + if !errors.Is(err, ErrBlockPresenceUnsupported) && !errors.Is(err, ErrBlockPresenceTopologyChanged) { + return nil, err + } + } + + for i, idx := range blockIdxs { + found, err := c.BlockExistsErr(ctx, bucket, key, etag, blockSize, idx) + if err != nil { + return nil, err + } + present[i] = found + } + return present, nil +} diff --git a/cache/block_presence_test.go b/cache/block_presence_test.go new file mode 100644 index 00000000..05366d5e --- /dev/null +++ b/cache/block_presence_test.go @@ -0,0 +1,192 @@ +package cache + +import ( + "context" + "errors" + "fmt" + "io" + "reflect" + "strings" + "testing" + + cacheclient "github.com/tigrisdata/ocache/client" + "github.com/tigrisdata/tag/config" +) + +type blockPresenceTestClient struct { + cacheclient.CacheClient + result []bool + batchErr error + batchKeys []string + pageSizes []int + probeCalls int + localKeys []string +} + +func (c *blockPresenceTestClient) BlockPresence(_ context.Context, keys []string) ([]bool, error) { + start := len(c.batchKeys) + c.batchKeys = append(c.batchKeys, keys...) + c.pageSizes = append(c.pageSizes, len(keys)) + if c.batchErr != nil { + return nil, c.batchErr + } + end := start + len(keys) + if end > len(c.result) { + return append([]bool(nil), c.result[start:]...), nil + } + return append([]bool(nil), c.result[start:end]...), nil +} + +func (c *blockPresenceTestClient) GetRangeStream(ctx context.Context, key string, start, end int64, w io.Writer) error { + if strings.HasPrefix(key, blockKeyPrefix) && start == 0 && end == 1 { + c.probeCalls++ + } + return c.CacheClient.GetRangeStream(ctx, key, start, end, w) +} + +func (c *blockPresenceTestClient) IsLocal(key string) bool { + c.localKeys = append(c.localKeys, key) + return true +} + +func newBlockPresenceTestCache(client cacheclient.CacheClient) *Cache { + cfg := config.NewDefault() + return NewCacheWithClient(client, &cfg.Cache) +} + +func TestBlockExistsBatchErr_OptionalClientPreservesOrderAndLocality(t *testing.T) { + client := &blockPresenceTestClient{ + CacheClient: cacheclient.NewMemoryCache(), + result: []bool{false, true, true}, + } + c := newBlockPresenceTestCache(client) + + got, err := c.BlockExistsBatchErr(context.Background(), "b", "k", `"v1"`, 4, []int64{3, 1, 2}) + if err != nil { + t.Fatalf("BlockExistsBatchErr: %v", err) + } + if want := []bool{false, true, true}; !reflect.DeepEqual(got, want) { + t.Fatalf("presence = %v, want %v", got, want) + } + wantKeys := []string{ + MakeBlockKey("b", "k", `"v1"`, 4, 3), + MakeBlockKey("b", "k", `"v1"`, 4, 1), + MakeBlockKey("b", "k", `"v1"`, 4, 2), + } + if !reflect.DeepEqual(client.batchKeys, wantKeys) { + t.Fatalf("batch keys = %v, want %v", client.batchKeys, wantKeys) + } + if !reflect.DeepEqual(client.localKeys, wantKeys[1:]) { + t.Fatalf("locality checks = %v, want present keys %v", client.localKeys, wantKeys[1:]) + } + if client.probeCalls != 0 { + t.Fatalf("batch path opened %d single-key streams, want 0", client.probeCalls) + } +} + +func TestBlockExistsBatchErr_FallsBackForMissingCapabilityAndUnsupportedPeer(t *testing.T) { + for _, tc := range []struct { + name string + batchErr error + wantBatch int + }{ + {name: "no capability"}, + {name: "old peer", batchErr: fmt.Errorf("peer response: %w", ErrBlockPresenceUnsupported), wantBatch: 3}, + {name: "owner changed", batchErr: fmt.Errorf("peer response: %w", ErrBlockPresenceTopologyChanged), wantBatch: 3}, + } { + t.Run(tc.name, func(t *testing.T) { + mem := cacheclient.NewMemoryCache() + presentKey := MakeBlockKey("b", "k", `"v1"`, 4, 0) + if err := mem.Put(context.Background(), presentKey, []byte("AB"), 0); err != nil { + t.Fatal(err) + } + + var wrapped cacheclient.CacheClient = mem + var probeClient *blockPresenceTestClient + if tc.batchErr != nil { + probeClient = &blockPresenceTestClient{CacheClient: mem, batchErr: tc.batchErr} + wrapped = probeClient + } + c := newBlockPresenceTestCache(wrapped) + got, err := c.BlockExistsBatchErr(context.Background(), "b", "k", `"v1"`, 4, []int64{2, 0, 1}) + if err != nil { + t.Fatalf("BlockExistsBatchErr: %v", err) + } + if want := []bool{false, true, false}; !reflect.DeepEqual(got, want) { + t.Fatalf("presence = %v, want %v", got, want) + } + if probeClient != nil { + if probeClient.probeCalls != 3 { + t.Fatalf("fallback single-key probes = %d, want 3", probeClient.probeCalls) + } + if got := len(probeClient.batchKeys); got != tc.wantBatch { + t.Fatalf("batch key count = %d, want %d", got, tc.wantBatch) + } + } + }) + } +} + +func TestBlockExistsBatchErr_PropagatesRealBatchFailure(t *testing.T) { + failure := errors.New("owner storage unavailable") + client := &blockPresenceTestClient{ + CacheClient: cacheclient.NewMemoryCache(), + batchErr: failure, + } + c := newBlockPresenceTestCache(client) + + _, err := c.BlockExistsBatchErr(context.Background(), "b", "k", `"v1"`, 4, []int64{0, 1}) + if !errors.Is(err, failure) { + t.Fatalf("BlockExistsBatchErr error = %v, want %v", err, failure) + } + if client.probeCalls != 0 { + t.Fatalf("real batch failure fell back to %d single-key probes", client.probeCalls) + } +} + +func TestBlockExistsBatchErr_PagesOversizedInput(t *testing.T) { + client := &blockPresenceTestClient{ + CacheClient: cacheclient.NewMemoryCache(), + result: make([]bool, maxBlockPresenceBatchSize+1), + } + for i := range client.result { + client.result[i] = true + } + c := newBlockPresenceTestCache(client) + indices := make([]int64, len(client.result)) + for i := range indices { + indices[i] = int64(i) + } + + got, err := c.BlockExistsBatchErr(context.Background(), "b", "k", `"v1"`, 4, indices) + if err != nil { + t.Fatalf("BlockExistsBatchErr: %v", err) + } + if len(got) != len(indices) { + t.Fatalf("presence result count = %d, want %d", len(got), len(indices)) + } + for i, present := range got { + if !present { + t.Errorf("presence[%d] = false, want true", i) + } + } + if want := []int{maxBlockPresenceBatchSize, 1}; !reflect.DeepEqual(client.pageSizes, want) { + t.Fatalf("batch page sizes = %v, want %v", client.pageSizes, want) + } +} + +func TestBlockExistsBatchErr_RejectsMalformedBatchResult(t *testing.T) { + client := &blockPresenceTestClient{ + CacheClient: cacheclient.NewMemoryCache(), + result: []bool{true}, + } + c := newBlockPresenceTestCache(client) + + _, err := c.BlockExistsBatchErr(context.Background(), "b", "k", `"v1"`, 4, []int64{0, 1}) + if err == nil { + t.Fatal("BlockExistsBatchErr accepted a short result") + } + if client.probeCalls != 0 { + t.Fatalf("malformed result fell back to %d single-key probes", client.probeCalls) + } +} diff --git a/cmd/tag/block_cache_client.go b/cmd/tag/block_cache_client.go index df36be03..4b191bac 100644 --- a/cmd/tag/block_cache_client.go +++ b/cmd/tag/block_cache_client.go @@ -2,21 +2,55 @@ package main import ( "context" + "errors" + "fmt" + "io" + "sort" + "sync" + cacheclient "github.com/tigrisdata/ocache/client" "github.com/tigrisdata/ocache/embedded" "github.com/tigrisdata/tag/cache" + "golang.org/x/sync/errgroup" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/structpb" ) // embeddedBlockCacheClient keeps embedded.Client's normal CacheClient behavior -// while providing a byte-slice write for blocks whose owner is remote. Local -// writes retain the embedded streaming path because they already write directly -// to local storage. +// while providing the byte-slice write and optional grouped block-presence paths. type embeddedBlockCacheClient struct { *embedded.Client + + dialOptions []grpc.DialOption + peerMu sync.Mutex + peers map[string]blockPresencePeer + closed bool + closeOnce sync.Once + closeErr error +} + +type blockPresencePeer struct { + address string + conn *grpc.ClientConn } -func newEmbeddedBlockCacheClient(client *embedded.Client) *embeddedBlockCacheClient { - return &embeddedBlockCacheClient{Client: client} +type blockPresenceOwnerGroup struct { + owner string + address string + local bool + keys []string + positions []int +} + +func newEmbeddedBlockCacheClient(client *embedded.Client, dialOptions ...grpc.DialOption) *embeddedBlockCacheClient { + options := append(cacheclient.DefaultDialOptions(), dialOptions...) + return &embeddedBlockCacheClient{ + Client: client, + dialOptions: options, + peers: make(map[string]blockPresencePeer), + } } // PutBlockBytes writes a fully staged block to its remote owner in one unary @@ -26,3 +60,254 @@ func newEmbeddedBlockCacheClient(client *embedded.Client) *embeddedBlockCacheCli func (c *embeddedBlockCacheClient) PutBlockBytes(ctx context.Context, key string, data []byte, ttlSeconds int64) (handled bool, err error) { return cache.PutRemoteBlockBytes(ctx, c.Operations(), key, data, ttlSeconds) } + +// BlockPresence checks one bounded range-preflight page. Remote keys are grouped +// by their current owner and sent in one compact exchange per owner; local keys +// use the same owner-local byte-zero read without opening a gRPC stream. +func (c *embeddedBlockCacheClient) BlockPresence(ctx context.Context, keys []string) ([]bool, error) { + if len(keys) == 0 || len(keys) > blockPresenceMaxKeys { + return nil, fmt.Errorf("invalid block presence key count %d", len(keys)) + } + if err := ctx.Err(); err != nil { + return nil, err + } + + coord := c.Coordinator() + var epoch uint64 + var localID string + if coord != nil { + epoch = coord.GetEpoch() + localID = coord.GetLocalNodeID() + } + groups := make(map[string]*blockPresenceOwnerGroup) + owners := make([]string, len(keys)) + addresses := make([]string, len(keys)) + for i, key := range keys { + owner := "" + address := "" + local := true + if coord != nil { + node, err := coord.GetNodeForKey(key) + if err != nil { + return nil, fmt.Errorf("resolve block owner for %q: %w", key, err) + } + if node == nil || node.ID == "" { + return nil, fmt.Errorf("resolve block owner for %q: no owner", key) + } + owner = node.ID + address = node.ListenAddress + local = owner == localID + if !local && address == "" { + return nil, fmt.Errorf("block owner %q has no gRPC address", owner) + } + } + owners[i] = owner + addresses[i] = address + group := groups[owner] + if group == nil { + group = &blockPresenceOwnerGroup{owner: owner, address: address, local: local} + groups[owner] = group + } else if group.address != address || group.local != local { + return nil, cache.ErrBlockPresenceTopologyChanged + } + group.keys = append(group.keys, key) + group.positions = append(group.positions, i) + } + if coord != nil && coord.GetEpoch() != epoch { + return nil, cache.ErrBlockPresenceTopologyChanged + } + + ownerIDs := make([]string, 0, len(groups)) + for owner := range groups { + ownerIDs = append(ownerIDs, owner) + } + sort.Strings(ownerIDs) + + present := make([]bool, len(keys)) + var mu sync.Mutex + var retryErr, fatalErr error + group, groupCtx := errgroup.WithContext(ctx) + for _, owner := range ownerIDs { + ownerGroup := groups[owner] + group.Go(func() error { + var found []bool + var err error + if ownerGroup.local { + found, err = embeddedBlockPresence(groupCtx, c.Client, ownerGroup.owner, epoch, ownerGroup.keys) + } else { + found, err = c.remoteBlockPresence(groupCtx, ownerGroup.owner, ownerGroup.address, epoch, ownerGroup.keys) + } + if err == nil && len(found) != len(ownerGroup.positions) { + err = fmt.Errorf("block presence returned %d results for %d keys", len(found), len(ownerGroup.positions)) + } + if err != nil { + mu.Lock() + switch { + case errors.Is(err, cache.ErrBlockPresenceUnsupported), errors.Is(err, cache.ErrBlockPresenceTopologyChanged): + if retryErr == nil || errors.Is(err, cache.ErrBlockPresenceTopologyChanged) { + retryErr = err + } + case isSiblingBlockPresenceCancellation(ctx, groupCtx, err): + // A sibling's failure canceled this exchange; the original error is + // recorded by the owner that caused the cancellation. + default: + if fatalErr == nil { + fatalErr = err + } + } + mu.Unlock() + return err + } + for i, value := range found { + present[ownerGroup.positions[i]] = value + } + return nil + }) + } + _ = group.Wait() + if err := ctx.Err(); err != nil { + return nil, err + } + if fatalErr != nil { + return nil, fatalErr + } + if retryErr != nil { + return nil, retryErr + } + if err := ctx.Err(); err != nil { + return nil, err + } + if coord != nil { + if coord.GetEpoch() != epoch { + return nil, cache.ErrBlockPresenceTopologyChanged + } + for i, key := range keys { + node, err := coord.GetNodeForKey(key) + if err != nil { + return nil, fmt.Errorf("recheck block owner for %q: %w", key, err) + } + if node == nil || node.ID != owners[i] || node.ListenAddress != addresses[i] { + return nil, cache.ErrBlockPresenceTopologyChanged + } + } + } + return present, nil +} + +func (c *embeddedBlockCacheClient) remoteBlockPresence(ctx context.Context, owner, address string, epoch uint64, keys []string) ([]bool, error) { + request, err := encodeBlockPresenceRequest(owner, epoch, keys) + if err != nil { + if errors.Is(err, errBlockPresenceRequestTooLarge) { + return nil, cache.ErrBlockPresenceUnsupported + } + return nil, err + } + conn, err := c.peerConnection(owner, address) + if err != nil { + return nil, err + } + stream, err := conn.NewStream(ctx, &grpc.StreamDesc{ + StreamName: "Check", + ClientStreams: true, + ServerStreams: true, + }, blockPresenceMethod) + if err != nil { + return nil, blockPresenceRPCError(ctx, err) + } + if err := stream.SendMsg(request); err != nil { + // A server-side Unimplemented/FailedPrecondition can arrive before SendMsg + // finishes. grpc-go reports io.EOF here and exposes the status on RecvMsg. + if !errors.Is(err, io.EOF) { + return nil, blockPresenceRPCError(ctx, err) + } + return receiveBlockPresenceResponse(ctx, stream, epoch, len(keys)) + } + if err := stream.CloseSend(); err != nil { + if !errors.Is(err, io.EOF) { + return nil, blockPresenceRPCError(ctx, err) + } + return receiveBlockPresenceResponse(ctx, stream, epoch, len(keys)) + } + return receiveBlockPresenceResponse(ctx, stream, epoch, len(keys)) +} + +func receiveBlockPresenceResponse(ctx context.Context, stream grpc.ClientStream, epoch uint64, keyCount int) ([]bool, error) { + var response structpb.Struct + if err := stream.RecvMsg(&response); err != nil { + return nil, blockPresenceRPCError(ctx, err) + } + var extra structpb.Struct + if err := stream.RecvMsg(&extra); !errors.Is(err, io.EOF) { + if err != nil { + return nil, blockPresenceRPCError(ctx, err) + } + return nil, errors.New("block presence returned more than one response") + } + return decodeBlockPresenceResponse(&response, epoch, keyCount) +} + +func isSiblingBlockPresenceCancellation(ctx, groupCtx context.Context, err error) bool { + return ctx.Err() == nil && groupCtx.Err() != nil && + (errors.Is(err, context.Canceled) || status.Code(err) == codes.Canceled) +} + +func blockPresenceRPCError(ctx context.Context, err error) error { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } + switch status.Code(err) { + case codes.Unimplemented: + return fmt.Errorf("%w: %v", cache.ErrBlockPresenceUnsupported, err) + case codes.FailedPrecondition: + return fmt.Errorf("%w: %v", cache.ErrBlockPresenceTopologyChanged, err) + default: + return err + } +} + +func (c *embeddedBlockCacheClient) peerConnection(owner, address string) (*grpc.ClientConn, error) { + c.peerMu.Lock() + defer c.peerMu.Unlock() + if c.closed { + return nil, errors.New("embedded cache client is closed") + } + if peer, ok := c.peers[owner]; ok { + if peer.address == address { + return peer.conn, nil + } + _ = peer.conn.Close() + delete(c.peers, owner) + } + conn, err := grpc.NewClient(address, c.dialOptions...) + if err != nil { + return nil, err + } + c.peers[owner] = blockPresencePeer{address: address, conn: conn} + return conn, nil +} + +// Close releases the optional peer connections before stopping the embedded +// cache. The embedded cache close remains the caller-visible behavior. +func (c *embeddedBlockCacheClient) Close() error { + c.closeOnce.Do(func() { + c.peerMu.Lock() + c.closed = true + peers := c.peers + c.peers = nil + c.peerMu.Unlock() + + var closeErrors []error + for _, peer := range peers { + if err := peer.conn.Close(); err != nil { + closeErrors = append(closeErrors, err) + } + } + if c.Client != nil { + if err := c.Client.Close(); err != nil { + closeErrors = append(closeErrors, err) + } + } + c.closeErr = errors.Join(closeErrors...) + }) + return c.closeErr +} diff --git a/cmd/tag/block_cache_range_benchmark_test.go b/cmd/tag/block_cache_range_benchmark_test.go index 5954be8f..b9b83b65 100644 --- a/cmd/tag/block_cache_range_benchmark_test.go +++ b/cmd/tag/block_cache_range_benchmark_test.go @@ -35,14 +35,20 @@ const ( ) type embeddedBlockRangeBenchmarkFixture struct { - handler http.Handler - body []byte - key string - stats *embeddedBlockRangeRPCStats - requestSigner *auth.RequestSigner - accessKey string - secretKey string - region string + gateway *httptest.Server + client *http.Client + handler http.Handler + body []byte + key string + cacheConfig config.CacheConfig + cacheClient *embeddedBlockCacheClient + embeddedClient *embedded.Client + stats *embeddedBlockRangeRPCStats + requestSigner *auth.RequestSigner + gatewaySigner *auth.RequestSigner + accessKey string + secretKey string + region string } type benchmark206Writer struct { @@ -78,6 +84,7 @@ func (w *benchmark206Writer) Unwrap() http.ResponseWriter { return w.ResponseRec type embeddedBlockRangeRPCStats struct { presenceRPCs atomic.Int64 + presenceRequests atomic.Int64 presenceReplyBytes atomic.Int64 probeGetRPCs atomic.Int64 probePayloadBytes atomic.Int64 @@ -94,6 +101,13 @@ func (s *embeddedBlockRangeRPCStats) TagRPC(ctx context.Context, info *stats.RPC return context.WithValue(ctx, embeddedBlockRangeRPCKey{}, &embeddedBlockRangeRPCRecord{method: info.FullMethodName}) } +func (s *embeddedBlockRangeRPCStats) countBlockPresenceStream(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { + if method == benchmarkBlockPresenceMethod { + s.presenceRequests.Add(1) + } + return streamer(ctx, desc, cc, method, opts...) +} + func (s *embeddedBlockRangeRPCStats) HandleRPC(ctx context.Context, event stats.RPCStats) { record, _ := ctx.Value(embeddedBlockRangeRPCKey{}).(*embeddedBlockRangeRPCRecord) if record == nil { @@ -131,6 +145,7 @@ func (*embeddedBlockRangeRPCStats) HandleConn(context.Context, stats.ConnStats) func (s *embeddedBlockRangeRPCStats) reset() { s.presenceRPCs.Store(0) + s.presenceRequests.Store(0) s.presenceReplyBytes.Store(0) s.probeGetRPCs.Store(0) s.probePayloadBytes.Store(0) @@ -145,6 +160,14 @@ func newEmbeddedBlockRangeTransparentBenchmarkFixture(tb testing.TB, blockCount } func newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb testing.TB, blockCount int, transparent bool, withRPCStats ...bool) *embeddedBlockRangeBenchmarkFixture { + return newEmbeddedBlockRangeBenchmarkFixtureWithOptions(tb, blockCount, transparent, false, withRPCStats...) +} + +func newEmbeddedBlockRangeLegacyPeerFixture(tb testing.TB, blockCount int) *embeddedBlockRangeBenchmarkFixture { + return newEmbeddedBlockRangeBenchmarkFixtureWithOptions(tb, blockCount, false, true, true) +} + +func newEmbeddedBlockRangeBenchmarkFixtureWithOptions(tb testing.TB, blockCount int, transparent, legacyPeer bool, withRPCStats ...bool) *embeddedBlockRangeBenchmarkFixture { tb.Helper() if blockCount < 2 || blockCount > 32 { tb.Fatalf("benchmark block count %d outside 2..32", blockCount) @@ -180,6 +203,10 @@ func newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb testing.TB, blockCount int for i := range gossipAddrs { diskPath := tb.TempDir() writeEmbeddedBlockRangeRingTokens(tb, diskPath, i) + var presenceServer *blockPresenceServer + if !legacyPeer || i == 0 { + presenceServer = &blockPresenceServer{} + } embeddedCfg := &embedded.Config{ DiskPath: diskPath, TTL: cfg.Cache.TTL, @@ -195,6 +222,10 @@ func newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb testing.TB, blockCount int } if stats != nil { embeddedCfg.GRPCServerOptions = append(embeddedCfg.GRPCServerOptions, grpc.StatsHandler(stats)) + embeddedCfg.GRPCDialOptions = append(embeddedCfg.GRPCDialOptions, grpc.WithChainStreamInterceptor(stats.countBlockPresenceStream)) + } + if presenceServer != nil { + embeddedCfg.GRPCServerOptions = append(embeddedCfg.GRPCServerOptions, presenceServer.serverOption()) } if i > 0 { embeddedCfg.SeedNodes = []string{gossipAddrs[0]} @@ -203,9 +234,12 @@ func newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb testing.TB, blockCount int if err != nil { tb.Fatalf("create embedded range node %d: %v", i, err) } + if presenceServer != nil { + presenceServer.client = client + } nodes = append(nodes, client) if i == 0 { - blockCacheClient = newEmbeddedBlockCacheClient(client) + blockCacheClient = newEmbeddedBlockCacheClient(client, embeddedCfg.GRPCDialOptions...) tb.Cleanup(func() { _ = blockCacheClient.Close() }) } else { tb.Cleanup(func() { _ = client.Close() }) @@ -311,8 +345,10 @@ func newEmbeddedBlockRangeBenchmarkFixtureWithMode(tb testing.TB, blockCount int server := handlers.NewServer(service, "127.0.0.1", 0, false, cfg.Server.MaxInflightRequests) handler := server.Router() return &embeddedBlockRangeBenchmarkFixture{ - handler: handler, body: body, key: key, stats: stats, - requestSigner: requestSigner, accessKey: accessKey, secretKey: secretKey, region: cfg.Upstream.Region, + handler: handler, body: body, key: key, + cacheConfig: cfg.Cache, cacheClient: blockCacheClient, + embeddedClient: nodes[0], stats: stats, requestSigner: requestSigner, + accessKey: accessKey, secretKey: secretKey, region: cfg.Upstream.Region, } } diff --git a/cmd/tag/block_presence.go b/cmd/tag/block_presence.go new file mode 100644 index 00000000..553fa390 --- /dev/null +++ b/cmd/tag/block_presence.go @@ -0,0 +1,270 @@ +package main + +import ( + "context" + "errors" + "fmt" + "io" + "strconv" + "unicode/utf8" + + "github.com/tigrisdata/ocache/coordinator/ring" + "github.com/tigrisdata/ocache/embedded" + "github.com/tigrisdata/tag/cache" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/structpb" +) + +// blockPresenceMethod is a private bidi peer exchange carried by OCache's +// existing gRPC listener. Its request is {owner, decimal epoch, keys}; its +// response is {decimal epoch, found flags in key order}. +const ( + blockPresenceMethod = "/tag.cache.v1.BlockPresence/Check" + blockPresenceMaxKeys = 32 + blockPresenceMaxKeyBytes = 64 << 10 +) + +var errBlockPresenceRequestTooLarge = errors.New("block presence request is too large") + +type blockPresenceServer struct { + client *embedded.Client +} + +// serverOption adds the versioned private peer method to OCache's existing +// listener. Other unknown methods keep the gRPC Unimplemented response. +func (s *blockPresenceServer) serverOption() grpc.ServerOption { + return grpc.UnknownServiceHandler(s.serve) +} + +func (s *blockPresenceServer) serve(_ any, stream grpc.ServerStream) error { + method, ok := grpc.MethodFromServerStream(stream) + if !ok || method != blockPresenceMethod { + return status.Error(codes.Unimplemented, "unsupported cache method") + } + if s.client == nil { + return status.Error(codes.Unavailable, "embedded cache is not ready") + } + + var message structpb.Struct + if err := stream.RecvMsg(&message); err != nil { + return err + } + owner, epoch, keys, err := decodeBlockPresenceRequest(&message) + if err != nil { + return status.Error(codes.InvalidArgument, err.Error()) + } + coord := s.client.Coordinator() + if coord == nil || coord.GetLocalNodeID() != owner || coord.GetEpoch() != epoch { + return status.Error(codes.FailedPrecondition, "block presence owner changed") + } + + found, err := embeddedBlockPresence(stream.Context(), s.client, owner, epoch, keys) + if err != nil { + if errors.Is(err, cache.ErrBlockPresenceTopologyChanged) { + return status.Error(codes.FailedPrecondition, err.Error()) + } + if ctxErr := stream.Context().Err(); ctxErr != nil { + return status.FromContextError(ctxErr).Err() + } + return status.Errorf(codes.Internal, "block presence read: %v", err) + } + response, err := encodeBlockPresenceResponse(found, epoch) + if err != nil { + return status.Error(codes.Internal, err.Error()) + } + return stream.SendMsg(response) +} + +func decodeBlockPresenceRequest(message *structpb.Struct) (owner string, epoch uint64, keys []string, err error) { + if message == nil { + return "", 0, nil, errors.New("missing block presence request") + } + owner = message.GetFields()["owner"].GetStringValue() + if owner == "" { + return "", 0, nil, errors.New("missing block presence owner") + } + epochText := message.GetFields()["epoch"].GetStringValue() + if epochText == "" { + return "", 0, nil, errors.New("missing block presence epoch") + } + epoch, err = strconv.ParseUint(epochText, 10, 64) + if err != nil { + return "", 0, nil, fmt.Errorf("invalid block presence epoch: %w", err) + } + list := message.GetFields()["keys"].GetListValue() + if list == nil || len(list.Values) == 0 || len(list.Values) > blockPresenceMaxKeys { + return "", 0, nil, fmt.Errorf("block presence key count must be between 1 and %d", blockPresenceMaxKeys) + } + keys = make([]string, len(list.Values)) + keyBytes := 0 + for i, value := range list.Values { + if value == nil { + return "", 0, nil, fmt.Errorf("block presence key %d is missing", i) + } + key, ok := value.Kind.(*structpb.Value_StringValue) + if !ok || key.StringValue == "" { + return "", 0, nil, fmt.Errorf("block presence key %d is invalid", i) + } + keys[i] = key.StringValue + keyBytes += len(key.StringValue) + if keyBytes > blockPresenceMaxKeyBytes { + return "", 0, nil, errBlockPresenceRequestTooLarge + } + } + return owner, epoch, keys, nil +} + +func encodeBlockPresenceRequest(owner string, epoch uint64, keys []string) (*structpb.Struct, error) { + if owner == "" || len(keys) == 0 || len(keys) > blockPresenceMaxKeys { + return nil, fmt.Errorf("invalid block presence request: owner=%q keys=%d", owner, len(keys)) + } + keyValues := make([]any, len(keys)) + keyBytes := 0 + for i, key := range keys { + if key == "" { + return nil, fmt.Errorf("block presence key %d is empty", i) + } + if !utf8.ValidString(key) { + // The protobuf transport uses UTF-8 strings; preserve legacy handling for other keys. + return nil, cache.ErrBlockPresenceUnsupported + } + keyBytes += len(key) + if keyBytes > blockPresenceMaxKeyBytes { + return nil, errBlockPresenceRequestTooLarge + } + keyValues[i] = key + } + return structpb.NewStruct(map[string]any{ + "owner": owner, + "epoch": strconv.FormatUint(epoch, 10), + "keys": keyValues, + }) +} + +func encodeBlockPresenceResponse(found []bool, epoch uint64) (*structpb.Struct, error) { + if len(found) == 0 || len(found) > blockPresenceMaxKeys { + return nil, fmt.Errorf("invalid block presence response count %d", len(found)) + } + values := make([]any, len(found)) + for i, present := range found { + values[i] = present + } + return structpb.NewStruct(map[string]any{ + "epoch": strconv.FormatUint(epoch, 10), + "found": values, + }) +} + +func decodeBlockPresenceResponse(message *structpb.Struct, expectedEpoch uint64, expectedCount int) ([]bool, error) { + if message == nil { + return nil, errors.New("missing block presence response") + } + epochText := message.GetFields()["epoch"].GetStringValue() + epoch, err := strconv.ParseUint(epochText, 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid block presence response epoch: %w", err) + } + if epoch != expectedEpoch { + return nil, cache.ErrBlockPresenceTopologyChanged + } + list := message.GetFields()["found"].GetListValue() + if list == nil || len(list.Values) != expectedCount || len(list.Values) > blockPresenceMaxKeys { + return nil, fmt.Errorf("invalid block presence result count") + } + found := make([]bool, len(list.Values)) + for i, value := range list.Values { + if value == nil { + return nil, fmt.Errorf("block presence result %d is missing", i) + } + result, ok := value.Kind.(*structpb.Value_BoolValue) + if !ok { + return nil, fmt.Errorf("block presence result %d is not boolean", i) + } + found[i] = result.BoolValue + } + return found, nil +} + +// embeddedBlockPresence reads only the same byte-zero probe range as +// BlockExistsErr, on the node that currently owns every key. A short range with +// at least one byte is present; an empty range is absent; storage errors remain +// errors. Owner checks make a stale ring a retry, not a miss. +func embeddedBlockPresence(ctx context.Context, client *embedded.Client, owner string, epoch uint64, keys []string) ([]bool, error) { + if len(keys) == 0 || len(keys) > blockPresenceMaxKeys { + return nil, fmt.Errorf("invalid block presence key count %d", len(keys)) + } + coord := client.Coordinator() + if coord != nil { + if coord.GetLocalNodeID() != owner || coord.GetEpoch() != epoch { + return nil, cache.ErrBlockPresenceTopologyChanged + } + } else if owner != "" || epoch != 0 { + return nil, cache.ErrBlockPresenceTopologyChanged + } + + found := make([]bool, len(keys)) + for i, key := range keys { + if err := ctx.Err(); err != nil { + return nil, err + } + if coord != nil { + if err := checkBlockPresenceOwner(coord, key, owner); err != nil { + return nil, err + } + } + reader, exists, err := client.Operations().GetLocal(ctx, key, 0, 1) + if err != nil { + return nil, err + } + if !exists { + continue + } + if reader == nil { + return nil, fmt.Errorf("block %q returned a nil reader", key) + } + n, readErr := io.Copy(io.Discard, &io.LimitedReader{R: reader, N: 2}) + if closer, ok := reader.(io.Closer); ok { + _ = closer.Close() + } + if readErr != nil { + return nil, readErr + } + found[i] = n > 0 + } + + if err := ctx.Err(); err != nil { + return nil, err + } + if coord != nil { + if coord.GetEpoch() != epoch { + return nil, cache.ErrBlockPresenceTopologyChanged + } + for _, key := range keys { + if err := checkBlockPresenceOwner(coord, key, owner); err != nil { + return nil, err + } + } + } + return found, nil +} + +func checkBlockPresenceOwner(coord blockPresenceCoordinator, key, owner string) error { + node, err := coord.GetNodeForKey(key) + if err != nil { + return fmt.Errorf("resolve block owner for %q: %w", key, err) + } + if node.ID != owner { + return cache.ErrBlockPresenceTopologyChanged + } + return nil +} + +// blockPresenceCoordinator is the small, source-owned part of the embedded +// coordinator used by both the peer handler and its caller. +type blockPresenceCoordinator interface { + GetEpoch() uint64 + GetLocalNodeID() string + GetNodeForKey(string) (*ring.NodeInfo, error) +} diff --git a/cmd/tag/block_presence_test.go b/cmd/tag/block_presence_test.go new file mode 100644 index 00000000..33594703 --- /dev/null +++ b/cmd/tag/block_presence_test.go @@ -0,0 +1,774 @@ +package main + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + cacheclient "github.com/tigrisdata/ocache/client" + "github.com/tigrisdata/ocache/embedded" + "github.com/tigrisdata/tag/auth" + "github.com/tigrisdata/tag/cache" + "github.com/tigrisdata/tag/config" + "github.com/tigrisdata/tag/handlers" + "github.com/tigrisdata/tag/proxy" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/structpb" +) + +func TestBlockPresenceRequestFallsBackForNonUTF8Keys(t *testing.T) { + key := string([]byte{'b', 'l', 'k', '|', 0xff}) + if _, err := encodeBlockPresenceRequest("owner", 1, []string{key}); !errors.Is(err, cache.ErrBlockPresenceUnsupported) { + t.Fatalf("non-UTF8 key error = %v, want optional capability fallback", err) + } +} + +func TestBlockPresenceResponseCodecPreservesOrderAndRejectsInvalidResults(t *testing.T) { + want := []bool{true, false, true} + response, err := encodeBlockPresenceResponse(want, 7) + if err != nil { + t.Fatalf("encode response: %v", err) + } + got, err := decodeBlockPresenceResponse(response, 7, len(want)) + if err != nil { + t.Fatalf("decode response: %v", err) + } + if len(got) != len(want) { + t.Fatalf("decoded result count = %d, want %d", len(got), len(want)) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("decoded presence[%d] = %t, want %t", i, got[i], want[i]) + } + } + + wrongEpoch, err := encodeBlockPresenceResponse(want, 8) + if err != nil { + t.Fatalf("encode wrong-epoch response: %v", err) + } + if _, err := decodeBlockPresenceResponse(wrongEpoch, 7, len(want)); !errors.Is(err, cache.ErrBlockPresenceTopologyChanged) { + t.Errorf("wrong-epoch response error = %v, want topology change", err) + } + + for _, results := range [][]any{{true, false}, {"true", false, true}} { + invalidResponse, err := structpb.NewStruct(map[string]any{ + "epoch": "7", + "found": results, + }) + if err != nil { + t.Fatalf("create malformed response: %v", err) + } + if _, err := decodeBlockPresenceResponse(invalidResponse, 7, len(want)); err == nil { + t.Errorf("decoder accepted malformed results %v", results) + } + } +} + +func (f *embeddedBlockRangeBenchmarkFixture) startGateway(tb testing.TB) { + tb.Helper() + f.gateway = httptest.NewServer(f.handler) + tb.Cleanup(f.gateway.Close) + f.gatewaySigner = auth.NewRequestSigner(f.gateway.URL, f.region) + f.client = f.gateway.Client() + tb.Cleanup(f.client.CloseIdleConnections) +} + +type earlyUnimplementedPresenceClient struct { + cacheclient.CacheClient + peer *embeddedBlockCacheClient + address string + probeGets int +} + +func (c *earlyUnimplementedPresenceClient) BlockPresence(ctx context.Context, keys []string) ([]bool, error) { + return c.peer.remoteBlockPresence(ctx, "legacy-owner", c.address, 0, keys) +} + +func (c *earlyUnimplementedPresenceClient) GetRangeStream(ctx context.Context, key string, start, end int64, w io.Writer) error { + if strings.HasPrefix(key, "blk|") && start == 0 && end == 1 { + c.probeGets++ + } + return c.CacheClient.GetRangeStream(ctx, key, start, end, w) +} + +type headerBarrierPresenceStream struct { + grpc.ClientStream +} + +func (s *headerBarrierPresenceStream) SendMsg(message any) error { + _, _ = s.ClientStream.Header() + return s.ClientStream.SendMsg(message) +} + +func TestEarlyUnimplementedPeerFallsBackToPerKeyProbe(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + server := grpc.NewServer() + go func() { _ = server.Serve(listener) }() + defer server.Stop() + defer listener.Close() + + streamInterceptor := func(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { + stream, err := streamer(ctx, desc, cc, method, opts...) + if err != nil { + return nil, err + } + return &headerBarrierPresenceStream{ClientStream: stream}, nil + } + peer := newEmbeddedBlockCacheClient(nil, grpc.WithStreamInterceptor(streamInterceptor)) + defer peer.Close() + + memory := cacheclient.NewMemoryCache() + const bucket, object, etag = "b", "k", `"v1"` + key := cache.MakeBlockKey(bucket, object, etag, 4, 0) + if err := memory.Put(context.Background(), key, []byte("AB"), 0); err != nil { + t.Fatal(err) + } + client := &earlyUnimplementedPresenceClient{ + CacheClient: memory, + peer: peer, + address: listener.Addr().String(), + } + cfg := config.NewDefault() + store := cache.NewCacheWithClient(client, &cfg.Cache) + + present, err := store.BlockExistsBatchErr(context.Background(), bucket, object, etag, 4, []int64{0}) + if err != nil || len(present) != 1 || !present[0] { + t.Fatalf("presence after early Unimplemented = %v, err=%v; want legacy present result", present, err) + } + if client.probeGets != 1 { + t.Fatalf("legacy per-key probes = %d, want 1", client.probeGets) + } +} + +// A real embedded peer without the optional method must still serve a warm +// multi-block Range through its ordinary Get RPCs after rejecting the batch. +func TestEarlyUnimplementedPeerServesWarmMultiBlockRange(t *testing.T) { + fixture := newEmbeddedBlockRangeLegacyPeerFixture(t, 2) + store := cache.NewCacheWithClient(fixture.cacheClient, &fixture.cacheConfig) + meta, found, err := store.GetMeta(context.Background(), "benchmark", fixture.key) + if err != nil || !found || meta == nil || meta.BlockSize != fixture.cacheConfig.BlockSize { + t.Fatalf("seeded metadata = (%v,%v,%v), want cached two-block metadata", meta, found, err) + } + present, err := store.BlockExistsBatchErr(context.Background(), "benchmark", fixture.key, meta.ETag, meta.BlockSize, []int64{0, 1}) + if err != nil || len(present) != 2 || !present[0] || !present[1] { + t.Fatalf("legacy peer presence fallback = (%v,%v), want two present blocks", present, err) + } + if got := fixture.stats.presenceRequests.Load(); got != 1 { + t.Fatalf("legacy peer block-presence attempts = %d, want one rejected exchange", got) + } + if got := fixture.stats.probeGetRPCs.Load(); got != 1 { + t.Fatalf("legacy peer per-key probe RPCs = %d, want one remote probe", got) + } + if got := fixture.stats.probePayloadBytes.Load(); got != 2 { + t.Fatalf("legacy peer probe payload bytes = %d, want 2", got) + } + + fixture.stats.reset() + response := fixture.doRange(t, nil) + fixture.verifyResponse(t, response) + if got := fixture.stats.presenceRequests.Load(); got != 1 { + t.Fatalf("warm Range made %d legacy batch attempts, want 1", got) + } + if got := fixture.stats.probeGetRPCs.Load(); got != 1 || fixture.stats.probePayloadBytes.Load() != 2 { + t.Fatalf("warm Range fallback probes = (%d RPCs, %d bytes), want (1, 2)", got, fixture.stats.probePayloadBytes.Load()) + } +} + +type blockPresenceCallRecorder struct { + *embeddedBlockCacheClient + pageSizes []int +} + +func (c *blockPresenceCallRecorder) BlockPresence(ctx context.Context, keys []string) ([]bool, error) { + c.pageSizes = append(c.pageSizes, len(keys)) + return c.embeddedBlockCacheClient.BlockPresence(ctx, keys) +} + +type legacyBlockPresenceRecorder struct { + *embedded.Client + probeCalls int + probeBytes int64 +} + +type wrongOwnerBlockPresenceClient struct { + *embeddedBlockCacheClient + remoteKey string + remoteAddress string + epoch uint64 +} + +func (c *wrongOwnerBlockPresenceClient) BlockPresence(ctx context.Context, _ []string) ([]bool, error) { + return c.remoteBlockPresence(ctx, "stale-owner", c.remoteAddress, c.epoch, []string{c.remoteKey}) +} + +func (c *legacyBlockPresenceRecorder) GetRangeStream(ctx context.Context, key string, start, end int64, w io.Writer) error { + if strings.HasPrefix(key, "blk|") && start == 0 && end == 1 { + c.probeCalls++ + counter := &blockPresenceCountingWriter{Writer: w} + err := c.Client.GetRangeStream(ctx, key, start, end, counter) + c.probeBytes += counter.written + return err + } + return c.Client.GetRangeStream(ctx, key, start, end, w) +} + +type blockPresenceCountingWriter struct { + io.Writer + written int64 +} + +func (w *blockPresenceCountingWriter) Write(p []byte) (int, error) { + n, err := w.Writer.Write(p) + w.written += int64(n) + return n, err +} + +// The real embedded cluster path returns ordered bits with one remote exchange +// per owner, while a plain CacheClient retains the old one-stream-per-key path. +func TestEmbeddedBlockPresenceMultiNodeAndFallback(t *testing.T) { + fixture := newEmbeddedBlockRangeBenchmarkFixture(t, 32, true) + fixture.startGateway(t) + ctx := context.Background() + const ( + bucket = "benchmark" + etag = `"block-range"` + ) + indices := func(count int) []int64 { + idxs := make([]int64, count) + for i := range idxs { + idxs[i] = int64(i) + } + return idxs + } + recordRemoteOwners := func(idxs []int64) (remoteBlocks int, owners map[string]struct{}) { + owners = make(map[string]struct{}) + localID := fixture.embeddedClient.Coordinator().GetLocalNodeID() + for _, idx := range idxs { + key := cache.MakeBlockKey(bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, idx) + owner, err := fixture.embeddedClient.Coordinator().GetNodeForKey(key) + if err != nil { + t.Fatalf("resolve block %d owner: %v", idx, err) + } + if owner.ID != localID { + remoteBlocks++ + owners[owner.ID] = struct{}{} + } + } + return remoteBlocks, owners + } + + recorded := &blockPresenceCallRecorder{embeddedBlockCacheClient: fixture.cacheClient} + batchCache := cache.NewCacheWithClient(recorded, &fixture.cacheConfig) + legacy := &legacyBlockPresenceRecorder{Client: fixture.embeddedClient} + fallbackCache := cache.NewCacheWithClient(legacy, &fixture.cacheConfig) + rangeConfig := config.NewDefault() + rangeConfig.Cache.SetBlockCachingEnabled(true) + rangeConfig.Cache.BlockSize = embeddedBlockRangeBenchmarkBlockSize + rangeConfig.Cache.SizeThreshold = int64(len(fixture.body)) + fallbackService := proxy.NewService(embeddedCacheHitBenchmarkForwarder{}, fallbackCache, rangeConfig) + fallbackHandler := handlers.NewServer(fallbackService, "127.0.0.1", 0, false, rangeConfig.Server.MaxInflightRequests) + fallbackGateway := httptest.NewServer(fallbackHandler.Router()) + t.Cleanup(fallbackGateway.Close) + fallbackHTTP := fallbackGateway.Client() + t.Cleanup(fallbackHTTP.CloseIdleConnections) + + for _, blockCount := range []int{2, 4, 8, 32} { + t.Run(fmt.Sprintf("%d_blocks", blockCount), func(t *testing.T) { + idxs := indices(blockCount) + remoteBlocks, remoteOwners := recordRemoteOwners(idxs) + if remoteBlocks == 0 || len(remoteOwners) != 1 { + t.Fatalf("%d-block range has %d remote blocks across %d owners; want a remote owner", blockCount, remoteBlocks, len(remoteOwners)) + } + recorded.pageSizes = nil + fixture.stats.reset() + got, err := batchCache.BlockExistsBatchErr(ctx, bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, idxs) + if err != nil { + t.Fatalf("batched presence: %v", err) + } + for i, present := range got { + if !present { + t.Errorf("batched presence[%d] = false, want present", i) + } + } + if want := []int{blockCount}; !equalInts(recorded.pageSizes, want) { + t.Errorf("batch page sizes = %v, want %v", recorded.pageSizes, want) + } + if got := fixture.stats.presenceRPCs.Load(); got != int64(len(remoteOwners)) { + t.Errorf("remote presence RPCs = %d, want one for each of %d owners", got, len(remoteOwners)) + } + if len(remoteOwners) == 0 { + if got := fixture.stats.presenceReplyBytes.Load(); got != 0 { + t.Errorf("local-only page returned %d remote response bytes", got) + } + } else if got := fixture.stats.presenceReplyBytes.Load(); got == 0 || got > int64(len(remoteOwners))*4096 { + t.Errorf("presence response payload bytes = %d, want a compact nonempty result", got) + } + if got := fixture.stats.probeGetRPCs.Load(); got != 0 { + t.Errorf("batched path opened %d per-key probe RPCs, want 0", got) + } + if got := fixture.stats.probePayloadBytes.Load(); got != 0 { + t.Errorf("batched path transferred %d probe block bytes, want 0", got) + } + + legacy.probeCalls = 0 + legacy.probeBytes = 0 + fixture.stats.reset() + fallback, err := fallbackCache.BlockExistsBatchErr(ctx, bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, idxs) + if err != nil { + t.Fatalf("fallback presence: %v", err) + } + for i, present := range fallback { + if !present { + t.Errorf("fallback presence[%d] = false, want present", i) + } + } + if legacy.probeCalls != blockCount { + t.Errorf("fallback per-key probe calls = %d, want %d", legacy.probeCalls, blockCount) + } + if legacy.probeBytes != int64(2*blockCount) { + t.Errorf("fallback probe payload bytes = %d, want %d", legacy.probeBytes, 2*blockCount) + } + if got := fixture.stats.probeGetRPCs.Load(); got != int64(remoteBlocks) { + t.Errorf("fallback remote probe RPCs = %d, want %d", got, remoteBlocks) + } + if got := fixture.stats.probePayloadBytes.Load(); got != int64(2*remoteBlocks) { + t.Errorf("fallback remote probe bytes = %d, want %d", got, 2*remoteBlocks) + } + if got := fixture.stats.presenceRPCs.Load(); got != 0 { + t.Errorf("fallback path used %d batch presence RPCs, want 0", got) + } + + fixture.stats.reset() + assertEmbeddedRangeResponse(t, fixture.client, fixture.gateway.URL, fixture.key, fixture.body, blockCount, fixture.gatewaySigner, fixture.accessKey, fixture.secretKey) + if got := fixture.stats.presenceRPCs.Load(); got != int64(len(remoteOwners)) { + t.Errorf("range handler presence RPCs = %d, want one per remote owner (%d)", got, len(remoteOwners)) + } + if got := fixture.stats.probeGetRPCs.Load(); got != 0 { + t.Errorf("range handler opened %d per-key probe RPCs", got) + } + + legacy.probeCalls = 0 + legacy.probeBytes = 0 + fixture.stats.reset() + assertEmbeddedRangeResponse(t, fallbackHTTP, fallbackGateway.URL, fixture.key, fixture.body, blockCount, nil, "", "") + if legacy.probeCalls != blockCount || legacy.probeBytes != int64(2*blockCount) { + t.Errorf("fallback range preflight = (%d streams, %d bytes), want (%d, %d)", legacy.probeCalls, legacy.probeBytes, blockCount, 2*blockCount) + } + }) + } + + // A missing key stays a zero-payload miss in both paths and retains input + // order. The range caller can then apply its existing fetch cap. + missing := int64(31) + missingKey := cache.MakeBlockKey(bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, missing) + if err := fixture.embeddedClient.Delete(ctx, missingKey); err != nil { + t.Fatalf("delete block %d: %v", missing, err) + } + all := indices(32) + fixture.stats.reset() + batch, err := batchCache.BlockExistsBatchErr(ctx, bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, all) + if err != nil { + t.Fatalf("batched presence after eviction: %v", err) + } + if batch[missing] || !batch[0] { + t.Fatalf("batched order/presence at indexes 0,%d = %v, want present then absent", missing, batch) + } + legacy.probeCalls = 0 + legacy.probeBytes = 0 + fixture.stats.reset() + fallback, err := fallbackCache.BlockExistsBatchErr(ctx, bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, all) + if err != nil { + t.Fatalf("fallback presence after eviction: %v", err) + } + if fallback[missing] || !fallback[0] { + t.Fatalf("fallback order/presence at indexes 0,%d = %v, want present then absent", missing, fallback) + } + if legacy.probeCalls != len(all) { + t.Errorf("fallback probes after one eviction = %d, want %d", legacy.probeCalls, len(all)) + } + if want := int64(2 * (len(all) - 1)); legacy.probeBytes != want { + t.Errorf("fallback probe payload bytes after one eviction = %d, want %d (the missing block contributes zero)", legacy.probeBytes, want) + } + + // A peer must reject a request naming the wrong owner; the cache wrapper + // converts that response into a discarded page, never an absent bit. + coord := fixture.embeddedClient.Coordinator() + var remoteKey, remoteAddress string + for _, idx := range all { + candidate := cache.MakeBlockKey(bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, idx) + owner, err := coord.GetNodeForKey(candidate) + if err != nil { + t.Fatalf("resolve block owner: %v", err) + } + if owner.ID != coord.GetLocalNodeID() { + remoteKey, remoteAddress = candidate, owner.ListenAddress + break + } + } + if remoteKey == "" { + t.Fatal("fixture has no remote-owned block") + } + _, err = fixture.cacheClient.remoteBlockPresence(ctx, "wrong-owner", remoteAddress, coord.GetEpoch(), []string{remoteKey}) + if !errors.Is(err, cache.ErrBlockPresenceTopologyChanged) { + t.Fatalf("wrong-owner response = %v, want topology-change fallback", err) + } + fixture.stats.reset() + staleOwnerClient := &wrongOwnerBlockPresenceClient{ + embeddedBlockCacheClient: fixture.cacheClient, + remoteKey: remoteKey, + remoteAddress: remoteAddress, + epoch: coord.GetEpoch(), + } + staleOwnerCache := cache.NewCacheWithClient(staleOwnerClient, &fixture.cacheConfig) + remoteIndex, err := strconv.ParseInt(remoteKey[strings.LastIndexByte(remoteKey, '|')+1:], 10, 64) + if err != nil { + t.Fatalf("parse remote block index: %v", err) + } + retried, err := staleOwnerCache.BlockExistsBatchErr(ctx, bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, []int64{remoteIndex}) + if err != nil || len(retried) != 1 || !retried[0] { + t.Fatalf("legacy probe after owner mismatch = %v, err=%v; want present", retried, err) + } + if got := fixture.stats.probeGetRPCs.Load(); got != 1 { + t.Errorf("owner mismatch retry opened %d legacy Get RPCs, want 1", got) + } + + canceled, cancel := context.WithCancel(ctx) + cancel() + _, err = batchCache.BlockExistsBatchErr(canceled, bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, []int64{0}) + if !errors.Is(err, context.Canceled) { + t.Fatalf("canceled presence = %v, want context.Canceled", err) + } +} + +func TestEmbeddedBlockPresenceUsesClusterGRPCAuth(t *testing.T) { + token := auth.DeriveGRPCAuthToken("access", "secret") + gossip := []string{embeddedBlockRangeFreeAddress(t), embeddedBlockRangeFreeAddress(t)} + grpcAddrs := []string{embeddedBlockRangeFreeAddress(t), embeddedBlockRangeFreeAddress(t)} + clients := make([]*embedded.Client, 2) + wrappers := make([]*embeddedBlockCacheClient, 2) + for i := range clients { + presenceServer := &blockPresenceServer{} + serverOptions := auth.GRPCServerOptions(token) + serverOptions = append(serverOptions, presenceServer.serverOption()) + cfg := &embedded.Config{ + DiskPath: t.TempDir(), + TTL: time.Minute, + NodeID: fmt.Sprintf("auth-presence-%d", i), + ClusterAddr: gossip[i], + GRPCAddr: grpcAddrs[i], + AdvertiseAddr: grpcAddrs[i], + GRPCServerOptions: serverOptions, + GRPCDialOptions: auth.GRPCDialOptions(token), + Registerer: prometheus.NewRegistry(), + } + if i > 0 { + cfg.SeedNodes = []string{gossip[0]} + } + client, err := embedded.New(cfg) + if err != nil { + t.Fatalf("create authenticated embedded node %d: %v", i, err) + } + presenceServer.client = client + clients[i] = client + wrappers[i] = newEmbeddedBlockCacheClient(client, cfg.GRPCDialOptions...) + wrapper := wrappers[i] + t.Cleanup(func() { _ = wrapper.Close() }) + if err := client.StartGRPCServer(); err != nil { + t.Fatalf("start authenticated embedded node %d: %v", i, err) + } + } + readyCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + for _, client := range clients { + if err := client.WaitReady(readyCtx); err != nil { + t.Fatalf("authenticated embedded node not ready: %v", err) + } + } + for { + ready := true + for _, client := range clients { + if len(client.Coordinator().GetRing().GetActiveNodes()) != len(clients) { + ready = false + break + } + } + if ready { + break + } + select { + case <-readyCtx.Done(): + t.Fatalf("authenticated embedded cluster did not converge: %v", readyCtx.Err()) + case <-time.After(20 * time.Millisecond): + } + } + + cfg := config.NewDefault() + cfg.Cache.SetBlockCachingEnabled(true) + cfg.Cache.BlockSize = embeddedBlockRangeBenchmarkBlockSize + store := cache.NewCacheWithClient(wrappers[0], &cfg.Cache) + const bucket, key, etag = "auth-presence", "object", `"auth"` + var remoteIndex int64 = -1 + var remoteAddress string + for i := int64(0); i < 1000; i++ { + blockKey := cache.MakeBlockKey(bucket, key, etag, embeddedBlockRangeBenchmarkBlockSize, i) + owner, err := clients[0].Coordinator().GetNodeForKey(blockKey) + if err != nil { + t.Fatalf("resolve authenticated block owner: %v", err) + } + if owner.ID != clients[0].Coordinator().GetLocalNodeID() { + remoteIndex, remoteAddress = i, owner.ListenAddress + break + } + } + if remoteIndex < 0 { + t.Fatal("authenticated cluster has no remote-owned key") + } + if err := store.PutBlock(context.Background(), bucket, key, etag, embeddedBlockRangeBenchmarkBlockSize, remoteIndex, []byte("AB"), 60); err != nil { + t.Fatalf("store authenticated remote block: %v", err) + } + present, err := store.BlockExistsBatchErr(context.Background(), bucket, key, etag, embeddedBlockRangeBenchmarkBlockSize, []int64{remoteIndex}) + if err != nil || len(present) != 1 || !present[0] { + t.Fatalf("authenticated remote presence = %v, err=%v, want present", present, err) + } + if _, err := wrappers[0].remoteBlockPresence(context.Background(), "wrong-owner", remoteAddress, clients[0].Coordinator().GetEpoch(), []string{cache.MakeBlockKey(bucket, key, etag, embeddedBlockRangeBenchmarkBlockSize, remoteIndex)}); !errors.Is(err, cache.ErrBlockPresenceTopologyChanged) { + t.Fatalf("authenticated wrong-owner response = %v, want topology mismatch", err) + } +} + +func assertEmbeddedRangeResponse(tb testing.TB, client *http.Client, gatewayURL, key string, object []byte, blockCount int, signer *auth.RequestSigner, accessKey, secretKey string) { + tb.Helper() + rangeLen := blockCount * embeddedBlockRangeBenchmarkBlockSize + req, err := http.NewRequest(http.MethodGet, gatewayURL+"/benchmark/"+key, nil) + if err != nil { + tb.Fatal(err) + } + req.Header.Set("Range", fmt.Sprintf("bytes=0-%d", rangeLen-1)) + if signer == nil { + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test/20260101/us-east-1/s3/aws4_request, Signature=deadbeef") + } else { + signed, err := signer.SignRequest(req.Context(), req.Method, req.URL.RequestURI(), req.Body, "", accessKey, secretKey, req.Header) + if err != nil { + tb.Fatalf("sign range request: %v", err) + } + signed.RemoteAddr = req.RemoteAddr + req = signed + } + resp, err := client.Do(req) + if err != nil { + tb.Fatalf("range request: %v", err) + } + body, readErr := io.ReadAll(resp.Body) + closeErr := resp.Body.Close() + if readErr != nil { + tb.Fatalf("read range response: %v", readErr) + } + if closeErr != nil { + tb.Fatalf("close range response: %v", closeErr) + } + if resp.StatusCode != http.StatusPartialContent || resp.Header.Get(proxy.XCacheHeader) != proxy.XCacheHit || !bytes.Equal(body, object[:rangeLen]) { + tb.Fatalf("range response = (status=%d, cache=%q, bytes=%d, exact=%t), want exact cache 206", resp.StatusCode, resp.Header.Get(proxy.XCacheHeader), len(body), bytes.Equal(body, object[:rangeLen])) + } + if got, want := resp.Header.Get("Content-Range"), fmt.Sprintf("bytes 0-%d/%d", rangeLen-1, len(object)); got != want { + tb.Errorf("Content-Range=%q, want %q", got, want) + } + if got, want := resp.Header.Get("Content-Length"), fmt.Sprint(rangeLen); got != want { + tb.Errorf("Content-Length=%q, want %q", got, want) + } + if got := resp.Header.Get("ETag"); got != `"block-range"` { + tb.Errorf("ETag=%q, want %q", got, `"block-range"`) + } +} + +func TestBlockPresencePeerCancellationIsFatalUnlessSiblingCanceledGroup(t *testing.T) { + peerError := status.Error(codes.Canceled, "peer canceled the presence exchange") + callerCtx := context.Background() + if isSiblingBlockPresenceCancellation(callerCtx, context.Background(), peerError) { + t.Fatal("standalone peer cancellation was classified as a sibling cancellation") + } + + siblingCtx, cancelSibling := context.WithCancel(callerCtx) + cancelSibling() + if !isSiblingBlockPresenceCancellation(callerCtx, siblingCtx, peerError) { + t.Fatal("cancellation after a sibling failure was not recognized") + } + + canceledCaller, cancelCaller := context.WithCancel(callerCtx) + cancelCaller() + if isSiblingBlockPresenceCancellation(canceledCaller, canceledCaller, peerError) { + t.Fatal("caller cancellation was classified as a sibling cancellation") + } +} + +func TestEmbeddedBlockPresencePeerCancellationIsError(t *testing.T) { + gossip := []string{embeddedBlockRangeFreeAddress(t), embeddedBlockRangeFreeAddress(t)} + grpcAddrs := []string{embeddedBlockRangeFreeAddress(t), embeddedBlockRangeFreeAddress(t)} + clients := make([]*embedded.Client, 2) + var cacheClient *embeddedBlockCacheClient + for i := range clients { + cfg := &embedded.Config{ + DiskPath: t.TempDir(), + TTL: time.Minute, + NodeID: fmt.Sprintf("cancel-presence-%d", i), + ClusterAddr: gossip[i], + GRPCAddr: grpcAddrs[i], + AdvertiseAddr: grpcAddrs[i], + Registerer: prometheus.NewRegistry(), + } + if i > 0 { + cfg.SeedNodes = []string{gossip[0]} + cfg.GRPCServerOptions = append(cfg.GRPCServerOptions, grpc.UnknownServiceHandler(func(_ any, stream grpc.ServerStream) error { + method, ok := grpc.MethodFromServerStream(stream) + if !ok || method != blockPresenceMethod { + return status.Error(codes.Unimplemented, "unsupported method") + } + var request structpb.Struct + if err := stream.RecvMsg(&request); err != nil { + return err + } + return status.Error(codes.Canceled, "simulated owner cancellation") + })) + } + client, err := embedded.New(cfg) + if err != nil { + t.Fatalf("create cancellation-test node %d: %v", i, err) + } + clients[i] = client + if i == 0 { + cacheClient = newEmbeddedBlockCacheClient(client) + wrapper := cacheClient + t.Cleanup(func() { _ = wrapper.Close() }) + } else { + node := client + t.Cleanup(func() { _ = node.Close() }) + } + if err := client.StartGRPCServer(); err != nil { + t.Fatalf("start cancellation-test node %d: %v", i, err) + } + } + + readyCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + for _, client := range clients { + if err := client.WaitReady(readyCtx); err != nil { + t.Fatalf("cancellation-test node not ready: %v", err) + } + } + for { + ready := true + for _, client := range clients { + if len(client.Coordinator().GetRing().GetActiveNodes()) != len(clients) { + ready = false + break + } + } + if ready { + break + } + select { + case <-readyCtx.Done(): + t.Fatalf("cancellation-test cluster did not converge: %v", readyCtx.Err()) + case <-time.After(20 * time.Millisecond): + } + } + + cfg := config.NewDefault() + cfg.Cache.SetBlockCachingEnabled(true) + cfg.Cache.BlockSize = embeddedBlockRangeBenchmarkBlockSize + store := cache.NewCacheWithClient(cacheClient, &cfg.Cache) + const bucket, object, etag = "cancel-presence", "object", `"cancel"` + var remoteIndex int64 = -1 + for i := int64(0); i < 1000; i++ { + candidate := cache.MakeBlockKey(bucket, object, etag, embeddedBlockRangeBenchmarkBlockSize, i) + owner, err := clients[0].Coordinator().GetNodeForKey(candidate) + if err != nil { + t.Fatalf("resolve cancellation-test owner: %v", err) + } + if owner.ID != clients[0].Coordinator().GetLocalNodeID() { + remoteIndex = i + break + } + } + if remoteIndex < 0 { + t.Fatal("cancellation-test cluster has no remote key") + } + + _, err := store.BlockExistsBatchErr(context.Background(), bucket, object, etag, embeddedBlockRangeBenchmarkBlockSize, []int64{remoteIndex}) + if err == nil || status.Code(err) != codes.Canceled { + t.Fatalf("peer cancellation result = (err=%v), want a canceled error rather than an absent bit", err) + } +} + +func TestEmbeddedBlockPresenceCancelsInFlightOwnerRPC(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + started := make(chan struct{}) + server := grpc.NewServer(grpc.UnknownServiceHandler(func(_ any, stream grpc.ServerStream) error { + method, ok := grpc.MethodFromServerStream(stream) + if !ok || method != blockPresenceMethod { + return status.Error(codes.Unimplemented, "unsupported method") + } + var request structpb.Struct + if err := stream.RecvMsg(&request); err != nil { + return err + } + close(started) + <-stream.Context().Done() + return status.FromContextError(stream.Context().Err()).Err() + })) + go func() { _ = server.Serve(listener) }() + defer server.Stop() + defer listener.Close() + + client := newEmbeddedBlockCacheClient(nil) + defer client.Close() + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { + _, err := client.remoteBlockPresence(ctx, "owner", listener.Addr().String(), 0, []string{"blk|b|k|e|4|0"}) + done <- err + }() + select { + case <-started: + case <-time.After(2 * time.Second): + cancel() + t.Fatal("presence RPC never reached the owner") + } + cancel() + select { + case err := <-done: + if !errors.Is(err, context.Canceled) { + t.Fatalf("canceled owner RPC error = %v, want context.Canceled", err) + } + case <-time.After(2 * time.Second): + t.Fatal("canceled presence RPC did not return") + } +} + +func equalInts(got, want []int) bool { + if len(got) != len(want) { + return false + } + for i := range got { + if got[i] != want[i] { + return false + } + } + return true +} diff --git a/cmd/tag/main.go b/cmd/tag/main.go index 66b0d15d..d11473c8 100644 --- a/cmd/tag/main.go +++ b/cmd/tag/main.go @@ -255,6 +255,8 @@ func main() { CompactionBytesPerSecond: cfg.Cache.CompactionBytesPerSecond, } + presenceServer := &blockPresenceServer{} + // Configure gRPC auth for cache cluster communication if cfg.Cache.IsGRPCAuthEnabled() { accessKey := os.Getenv("AWS_ACCESS_KEY_ID") @@ -267,12 +269,15 @@ func main() { embeddedCfg.GRPCDialOptions = auth.GRPCDialOptions(grpcToken) log.Info().Msg("Cache gRPC auth enabled") } + embeddedCfg.GRPCServerOptions = append(embeddedCfg.GRPCServerOptions, presenceServer.serverOption()) embeddedCache, err := embedded.New(embeddedCfg) if err != nil { log.Fatal().Err(err).Msg("Failed to initialize embedded cache") } - defer embeddedCache.Close() + presenceServer.client = embeddedCache + blockCacheClient := newEmbeddedBlockCacheClient(embeddedCache, embeddedCfg.GRPCDialOptions...) + defer blockCacheClient.Close() // Start gRPC server for cluster routing if err := embeddedCache.StartGRPCServer(); err != nil { @@ -288,7 +293,7 @@ func main() { readyCancel() // Wrap embedded cache with the cache.Cache interface. - objectCache = cache.NewCacheWithClient(newEmbeddedBlockCacheClient(embeddedCache), &cfg.Cache) + objectCache = cache.NewCacheWithClient(blockCacheClient, &cfg.Cache) if cfg.Cache.IsLegacyCoordination() { log.Info().Msg("Cache meta coordination: legacy tombstones (set cache.legacy_coordination=false for fenced CAS once all nodes support it)") } else { diff --git a/docs/rfcs/0001-block-aligned-caching.md b/docs/rfcs/0001-block-aligned-caching.md index 876c74f5..a03088b1 100644 --- a/docs/rfcs/0001-block-aligned-caching.md +++ b/docs/rfcs/0001-block-aligned-caching.md @@ -84,7 +84,7 @@ Readers dispatch on `meta.BlockSize`: `0` → existing `serveRangeFromCache`/`se 1. Parse the range (`parseRangeHeader`). Preserve the existing `serveRangeFromCache` error semantics *before* computing any block indices: a malformed/unsatisfiable range (parse error) or an empty range list → `416` with `Content-Range: bytes */`; a **multi-range** request (`len(ranges) > 1`) → `416` (multi-range from cache is unsupported today and stays so). Only a single, valid range proceeds; compute its covering block indices `b0 = start/BlockSize … bK = end/BlockSize`. 2. **Assembled fast path** (`serveAssembledRange`, for a range no longer than one block — the footer/row-group pattern, at most two covering blocks): the requested bytes are assembled into a pooled buffer **before anything is committed**. Each present block is read exactly once — the read itself is the existence probe, halving warm-serve cache ops versus probe-then-stream — and blocks the read reports absent are fetched (coalesced, bounded) and re-read, all pre-commit, so every failure falls through to upstream with the response untouched. The buffer reserves its actual size against the serve-staging budget (see Memory bounds); on decline the probe-first path serves instead. -3. **Probe-first path** (larger ranges, or a staging-budget decline): probe each covering block with `BlockExistsErr` — a transient probe failure aborts rather than counting as missing, so a network blip can't masquerade as eviction — then fetch the missing ones and stream. At most `maxRangeBlockFanout` (32) absent blocks are fetched per request; a pathologically large client range with more absent blocks than that is served from a single upstream range GET instead of a fetch storm. +3. **Probe-first path** (larger ranges, or a staging-budget decline): check covering blocks in bounded pages of at most `maxRangeBlockFanout` (32). Embedded clients group each page by its current block owner and exchange compact ordered presence results once per remote owner; owners verify placement and read only the byte-zero probe range. If a peer lacks this optional capability, or an owner/epoch change invalidates a page, retry that page through the existing per-key `BlockExistsErr` path. Other routing, storage, and cancellation failures abort preflight rather than counting as missing, so a network blip can't masquerade as eviction. The range path stops once more than `maxRangeBlockFanout` blocks are confirmed absent, then serves from one upstream range GET instead of a fetch storm. Full-object probes retain their complete scan and existing majority-missing gate. 4. **Pipelined streaming**: multi-block spans use a bounded ordered prefetch window (at most `maxRangeBlockFanout`, currently 32 buffers). Independent cache reads begin concurrently, which lets blocks routed to different cache owners overlap; the write loop still commits block *i* before block *i+1*. Workers perform only the immutable cache read. A miss is recovered in write order, preserving the shared inline-fetch cap and first-error behavior. The largest two-to-32-buffer window that fits the staging budget is used; otherwise, and for single-block spans, serving degrades to the direct sequential stream. ### Block fetch and populate diff --git a/proxy/block_cache.go b/proxy/block_cache.go index 0581d568..67b03157 100644 --- a/proxy/block_cache.go +++ b/proxy/block_cache.go @@ -1403,39 +1403,66 @@ func (s *Service) invalidateStaleMeta(bucket, key, staleETag string) { } } -// ensureBlocksCached makes covering blocks [b0,bK] present in cache: it probes the range once, -// fetches any that are missing, and records per-block hits/misses plus whether the serve was a -// full or partial hit. It returns an error only when a missing block could not be populated -// (budget shed, upstream/ETag failure) — the caller then falls through to upstream. +// ensureBlocksCached makes covering blocks [b0,bK] present in cache, fetches any that are missing, +// and records per-block hits/misses plus whether the serve was a full or partial hit. It returns an +// error only when a missing block could not be populated (budget shed, upstream/ETag failure) — the +// caller then falls through to upstream. // // It returns errBlockAssemblyWouldAmplify — without fetching or recording serve metrics — when // fanning out into per-block fetches would be a large amplification versus one streaming fetch, so // the caller falls through instead. Two amplification gates: bailIfMostlyMissing (the full-object // serve path) bails when a MAJORITY of covering blocks are absent (assemble only a mostly-cached -// object, else stream it once); maxFetchFanout>0 (the range serve path) bails on an ABSOLUTE count -// of absent blocks, so a footer/row-group read still assembles its few blocks but a pathologically -// large client range doesn't fan out into hundreds of aligned GETs. +// object, else stream it once); maxFetchFanout>0 (the range serve path) probes bounded pages and +// bails on an ABSOLUTE count of absent blocks, so a pathologically large range does not fan out into +// hundreds of aligned GETs. Range pages use the optional owner-grouped presence operation when the +// cache client supports it; clients without it retain the per-key BlockExistsErr probe. // -// Probing uses BlockExistsErr so a transient probe failure (canceled ctx, cluster gRPC blip) is -// NOT counted as a missing block: it returns that error immediately instead, so a network hiccup -// can't inflate the missing count into a false amplify-bail (which the full-object caller would -// then act on by DELETING a still-valid entry). +// A presence error (including cancellation, storage, or routing failure) is never counted as a +// missing block. This keeps a transient cache failure from inflating the missing count into a false +// amplify-bail that the full-object caller could then act on by DELETING a still-valid entry. func (s *Service) ensureBlocksCached(ctx context.Context, bucket, key, accessKey, secretKey string, meta *cache.CachedObjectMeta, b0, bK int64, bailIfMostlyMissing bool, maxFetchFanout int64) error { + total := bK - b0 + 1 var missing []int64 - for i := b0; i <= bK; i++ { - present, perr := s.cache.BlockExistsErr(ctx, bucket, key, meta.ETag, meta.BlockSize, i) - if perr != nil { - return perr // transient probe failure — abort without bailing or invalidating + if maxFetchFanout > 0 { + // Range preflight uses bounded, owner-grouped pages. Stop once this range has + // enough confirmed misses to take its existing single-upstream fall-through. + // The full-object caller (maxFetchFanout == 0) deliberately keeps its original + // complete scan and error precedence before the mostly-missing decision. + for pageStart := b0; pageStart <= bK; { + pageCount := min(int64(maxRangeBlockFanout), bK-pageStart+1) + indices := make([]int64, int(pageCount)) + for i := range indices { + indices[i] = pageStart + int64(i) + } + + present, perr := s.cache.BlockExistsBatchErr(ctx, bucket, key, meta.ETag, meta.BlockSize, indices) + if perr != nil { + return perr // transient probe failure — abort without bailing or invalidating + } + for i, found := range present { + if !found { + missing = append(missing, indices[i]) + } + } + if (bailIfMostlyMissing && int64(len(missing))*2 > total) || int64(len(missing)) > maxFetchFanout { + return errBlockAssemblyWouldAmplify + } + pageStart += pageCount } - if !present { - missing = append(missing, i) + } else { + for i := b0; i <= bK; i++ { + present, perr := s.cache.BlockExistsErr(ctx, bucket, key, meta.ETag, meta.BlockSize, i) + if perr != nil { + return perr // transient probe failure — abort without bailing or invalidating + } + if !present { + missing = append(missing, i) + } } } - total := bK - b0 + 1 - // Decided from the single probe above — no second scan. Hit/miss and serve metrics are all - // recorded ONLY on a committed block-cache serve below, so a bail or a failed fetch (both fall - // through to upstream, not a block serve) records none — matching CacheBlockRangeServed and - // avoiding a hit-ratio skew (failed fetches correlate with more-missing requests). + + // A bail or failed fetch falls through to upstream, not a block serve, so hit/miss + // and serve metrics remain aligned with committed block-cache responses. if (bailIfMostlyMissing && int64(len(missing))*2 > total) || (maxFetchFanout > 0 && int64(len(missing)) > maxFetchFanout) { return errBlockAssemblyWouldAmplify diff --git a/proxy/block_cache_integration_test.go b/proxy/block_cache_integration_test.go index 0928c678..6e813478 100644 --- a/proxy/block_cache_integration_test.go +++ b/proxy/block_cache_integration_test.go @@ -7,7 +7,9 @@ import ( "io" "net/http" "net/http/httptest" + "reflect" "strings" + "sync" "sync/atomic" "testing" "time" @@ -17,6 +19,8 @@ import ( "github.com/tigrisdata/tag/cache" "github.com/tigrisdata/tag/config" "github.com/tigrisdata/tag/metrics" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) // blockMockForwarder serves range GETs from a backing object, for both the initial @@ -36,6 +40,8 @@ type blockMockForwarder struct { blockGetShortBody bool // if set, per-block 206 body is shorter than its Content-Length blockGets atomic.Int32 // count of per-block DoConditionalGetRequest calls forwards atomic.Int32 // count of client-range forwards (DoRequestWithCreds) + blockGetStarted chan struct{} + blockGetRelease <-chan struct{} } func newBlockMock(object []byte, etag string) *blockMockForwarder { @@ -91,6 +97,15 @@ func (m *blockMockForwarder) serveRange(rangeHeader, etag string) *http.Response func (m *blockMockForwarder) DoConditionalGetRequest(_ context.Context, _, _, _, _, _ string, _ int64, rangeHeader string) (*http.Response, error) { m.blockGets.Add(1) + if m.blockGetRelease != nil { + if m.blockGetStarted != nil { + select { + case m.blockGetStarted <- struct{}{}: + default: + } + } + <-m.blockGetRelease + } if m.blockGetTransient { return nil, fmt.Errorf("simulated upstream blip") } @@ -144,6 +159,64 @@ func newBlockService(t *testing.T, mock *blockMockForwarder) (*Service, *cache.C return svc, c } +// blockPresenceRecordingClient is a correctness-test double for the optional batch capability. +// Its internal exact [0,1] reads preserve BlockExistsErr's miss/error semantics; probeCalls counts +// only calls made through the CacheClient interface under test, not this fake's implementation. +type blockPresenceRecordingClient struct { + cacheclient.CacheClient + pageMu sync.Mutex + pageSizes []int + batchErr error + probeCalls atomic.Int32 +} + +func (c *blockPresenceRecordingClient) BlockPresence(ctx context.Context, keys []string) ([]bool, error) { + c.pageMu.Lock() + c.pageSizes = append(c.pageSizes, len(keys)) + c.pageMu.Unlock() + if c.batchErr != nil { + return nil, c.batchErr + } + found := make([]bool, len(keys)) + for i, key := range keys { + writer := &blockPresenceRecordingWriter{} + err := c.CacheClient.GetRangeStream(ctx, key, 0, 1, writer) + if status.Code(err) == codes.NotFound { + continue + } + if err != nil { + return nil, err + } + found[i] = writer.written > 0 + } + return found, nil +} + +type blockPresenceRecordingWriter struct { + written int64 +} + +func (w *blockPresenceRecordingWriter) Write(p []byte) (int, error) { + w.written += int64(len(p)) + return len(p), nil +} + +func (c *blockPresenceRecordingClient) GetRangeStream(ctx context.Context, key string, start, end int64, w io.Writer) error { + if strings.HasPrefix(key, "blk|") && start == 0 && end == 1 { + c.probeCalls.Add(1) + } + return c.CacheClient.GetRangeStream(ctx, key, start, end, w) +} + +func newBlockServiceWithClient(mock *blockMockForwarder, client cacheclient.CacheClient) (*Service, *cache.Cache) { + cfg := config.NewDefault() + cfg.Cache.SetBlockCachingEnabled(true) + cfg.Cache.BlockSize = 4 + cfg.Cache.SizeThreshold = 1 << 20 + c := cache.NewCacheWithClient(client, &cfg.Cache) + return NewService(mock, c, cfg), c +} + // newBlockServiceWithBudget builds a block-mode service with a specific populate byte budget, // set at construction so the byteBudget reflects it (the budget is built in NewService). func newBlockServiceWithBudget(t *testing.T, mock *blockMockForwarder, blockSize, budget int64) (*Service, *cache.Cache) { @@ -1097,12 +1170,13 @@ func TestBlockCache_Block403DoesNotInvalidateSharedEntry(t *testing.T) { // into one aligned GET per block (a request storm). serveRangeFromBlockCache bails to a single // upstream range GET, without touching the still-valid entry. func TestBlockCache_LargeRangeServeBailsInsteadOfFanningOut(t *testing.T) { - obj := make([]byte, 200) // block_size 4 -> 50 covering blocks, > maxRangeBlockFanout (32) + obj := make([]byte, 400) // block_size 4 -> 100 covering blocks, > maxRangeBlockFanout (32) for i := range obj { obj[i] = byte('A' + i%26) } mock := newBlockMock(obj, `"v1"`) - svc, c := newBlockService(t, mock) + presenceClient := &blockPresenceRecordingClient{CacheClient: cacheclient.NewMemoryCache()} + svc, c := newBlockServiceWithClient(mock, presenceClient) // Establish the block-mode entry with a small read (block 0 + meta). w := httptest.NewRecorder() @@ -1113,18 +1187,26 @@ func TestBlockCache_LargeRangeServeBailsInsteadOfFanningOut(t *testing.T) { t.Fatal("entry not established") } before := mock.blockGets.Load() + probesBeforeRange := presenceClient.probeCalls.Load() - // A range spanning all 50 blocks (~49 missing > cap) must bail to one upstream range GET. + // A range spanning 100 blocks (99 missing) must bail after two bounded pages + // and use one upstream range GET rather than probing the rest of the object. w2 := httptest.NewRecorder() - if err := svc.HandleGetObject(w2, blockGet(wowBucket, wowKey, "bytes=0-199")); err != nil { + if err := svc.HandleGetObject(w2, blockGet(wowBucket, wowKey, "bytes=0-399")); err != nil { t.Fatalf("large range: %v", err) } - if w2.Code != http.StatusPartialContent || w2.Body.Len() != 200 { - t.Fatalf("large range: code=%d len=%d, want 206 with 200 bytes", w2.Code, w2.Body.Len()) + if w2.Code != http.StatusPartialContent || w2.Body.Len() != 400 { + t.Fatalf("large range: code=%d len=%d, want 206 with 400 bytes", w2.Code, w2.Body.Len()) } if after := mock.blockGets.Load(); after != before { t.Errorf("large range fanned out into per-block fetches: blockGets %d -> %d", before, after) } + if want := []int{32, 32}; !reflect.DeepEqual(presenceClient.pageSizes, want) { + t.Errorf("presence page sizes = %v, want %v", presenceClient.pageSizes, want) + } + if got := presenceClient.probeCalls.Load() - probesBeforeRange; got != 0 { + t.Errorf("batched cap check opened %d block reads after preflight, want 0", got) + } if _, found, _ := c.GetMeta(context.Background(), wowBucket, wowKey); !found { t.Error("range amplify-bail wrongly invalidated the entry") } @@ -1722,7 +1804,8 @@ func TestBlockCache_PipelineBudgetDeclineDegradesToSequential(t *testing.T) { // is byte-exact across all block boundaries. func TestBlockCache_WarmMultiBlockRangeServesPipelined(t *testing.T) { mock := newBlockMock([]byte("ABCDEFGHIJ"), `"v1"`) // blocks [0..3][4..7][8..9] - svc, c := newBlockService(t, mock) + presenceClient := &blockPresenceRecordingClient{CacheClient: cacheclient.NewMemoryCache()} + svc, c := newBlockServiceWithClient(mock, presenceClient) // Cold miss on bytes=0-7 populates blocks 0 and 1 (and the meta) in the background. w := httptest.NewRecorder() @@ -1733,8 +1816,9 @@ func TestBlockCache_WarmMultiBlockRangeServesPipelined(t *testing.T) { t.Fatal("block-mode meta not populated") } - // bytes=0-9 (10 bytes > BlockSize 4) skips the assembled-range path: probe finds block 2 - // missing, fetches it, then the three blocks stream pipelined. + // bytes=0-9 (10 bytes > BlockSize 4) skips the assembled-range path: batched presence + // finds block 2 missing, fetches it, then the three blocks stream pipelined. + probesBeforeRange := presenceClient.probeCalls.Load() w2 := httptest.NewRecorder() if err := svc.HandleGetObject(w2, blockGet(wowBucket, wowKey, "bytes=0-9")); err != nil { t.Fatalf("warm multi-block range: %v", err) @@ -1745,11 +1829,236 @@ func TestBlockCache_WarmMultiBlockRangeServesPipelined(t *testing.T) { if got := w2.Header().Get("X-Cache"); got != XCacheHit { t.Errorf("X-Cache=%q, want %q", got, XCacheHit) } + if got := w2.Header().Get("Content-Range"); got != "bytes 0-9/10" { + t.Errorf("Content-Range=%q, want %q", got, "bytes 0-9/10") + } + if got := w2.Header().Get("Content-Length"); got != "10" { + t.Errorf("Content-Length=%q, want 10", got) + } + if got := w2.Header().Get("ETag"); got != `"v1"` { + t.Errorf("ETag=%q, want %q", got, `"v1"`) + } + if want := []int{3}; !reflect.DeepEqual(presenceClient.pageSizes, want) { + t.Errorf("presence page sizes = %v, want %v", presenceClient.pageSizes, want) + } + if got := presenceClient.probeCalls.Load() - probesBeforeRange; got != 2 { + t.Errorf("partial warm range made %d byte-zero reads, want 2 (one missing-block recheck and the short final payload block)", got) + } if !c.BlockExists(context.Background(), wowBucket, wowKey, `"v1"`, 4, 2) { t.Error("block 2 not cached after the probe-path fetch") } } +func TestBlockCache_BatchedPresenceFailureFallsThroughWithoutInvalidating(t *testing.T) { + mock := newBlockMock([]byte("ABCDEFGHIJ"), `"v1"`) + presenceClient := &blockPresenceRecordingClient{CacheClient: cacheclient.NewMemoryCache()} + svc, c := newBlockServiceWithClient(mock, presenceClient) + + cold := httptest.NewRecorder() + if err := svc.HandleGetObject(cold, blockGet(wowBucket, wowKey, "bytes=0-7")); err != nil { + t.Fatalf("cold range: %v", err) + } + if !metaCached(c, wowBucket, wowKey, 2*time.Second) { + t.Fatal("cold range did not populate block-mode metadata") + } + beforeBlockGets := mock.blockGets.Load() + presenceClient.batchErr = errors.New("simulated owner read failure") + + warm := httptest.NewRecorder() + if err := svc.HandleGetObject(warm, blockGet(wowBucket, wowKey, "bytes=0-9")); err != nil { + t.Fatalf("range after batch failure: %v", err) + } + if warm.Code != http.StatusPartialContent || warm.Body.String() != "ABCDEFGHIJ" { + t.Fatalf("fall-through response = (status=%d, body=%q), want exact upstream 206", warm.Code, warm.Body.String()) + } + if got := warm.Header().Get("X-Cache"); got != XCacheMiss { + t.Errorf("X-Cache=%q, want upstream miss %q", got, XCacheMiss) + } + if after := mock.blockGets.Load(); after != beforeBlockGets { + t.Errorf("batch probe failure fetched %d blocks, want no per-block fetch", after-beforeBlockGets) + } + if _, found, _ := c.GetMeta(context.Background(), wowBucket, wowKey); !found { + t.Error("transient batch probe failure invalidated the still-valid entry") + } +} + +// Warm probe-first ranges keep exact 206 bytes and headers while batching presence checks for +// every required multi-block width. +func TestBlockCache_BatchedRangePresenceBlockCounts(t *testing.T) { + for _, blockCount := range []int{2, 4, 8, 32} { + t.Run(fmt.Sprintf("%d_blocks", blockCount), func(t *testing.T) { + object := make([]byte, blockCount*4) + for i := range object { + object[i] = byte('A' + i%26) + } + mock := newBlockMock(object, `"v1"`) + presenceClient := &blockPresenceRecordingClient{CacheClient: cacheclient.NewMemoryCache()} + svc, c := newBlockServiceWithClient(mock, presenceClient) + rangeHeader := fmt.Sprintf("bytes=0-%d", len(object)-1) + + cold := httptest.NewRecorder() + if err := svc.HandleGetObject(cold, blockGet(wowBucket, wowKey, rangeHeader)); err != nil { + t.Fatalf("cold range: %v", err) + } + if cold.Code != http.StatusPartialContent || cold.Body.Len() != len(object) { + t.Fatalf("cold range = (status=%d, bytes=%d), want (206, %d)", cold.Code, cold.Body.Len(), len(object)) + } + if !metaCached(c, wowBucket, wowKey, 2*time.Second) { + t.Fatal("cold range did not populate block-mode metadata") + } + + presenceClient.pageSizes = nil + probesBeforeRange := presenceClient.probeCalls.Load() + warm := httptest.NewRecorder() + if err := svc.HandleGetObject(warm, blockGet(wowBucket, wowKey, rangeHeader)); err != nil { + t.Fatalf("warm range: %v", err) + } + if warm.Code != http.StatusPartialContent || warm.Body.String() != string(object) { + t.Fatalf("warm range = (status=%d, body=%q), want exact 206 body", warm.Code, warm.Body.String()) + } + if got := warm.Header().Get("X-Cache"); got != XCacheHit { + t.Errorf("X-Cache=%q, want %q", got, XCacheHit) + } + if got, want := warm.Header().Get("Content-Range"), fmt.Sprintf("bytes 0-%d/%d", len(object)-1, len(object)); got != want { + t.Errorf("Content-Range=%q, want %q", got, want) + } + if got, want := warm.Header().Get("Content-Length"), fmt.Sprint(len(object)); got != want { + t.Errorf("Content-Length=%q, want %q", got, want) + } + if got := warm.Header().Get("ETag"); got != `"v1"` { + t.Errorf("ETag=%q, want %q", got, `"v1"`) + } + if want := []int{blockCount}; !reflect.DeepEqual(presenceClient.pageSizes, want) { + t.Errorf("presence page sizes = %v, want %v", presenceClient.pageSizes, want) + } + if got := presenceClient.probeCalls.Load() - probesBeforeRange; got != 0 { + t.Errorf("batched range opened %d byte-zero block reads, want 0", got) + } + }) + } +} + +func TestBlockCache_ProbeFirstRangesCoalesceMissingBlockFetch(t *testing.T) { + mock := newBlockMock([]byte("ABCDEFGH"), `"v1"`) + presenceClient := &blockPresenceRecordingClient{CacheClient: cacheclient.NewMemoryCache()} + svc, c := newBlockServiceWithClient(mock, presenceClient) + + cold := httptest.NewRecorder() + if err := svc.HandleGetObject(cold, blockGet(wowBucket, wowKey, "bytes=0-3")); err != nil { + t.Fatalf("cold range: %v", err) + } + if !metaCached(c, wowBucket, wowKey, 2*time.Second) || + !c.BlockExists(context.Background(), wowBucket, wowKey, `"v1"`, 4, 0) || + c.BlockExists(context.Background(), wowBucket, wowKey, `"v1"`, 4, 1) { + t.Fatal("expected block 0 and metadata cached, with block 1 absent") + } + + mock.blockGetStarted = make(chan struct{}, 2) + release := make(chan struct{}) + mock.blockGetRelease = release + var releaseOnce sync.Once + releaseFetch := func() { releaseOnce.Do(func() { close(release) }) } + before := mock.blockGets.Load() + + type rangeResult struct { + response *httptest.ResponseRecorder + err error + } + results := make(chan rangeResult, 2) + startedRanges, completedRanges := 0, 0 + t.Cleanup(func() { + releaseFetch() + for completedRanges < startedRanges { + select { + case <-results: + completedRanges++ + case <-time.After(5 * time.Second): + return + } + } + }) + startRange := func() { + startedRanges++ + go func() { + response := httptest.NewRecorder() + err := svc.HandleGetObject(response, blockGet(wowBucket, wowKey, "bytes=0-7")) + results <- rangeResult{response: response, err: err} + }() + } + startRange() + select { + case <-mock.blockGetStarted: + case <-time.After(5 * time.Second): + t.Fatal("first range did not start the missing-block fetch") + } + startRange() + + blockKey := cache.MakeBlockKey(wowBucket, wowKey, `"v1"`, 4, 1) + joined := false + deadline := time.NewTimer(5 * time.Second) + defer deadline.Stop() + ticker := time.NewTicker(time.Millisecond) + defer ticker.Stop() + for !joined { + svc.blockFetchMu.Lock() + state := svc.blockFetches[blockKey] + svc.blockFetchMu.Unlock() + if state != nil { + state.mu.Lock() + joined = state.consumers >= 2 + state.mu.Unlock() + } + if joined { + break + } + select { + case <-deadline.C: + t.Fatal("second range did not join the in-flight block fetch") + case <-ticker.C: + } + } + if got := mock.blockGets.Load(); got != before+1 { + t.Fatalf("overlapping ranges started %d upstream block fetches, want 1", got-before) + } + releaseFetch() + + for i := 0; i < 2; i++ { + select { + case result := <-results: + completedRanges++ + if result.err != nil { + t.Fatalf("warm range %d: %v", i, result.err) + } + if result.response.Code != http.StatusPartialContent || result.response.Body.String() != "ABCDEFGH" { + t.Fatalf("warm range %d = (%d, %q), want exact 206 ABCDEFGH", i, result.response.Code, result.response.Body.String()) + } + if got := result.response.Header().Get("X-Cache"); got != XCacheHit { + t.Errorf("warm range %d X-Cache=%q, want %q", i, got, XCacheHit) + } + if got := result.response.Header().Get("Content-Range"); got != "bytes 0-7/8" { + t.Errorf("warm range %d Content-Range=%q, want %q", i, got, "bytes 0-7/8") + } + if got := result.response.Header().Get("Content-Length"); got != "8" { + t.Errorf("warm range %d Content-Length=%q, want 8", i, got) + } + if got := result.response.Header().Get("ETag"); got != `"v1"` { + t.Errorf("warm range %d ETag=%q, want %q", i, got, `"v1"`) + } + case <-time.After(5 * time.Second): + t.Fatal("overlapping range did not finish after the shared fetch") + } + } + if got := mock.blockGets.Load(); got != before+1 { + t.Errorf("overlapping ranges fetched the missing block %d times, want 1", got-before) + } + presenceClient.pageMu.Lock() + pageSizes := append([]int(nil), presenceClient.pageSizes...) + presenceClient.pageMu.Unlock() + if !reflect.DeepEqual(pageSizes, []int{2, 2}) { + t.Errorf("overlapping range presence pages = %v, want two two-block pages", pageSizes) + } +} + // failAfterWriter fails every Write once `remaining` bytes have been accepted, simulating a // client that disconnects mid-body. type failAfterWriter struct { From 686f9218d6fc0059f687e2b5b442a3a9b1012d98 Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Sun, 27 Sep 2026 11:15:40 +0000 Subject: [PATCH 4/9] fix(cache): evict retired presence peers --- cmd/tag/block_cache_client.go | 143 ++++++++++++--- cmd/tag/block_cache_range_benchmark_test.go | 19 +- cmd/tag/block_presence_test.go | 183 ++++++++++++++++++++ 3 files changed, 320 insertions(+), 25 deletions(-) diff --git a/cmd/tag/block_cache_client.go b/cmd/tag/block_cache_client.go index 4b191bac..caf3e638 100644 --- a/cmd/tag/block_cache_client.go +++ b/cmd/tag/block_cache_client.go @@ -23,17 +23,23 @@ import ( type embeddedBlockCacheClient struct { *embedded.Client - dialOptions []grpc.DialOption - peerMu sync.Mutex - peers map[string]blockPresencePeer - closed bool - closeOnce sync.Once - closeErr error + dialOptions []grpc.DialOption + peerMu sync.Mutex + peers map[string]*blockPresencePeer + retiredPeers map[*blockPresencePeer]struct{} // detached entries kept only while leased + closed bool + closeOnce sync.Once + closeErr error } +// Peer lifecycle fields are protected by embeddedBlockCacheClient.peerMu. active +// counts RPC leases; retired peers are closed after their last lease is released. type blockPresencePeer struct { address string conn *grpc.ClientConn + active int // outstanding RPC leases, protected by peerMu + retired bool + closed bool } type blockPresenceOwnerGroup struct { @@ -47,9 +53,10 @@ type blockPresenceOwnerGroup struct { func newEmbeddedBlockCacheClient(client *embedded.Client, dialOptions ...grpc.DialOption) *embeddedBlockCacheClient { options := append(cacheclient.DefaultDialOptions(), dialOptions...) return &embeddedBlockCacheClient{ - Client: client, - dialOptions: options, - peers: make(map[string]blockPresencePeer), + Client: client, + dialOptions: options, + peers: make(map[string]*blockPresencePeer), + retiredPeers: make(map[*blockPresencePeer]struct{}), } } @@ -78,6 +85,17 @@ func (c *embeddedBlockCacheClient) BlockPresence(ctx context.Context, keys []str if coord != nil { epoch = coord.GetEpoch() localID = coord.GetLocalNodeID() + if ringManager := coord.GetRing(); ringManager != nil { + activeOwners := make(map[string]string) + for _, node := range ringManager.GetActiveNodes() { + if node != nil && node.ID != "" { + activeOwners[node.ID] = node.ListenAddress + } + } + if len(activeOwners) > 0 && coord.GetEpoch() == epoch { + c.prunePeerConnections(activeOwners) + } + } } groups := make(map[string]*blockPresenceOwnerGroup) owners := make([]string, len(keys)) @@ -202,11 +220,12 @@ func (c *embeddedBlockCacheClient) remoteBlockPresence(ctx context.Context, owne } return nil, err } - conn, err := c.peerConnection(owner, address) + peer, err := c.peerConnection(owner, address) if err != nil { return nil, err } - stream, err := conn.NewStream(ctx, &grpc.StreamDesc{ + defer c.releasePeerConnection(peer) + stream, err := peer.conn.NewStream(ctx, &grpc.StreamDesc{ StreamName: "Check", ClientStreams: true, ServerStreams: true, @@ -265,25 +284,92 @@ func blockPresenceRPCError(ctx context.Context, err error) error { } } -func (c *embeddedBlockCacheClient) peerConnection(owner, address string) (*grpc.ClientConn, error) { +func (c *embeddedBlockCacheClient) peerConnection(owner, address string) (*blockPresencePeer, error) { c.peerMu.Lock() - defer c.peerMu.Unlock() if c.closed { + c.peerMu.Unlock() return nil, errors.New("embedded cache client is closed") } + var closeConnections []*grpc.ClientConn if peer, ok := c.peers[owner]; ok { - if peer.address == address { - return peer.conn, nil + if peer.address == address && !peer.retired { + peer.active++ + c.peerMu.Unlock() + return peer, nil } - _ = peer.conn.Close() delete(c.peers, owner) + c.retirePeerLocked(peer, &closeConnections) } conn, err := grpc.NewClient(address, c.dialOptions...) if err != nil { + c.peerMu.Unlock() + closeBlockPresenceConnections(closeConnections) return nil, err } - c.peers[owner] = blockPresencePeer{address: address, conn: conn} - return conn, nil + peer := &blockPresencePeer{address: address, conn: conn, active: 1} + c.peers[owner] = peer + c.peerMu.Unlock() + closeBlockPresenceConnections(closeConnections) + return peer, nil +} + +// prunePeerConnections evicts cached connections for owners no longer active in +// the full ring snapshot. Page membership is deliberately not used: an active +// peer absent from one Range page still belongs in the cache. +func (c *embeddedBlockCacheClient) prunePeerConnections(activeOwners map[string]string) { + if len(activeOwners) == 0 { + return + } + var closeConnections []*grpc.ClientConn + c.peerMu.Lock() + if c.closed { + c.peerMu.Unlock() + return + } + for owner, peer := range c.peers { + address, active := activeOwners[owner] + if active && address == peer.address { + continue + } + delete(c.peers, owner) + c.retirePeerLocked(peer, &closeConnections) + } + c.peerMu.Unlock() + closeBlockPresenceConnections(closeConnections) +} + +// retirePeerLocked defers closing a detached connection until no RPC lease uses it. +func (c *embeddedBlockCacheClient) retirePeerLocked(peer *blockPresencePeer, closeConnections *[]*grpc.ClientConn) { + peer.retired = true + if peer.active == 0 { + if !peer.closed { + peer.closed = true + *closeConnections = append(*closeConnections, peer.conn) + } + return + } + c.retiredPeers[peer] = struct{}{} +} + +func (c *embeddedBlockCacheClient) releasePeerConnection(peer *blockPresencePeer) { + var closeConnection *grpc.ClientConn + c.peerMu.Lock() + peer.active-- + if peer.active == 0 && peer.retired && !peer.closed { + peer.closed = true + delete(c.retiredPeers, peer) + closeConnection = peer.conn + } + c.peerMu.Unlock() + if closeConnection != nil { + _ = closeConnection.Close() + } +} + +func closeBlockPresenceConnections(connections []*grpc.ClientConn) { + for _, connection := range connections { + _ = connection.Close() + } } // Close releases the optional peer connections before stopping the embedded @@ -292,13 +378,28 @@ func (c *embeddedBlockCacheClient) Close() error { c.closeOnce.Do(func() { c.peerMu.Lock() c.closed = true - peers := c.peers + peers := make(map[*blockPresencePeer]struct{}, len(c.peers)+len(c.retiredPeers)) + for _, peer := range c.peers { + peers[peer] = struct{}{} + } + for peer := range c.retiredPeers { + peers[peer] = struct{}{} + } c.peers = nil + c.retiredPeers = nil + var connections []*grpc.ClientConn + for peer := range peers { + peer.retired = true + if !peer.closed { + peer.closed = true + connections = append(connections, peer.conn) + } + } c.peerMu.Unlock() var closeErrors []error - for _, peer := range peers { - if err := peer.conn.Close(); err != nil { + for _, connection := range connections { + if err := connection.Close(); err != nil { closeErrors = append(closeErrors, err) } } diff --git a/cmd/tag/block_cache_range_benchmark_test.go b/cmd/tag/block_cache_range_benchmark_test.go index b9b83b65..ec73b286 100644 --- a/cmd/tag/block_cache_range_benchmark_test.go +++ b/cmd/tag/block_cache_range_benchmark_test.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "strings" + "sync" "sync/atomic" "testing" "time" @@ -43,11 +44,14 @@ type embeddedBlockRangeBenchmarkFixture struct { cacheConfig config.CacheConfig cacheClient *embeddedBlockCacheClient embeddedClient *embedded.Client + closeNodes []func() + seedGossipAddr string stats *embeddedBlockRangeRPCStats requestSigner *auth.RequestSigner gatewaySigner *auth.RequestSigner accessKey string secretKey string + grpcToken string region string } @@ -186,6 +190,7 @@ func newEmbeddedBlockRangeBenchmarkFixtureWithOptions(tb testing.TB, blockCount var blockCacheClient *embeddedBlockCacheClient var nodes []*embedded.Client + closeNodes := make([]func(), len(gossipAddrs)) var stats *embeddedBlockRangeRPCStats if len(withRPCStats) > 0 && withRPCStats[0] { stats = &embeddedBlockRangeRPCStats{} @@ -240,10 +245,15 @@ func newEmbeddedBlockRangeBenchmarkFixtureWithOptions(tb testing.TB, blockCount nodes = append(nodes, client) if i == 0 { blockCacheClient = newEmbeddedBlockCacheClient(client, embeddedCfg.GRPCDialOptions...) - tb.Cleanup(func() { _ = blockCacheClient.Close() }) + wrapper := blockCacheClient + var closeOnce sync.Once + closeNodes[i] = func() { closeOnce.Do(func() { _ = wrapper.Close() }) } } else { - tb.Cleanup(func() { _ = client.Close() }) + node := client + var closeOnce sync.Once + closeNodes[i] = func() { closeOnce.Do(func() { _ = node.Close() }) } } + tb.Cleanup(closeNodes[i]) if err := client.StartGRPCServer(); err != nil { tb.Fatalf("start embedded range node %d: %v", i, err) } @@ -347,8 +357,9 @@ func newEmbeddedBlockRangeBenchmarkFixtureWithOptions(tb testing.TB, blockCount return &embeddedBlockRangeBenchmarkFixture{ handler: handler, body: body, key: key, cacheConfig: cfg.Cache, cacheClient: blockCacheClient, - embeddedClient: nodes[0], stats: stats, requestSigner: requestSigner, - accessKey: accessKey, secretKey: secretKey, region: cfg.Upstream.Region, + embeddedClient: nodes[0], closeNodes: closeNodes, seedGossipAddr: gossipAddrs[0], + stats: stats, requestSigner: requestSigner, + accessKey: accessKey, secretKey: secretKey, grpcToken: grpcToken, region: cfg.Upstream.Region, } } diff --git a/cmd/tag/block_presence_test.go b/cmd/tag/block_presence_test.go index 33594703..3f08a1ed 100644 --- a/cmd/tag/block_presence_test.go +++ b/cmd/tag/block_presence_test.go @@ -11,6 +11,7 @@ import ( "net/http/httptest" "strconv" "strings" + "sync" "testing" "time" @@ -24,10 +25,63 @@ import ( "github.com/tigrisdata/tag/proxy" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/connectivity" "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/structpb" ) +func TestBlockPresencePeerPruningRetainsActiveOwnerAndLeasedConnection(t *testing.T) { + client := newEmbeddedBlockCacheClient(nil) + t.Cleanup(func() { _ = client.Close() }) + + retiredLease, err := client.peerConnection("retired", "127.0.0.1:1") + if err != nil { + t.Fatalf("open retired peer connection: %v", err) + } + activeLease, err := client.peerConnection("active", "127.0.0.1:2") + if err != nil { + t.Fatalf("open active peer connection: %v", err) + } + activeConn := activeLease.conn + client.releasePeerConnection(activeLease) + + // The current ring still contains the active peer even though this page has + // no key for it. The retired owner has a live lease while the ring changes. + client.prunePeerConnections(map[string]string{ + "active": "127.0.0.1:2", + "new": "127.0.0.1:3", + }) + client.peerMu.Lock() + activePeer := client.peers["active"] + _, retiredPresent := client.peers["retired"] + _, leaseRetained := client.retiredPeers[retiredLease] + client.peerMu.Unlock() + if activePeer != activeLease || retiredPresent || !leaseRetained { + t.Fatalf("pruned peers: active retained=%t retired present=%t lease retained=%t", activePeer == activeLease, retiredPresent, leaseRetained) + } + if activeConn.GetState() == connectivity.Shutdown || retiredLease.conn.GetState() == connectivity.Shutdown { + t.Fatal("pruning closed an active owner or a leased connection") + } + + newLease, err := client.peerConnection("new", "127.0.0.1:3") + if err != nil { + t.Fatalf("open replacement peer connection: %v", err) + } + client.releasePeerConnection(newLease) + client.releasePeerConnection(retiredLease) + if got := retiredLease.conn.GetState(); got != connectivity.Shutdown { + t.Fatalf("released retired connection state = %v, want shutdown", got) + } + client.peerMu.Lock() + _, activeRetained := client.peers["active"] + _, newRetained := client.peers["new"] + _, retiredLeaseRetained := client.retiredPeers[retiredLease] + client.peerMu.Unlock() + if !activeRetained || !newRetained || retiredLeaseRetained { + t.Fatalf("final peer cache: active=%t new=%t retired lease=%t", activeRetained, newRetained, retiredLeaseRetained) + } +} + func TestBlockPresenceRequestFallsBackForNonUTF8Keys(t *testing.T) { key := string([]byte{'b', 'l', 'k', '|', 0xff}) if _, err := encodeBlockPresenceRequest("owner", 1, []string{key}); !errors.Is(err, cache.ErrBlockPresenceUnsupported) { @@ -455,6 +509,135 @@ func TestEmbeddedBlockPresenceMultiNodeAndFallback(t *testing.T) { } } +func TestEmbeddedBlockPresencePrunesReplacedOwnerID(t *testing.T) { + fixture := newEmbeddedBlockRangeBenchmarkFixture(t, 2) + ctx := context.Background() + const bucket, etag = "benchmark", `"block-range"` + oldKey := cache.MakeBlockKey(bucket, fixture.key, etag, fixture.cacheConfig.BlockSize, 1) + oldOwner, err := fixture.embeddedClient.Coordinator().GetNodeForKey(oldKey) + if err != nil || oldOwner.ID != "range-bench-1" { + t.Fatalf("initial block owner = (%v,%v), want range-bench-1", oldOwner, err) + } + found, err := fixture.cacheClient.BlockPresence(ctx, []string{oldKey}) + if err != nil || len(found) != 1 || !found[0] { + t.Fatalf("initial peer presence = (%v,%v), want present", found, err) + } + fixture.cacheClient.peerMu.Lock() + oldPeer := fixture.cacheClient.peers[oldOwner.ID] + fixture.cacheClient.peerMu.Unlock() + if oldPeer == nil { + t.Fatal("initial remote presence did not cache its owner connection") + } + + fixture.closeNodes[1]() + waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0") + startEmbeddedBlockRangeReplacementNode(t, fixture, "range-bench-replacement") + waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0", "range-bench-replacement") + + newOwner, err := fixture.embeddedClient.Coordinator().GetNodeForKey(oldKey) + if err != nil || newOwner.ID != "range-bench-replacement" { + t.Fatalf("replacement block owner = (%v,%v), want range-bench-replacement", newOwner, err) + } + start := int(fixture.cacheConfig.BlockSize) + if err := fixture.cacheClient.Put(ctx, oldKey, fixture.body[start:], 60); err != nil { + t.Fatalf("seed replacement block: %v", err) + } + present, err := fixture.cacheClient.BlockPresence(ctx, []string{oldKey}) + if err != nil || len(present) != 1 || !present[0] { + t.Fatalf("replacement peer presence = (%v,%v), want present", present, err) + } + + fixture.cacheClient.peerMu.Lock() + _, oldRetained := fixture.cacheClient.peers[oldOwner.ID] + replacementPeer := fixture.cacheClient.peers["range-bench-replacement"] + peerCount := len(fixture.cacheClient.peers) + fixture.cacheClient.peerMu.Unlock() + if oldRetained || replacementPeer == nil || peerCount != 1 { + t.Fatalf("peer connections after replacement = (old_retained=%t, replacement=%v, count=%d), want only the new owner", oldRetained, replacementPeer, peerCount) + } + if got := oldPeer.conn.GetState(); got != connectivity.Shutdown { + t.Fatalf("retired owner connection state = %v, want shutdown", got) + } + + response := fixture.doRange(t, nil) + fixture.verifyResponse(t, response) +} + +func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBlockRangeBenchmarkFixture, nodeID string) { + tb.Helper() + diskPath := tb.TempDir() + writeEmbeddedBlockRangeRingTokens(tb, diskPath, 1) + presenceServer := &blockPresenceServer{} + gossipAddr := embeddedBlockRangeFreeAddress(tb) + grpcAddr := embeddedBlockRangeFreeAddress(tb) + cfg := &embedded.Config{ + DiskPath: diskPath, + TTL: fixture.cacheConfig.TTL, + NodeID: nodeID, + ClusterAddr: gossipAddr, + GRPCAddr: grpcAddr, + AdvertiseAddr: grpcAddr, + SeedNodes: []string{fixture.seedGossipAddr}, + Registerer: prometheus.NewRegistry(), + } + if fixture.cacheConfig.IsGRPCAuthEnabled() { + cfg.GRPCServerOptions = auth.GRPCServerOptions(fixture.grpcToken) + cfg.GRPCDialOptions = auth.GRPCDialOptions(fixture.grpcToken) + } + cfg.GRPCServerOptions = append(cfg.GRPCServerOptions, presenceServer.serverOption()) + client, err := embedded.New(cfg) + if err != nil { + tb.Fatalf("create replacement embedded node: %v", err) + } + presenceServer.client = client + var closeOnce sync.Once + tb.Cleanup(func() { closeOnce.Do(func() { _ = client.Close() }) }) + if err := client.StartGRPCServer(); err != nil { + tb.Fatalf("start replacement embedded node: %v", err) + } + readyCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + if err := client.WaitReady(readyCtx); err != nil { + tb.Fatalf("replacement embedded node did not become ready: %v", err) + } +} + +func waitForEmbeddedActiveNodeIDs(tb testing.TB, client *embedded.Client, want ...string) { + tb.Helper() + wantSet := make(map[string]struct{}, len(want)) + for _, id := range want { + wantSet[id] = struct{}{} + } + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + ticker := time.NewTicker(20 * time.Millisecond) + defer ticker.Stop() + for { + nodes := client.Coordinator().GetRing().GetActiveNodes() + if len(nodes) == len(wantSet) { + allPresent := true + for _, node := range nodes { + if _, ok := wantSet[node.ID]; !ok { + allPresent = false + break + } + } + if allPresent { + return + } + } + select { + case <-ctx.Done(): + got := make([]string, 0, len(nodes)) + for _, node := range nodes { + got = append(got, node.ID) + } + tb.Fatalf("active nodes = %v, want %v: %v", got, want, ctx.Err()) + case <-ticker.C: + } + } +} + func TestEmbeddedBlockPresenceUsesClusterGRPCAuth(t *testing.T) { token := auth.DeriveGRPCAuthToken("access", "secret") gossip := []string{embeddedBlockRangeFreeAddress(t), embeddedBlockRangeFreeAddress(t)} From 62c1573e2e87824c354dcb96e19ecff154e567a0 Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Sun, 27 Sep 2026 11:45:19 +0000 Subject: [PATCH 5/9] test(cache): cover range peer replacement --- cmd/tag/block_presence_test.go | 70 +++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/cmd/tag/block_presence_test.go b/cmd/tag/block_presence_test.go index 3f08a1ed..0b621e49 100644 --- a/cmd/tag/block_presence_test.go +++ b/cmd/tag/block_presence_test.go @@ -507,63 +507,60 @@ func TestEmbeddedBlockPresenceMultiNodeAndFallback(t *testing.T) { if !errors.Is(err, context.Canceled) { t.Fatalf("canceled presence = %v, want context.Canceled", err) } -} -func TestEmbeddedBlockPresencePrunesReplacedOwnerID(t *testing.T) { - fixture := newEmbeddedBlockRangeBenchmarkFixture(t, 2) - ctx := context.Background() - const bucket, etag = "benchmark", `"block-range"` - oldKey := cache.MakeBlockKey(bucket, fixture.key, etag, fixture.cacheConfig.BlockSize, 1) - oldOwner, err := fixture.embeddedClient.Coordinator().GetNodeForKey(oldKey) + // A replacement with a fresh ID owns the same deterministic block keys, but + // the old connection must be removed from the batcher's owner cache. + oldOwnerKey := cache.MakeBlockKey(bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, 1) + oldOwner, err := fixture.embeddedClient.Coordinator().GetNodeForKey(oldOwnerKey) if err != nil || oldOwner.ID != "range-bench-1" { t.Fatalf("initial block owner = (%v,%v), want range-bench-1", oldOwner, err) } - found, err := fixture.cacheClient.BlockPresence(ctx, []string{oldKey}) - if err != nil || len(found) != 1 || !found[0] { - t.Fatalf("initial peer presence = (%v,%v), want present", found, err) - } fixture.cacheClient.peerMu.Lock() oldPeer := fixture.cacheClient.peers[oldOwner.ID] + stalePeer := fixture.cacheClient.peers["stale-owner"] fixture.cacheClient.peerMu.Unlock() if oldPeer == nil { - t.Fatal("initial remote presence did not cache its owner connection") + t.Fatal("multi-node probes did not cache the old owner's connection") } fixture.closeNodes[1]() waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0") - startEmbeddedBlockRangeReplacementNode(t, fixture, "range-bench-replacement") + replacement := startEmbeddedBlockRangeReplacementNode(t, fixture, "range-bench-replacement") waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0", "range-bench-replacement") + waitForMatchingEmbeddedEpoch(t, fixture.embeddedClient, replacement) - newOwner, err := fixture.embeddedClient.Coordinator().GetNodeForKey(oldKey) + newOwner, err := fixture.embeddedClient.Coordinator().GetNodeForKey(oldOwnerKey) if err != nil || newOwner.ID != "range-bench-replacement" { t.Fatalf("replacement block owner = (%v,%v), want range-bench-replacement", newOwner, err) } - start := int(fixture.cacheConfig.BlockSize) - if err := fixture.cacheClient.Put(ctx, oldKey, fixture.body[start:], 60); err != nil { + blockStart := embeddedBlockRangeBenchmarkBlockSize + if err := fixture.cacheClient.Put(ctx, oldOwnerKey, fixture.body[blockStart:2*blockStart], 60); err != nil { t.Fatalf("seed replacement block: %v", err) } - present, err := fixture.cacheClient.BlockPresence(ctx, []string{oldKey}) - if err != nil || len(present) != 1 || !present[0] { - t.Fatalf("replacement peer presence = (%v,%v), want present", present, err) + fixture.stats.reset() + assertEmbeddedRangeResponse(t, fixture.client, fixture.gateway.URL, fixture.key, fixture.body, 2, fixture.gatewaySigner, fixture.accessKey, fixture.secretKey) + if got := fixture.stats.presenceRequests.Load(); got != 1 { + t.Fatalf("replacement Range made %d presence attempts, want 1", got) } fixture.cacheClient.peerMu.Lock() _, oldRetained := fixture.cacheClient.peers[oldOwner.ID] + _, staleRetained := fixture.cacheClient.peers["stale-owner"] replacementPeer := fixture.cacheClient.peers["range-bench-replacement"] peerCount := len(fixture.cacheClient.peers) fixture.cacheClient.peerMu.Unlock() - if oldRetained || replacementPeer == nil || peerCount != 1 { - t.Fatalf("peer connections after replacement = (old_retained=%t, replacement=%v, count=%d), want only the new owner", oldRetained, replacementPeer, peerCount) + if oldRetained || staleRetained || replacementPeer == nil || peerCount != 1 { + t.Fatalf("peer connections after replacement = (old=%t stale=%t replacement=%v count=%d), want only new owner", oldRetained, staleRetained, replacementPeer, peerCount) } - if got := oldPeer.conn.GetState(); got != connectivity.Shutdown { - t.Fatalf("retired owner connection state = %v, want shutdown", got) + if oldPeer.conn.GetState() != connectivity.Shutdown { + t.Fatalf("retired owner connection state = %v, want shutdown", oldPeer.conn.GetState()) + } + if stalePeer != nil && stalePeer.conn.GetState() != connectivity.Shutdown { + t.Fatalf("stale owner connection state = %v, want shutdown", stalePeer.conn.GetState()) } - - response := fixture.doRange(t, nil) - fixture.verifyResponse(t, response) } -func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBlockRangeBenchmarkFixture, nodeID string) { +func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBlockRangeBenchmarkFixture, nodeID string) *embedded.Client { tb.Helper() diskPath := tb.TempDir() writeEmbeddedBlockRangeRingTokens(tb, diskPath, 1) @@ -600,6 +597,25 @@ func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBloc if err := client.WaitReady(readyCtx); err != nil { tb.Fatalf("replacement embedded node did not become ready: %v", err) } + return client +} + +func waitForMatchingEmbeddedEpoch(tb testing.TB, first, second *embedded.Client) { + tb.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + ticker := time.NewTicker(20 * time.Millisecond) + defer ticker.Stop() + for { + if first.Coordinator().GetEpoch() == second.Coordinator().GetEpoch() { + return + } + select { + case <-ctx.Done(): + tb.Fatalf("replacement ring epochs did not converge: first=%d second=%d: %v", first.Coordinator().GetEpoch(), second.Coordinator().GetEpoch(), ctx.Err()) + case <-ticker.C: + } + } } func waitForEmbeddedActiveNodeIDs(tb testing.TB, client *embedded.Client, want ...string) { From 7b2e8c99ed814813f279199d7ecc940c799684d6 Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Fri, 2 Oct 2026 01:55:17 +0000 Subject: [PATCH 6/9] fix: guard missing block presence owners Signed-off-by: Perfloop Agent --- cmd/tag/block_presence.go | 2 +- cmd/tag/block_presence_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cmd/tag/block_presence.go b/cmd/tag/block_presence.go index 553fa390..466c8d98 100644 --- a/cmd/tag/block_presence.go +++ b/cmd/tag/block_presence.go @@ -255,7 +255,7 @@ func checkBlockPresenceOwner(coord blockPresenceCoordinator, key, owner string) if err != nil { return fmt.Errorf("resolve block owner for %q: %w", key, err) } - if node.ID != owner { + if node == nil || node.ID != owner { return cache.ErrBlockPresenceTopologyChanged } return nil diff --git a/cmd/tag/block_presence_test.go b/cmd/tag/block_presence_test.go index 0b621e49..6cca6796 100644 --- a/cmd/tag/block_presence_test.go +++ b/cmd/tag/block_presence_test.go @@ -17,6 +17,7 @@ import ( "github.com/prometheus/client_golang/prometheus" cacheclient "github.com/tigrisdata/ocache/client" + "github.com/tigrisdata/ocache/coordinator/ring" "github.com/tigrisdata/ocache/embedded" "github.com/tigrisdata/tag/auth" "github.com/tigrisdata/tag/cache" @@ -30,6 +31,21 @@ import ( "google.golang.org/protobuf/types/known/structpb" ) +type missingBlockPresenceOwnerCoordinator struct{} + +func (missingBlockPresenceOwnerCoordinator) GetEpoch() uint64 { return 1 } +func (missingBlockPresenceOwnerCoordinator) GetLocalNodeID() string { return "local" } +func (missingBlockPresenceOwnerCoordinator) GetNodeForKey(string) (*ring.NodeInfo, error) { + return nil, nil +} + +func TestCheckBlockPresenceOwnerRetriesWhenNodeIsMissing(t *testing.T) { + err := checkBlockPresenceOwner(missingBlockPresenceOwnerCoordinator{}, "block-key", "owner") + if !errors.Is(err, cache.ErrBlockPresenceTopologyChanged) { + t.Fatalf("missing owner error = %v, want topology change", err) + } +} + func TestBlockPresencePeerPruningRetainsActiveOwnerAndLeasedConnection(t *testing.T) { client := newEmbeddedBlockCacheClient(nil) t.Cleanup(func() { _ = client.Close() }) From 432ea5bffc6cb3da29ea68ff96645b9326f6f387 Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Fri, 2 Oct 2026 01:55:17 +0000 Subject: [PATCH 7/9] fix(cache): gate peer cleanup by ring epoch Signed-off-by: Perfloop Agent --- cmd/tag/block_cache_client.go | 57 +++++++++---- cmd/tag/block_presence_test.go | 143 ++++++++++++++++++++++++++++++++- 2 files changed, 184 insertions(+), 16 deletions(-) diff --git a/cmd/tag/block_cache_client.go b/cmd/tag/block_cache_client.go index caf3e638..3d39208d 100644 --- a/cmd/tag/block_cache_client.go +++ b/cmd/tag/block_cache_client.go @@ -23,13 +23,15 @@ import ( type embeddedBlockCacheClient struct { *embedded.Client - dialOptions []grpc.DialOption - peerMu sync.Mutex - peers map[string]*blockPresencePeer - retiredPeers map[*blockPresencePeer]struct{} // detached entries kept only while leased - closed bool - closeOnce sync.Once - closeErr error + dialOptions []grpc.DialOption + peerMu sync.Mutex + peers map[string]*blockPresencePeer + retiredPeers map[*blockPresencePeer]struct{} // detached entries kept only while leased + closed bool + closeOnce sync.Once + closeErr error + reconciledEpoch uint64 + reconciledEpochSet bool } // Peer lifecycle fields are protected by embeddedBlockCacheClient.peerMu. active @@ -85,15 +87,15 @@ func (c *embeddedBlockCacheClient) BlockPresence(ctx context.Context, keys []str if coord != nil { epoch = coord.GetEpoch() localID = coord.GetLocalNodeID() - if ringManager := coord.GetRing(); ringManager != nil { + if ringManager := coord.GetRing(); ringManager != nil && c.peerCacheNeedsReconcile(epoch) { activeOwners := make(map[string]string) for _, node := range ringManager.GetActiveNodes() { if node != nil && node.ID != "" { activeOwners[node.ID] = node.ListenAddress } } - if len(activeOwners) > 0 && coord.GetEpoch() == epoch { - c.prunePeerConnections(activeOwners) + if len(activeOwners) > 0 { + c.prunePeerConnections(activeOwners, coord, epoch) } } } @@ -313,27 +315,52 @@ func (c *embeddedBlockCacheClient) peerConnection(owner, address string) (*block return peer, nil } +func (c *embeddedBlockCacheClient) peerCacheNeedsReconcile(epoch uint64) bool { + c.peerMu.Lock() + defer c.peerMu.Unlock() + return !c.closed && (!c.reconciledEpochSet || c.reconciledEpoch != epoch) +} + // prunePeerConnections evicts cached connections for owners no longer active in // the full ring snapshot. Page membership is deliberately not used: an active -// peer absent from one Range page still belongs in the cache. -func (c *embeddedBlockCacheClient) prunePeerConnections(activeOwners map[string]string) { +// peer absent from one Range page still belongs in the cache. The epoch is an +// opaque topology fingerprint, so reconciliation is equality-gated and the full +// owner/peer scan stays off the steady-state Range path. A stale snapshot is +// discarded if the coordinator has already observed another epoch; this sweep +// is only connection cleanup, while per-key routing remains authoritative. An +// address change for an owner used by this page is handled by peerConnection. +func (c *embeddedBlockCacheClient) prunePeerConnections(activeOwners map[string]string, coord blockPresenceCoordinator, epoch uint64) { if len(activeOwners) == 0 { return } var closeConnections []*grpc.ClientConn c.peerMu.Lock() - if c.closed { + if c.closed || (c.reconciledEpochSet && c.reconciledEpoch == epoch) || coord.GetEpoch() != epoch { c.peerMu.Unlock() return } + type cachedPeer struct { + owner string + peer *blockPresencePeer + } + var stalePeers []cachedPeer for owner, peer := range c.peers { address, active := activeOwners[owner] if active && address == peer.address { continue } - delete(c.peers, owner) - c.retirePeerLocked(peer, &closeConnections) + stalePeers = append(stalePeers, cachedPeer{owner: owner, peer: peer}) + } + if coord.GetEpoch() != epoch { + c.peerMu.Unlock() + return + } + for _, stale := range stalePeers { + delete(c.peers, stale.owner) + c.retirePeerLocked(stale.peer, &closeConnections) } + c.reconciledEpoch = epoch + c.reconciledEpochSet = true c.peerMu.Unlock() closeBlockPresenceConnections(closeConnections) } diff --git a/cmd/tag/block_presence_test.go b/cmd/tag/block_presence_test.go index 6cca6796..29c44efc 100644 --- a/cmd/tag/block_presence_test.go +++ b/cmd/tag/block_presence_test.go @@ -39,6 +39,32 @@ func (missingBlockPresenceOwnerCoordinator) GetNodeForKey(string) (*ring.NodeInf return nil, nil } +type blockPresenceEpochCoordinator uint64 + +func (c blockPresenceEpochCoordinator) GetEpoch() uint64 { return uint64(c) } +func (blockPresenceEpochCoordinator) GetLocalNodeID() string { return "local" } +func (blockPresenceEpochCoordinator) GetNodeForKey(string) (*ring.NodeInfo, error) { + return nil, nil +} + +type blockPresenceEpochSequence struct { + epochs []uint64 + calls int +} + +func (c *blockPresenceEpochSequence) GetEpoch() uint64 { + if c.calls < len(c.epochs) { + epoch := c.epochs[c.calls] + c.calls++ + return epoch + } + return c.epochs[len(c.epochs)-1] +} +func (*blockPresenceEpochSequence) GetLocalNodeID() string { return "local" } +func (*blockPresenceEpochSequence) GetNodeForKey(string) (*ring.NodeInfo, error) { + return nil, nil +} + func TestCheckBlockPresenceOwnerRetriesWhenNodeIsMissing(t *testing.T) { err := checkBlockPresenceOwner(missingBlockPresenceOwnerCoordinator{}, "block-key", "owner") if !errors.Is(err, cache.ErrBlockPresenceTopologyChanged) { @@ -63,10 +89,14 @@ func TestBlockPresencePeerPruningRetainsActiveOwnerAndLeasedConnection(t *testin // The current ring still contains the active peer even though this page has // no key for it. The retired owner has a live lease while the ring changes. + coord := blockPresenceEpochCoordinator(1) client.prunePeerConnections(map[string]string{ "active": "127.0.0.1:2", "new": "127.0.0.1:3", - }) + }, coord, 1) + if client.peerCacheNeedsReconcile(1) || !client.peerCacheNeedsReconcile(2) { + t.Fatal("peer cache reconciliation should be complete only for the observed epoch") + } client.peerMu.Lock() activePeer := client.peers["active"] _, retiredPresent := client.peers["retired"] @@ -98,6 +128,117 @@ func TestBlockPresencePeerPruningRetainsActiveOwnerAndLeasedConnection(t *testin } } +func TestBlockPresencePeerPruningDiscardsStaleEpochSnapshot(t *testing.T) { + client := newEmbeddedBlockCacheClient(nil) + t.Cleanup(func() { _ = client.Close() }) + + oldPeer, err := client.peerConnection("old-owner", "127.0.0.1:1") + if err != nil { + t.Fatalf("open old owner connection: %v", err) + } + client.releasePeerConnection(oldPeer) + currentPeer, err := client.peerConnection("current-owner", "127.0.0.1:2") + if err != nil { + t.Fatalf("open current owner connection: %v", err) + } + client.releasePeerConnection(currentPeer) + + // An older Range took this snapshot before the topology changed, but its + // cleanup reaches the peer cache only after the current epoch is visible. + // Epochs are opaque hashes, so the current value can be numerically smaller. + coord := blockPresenceEpochCoordinator(1) + client.prunePeerConnections(map[string]string{"old-owner": "127.0.0.1:1"}, coord, 2) + client.peerMu.Lock() + _, oldRetained := client.peers["old-owner"] + _, currentRetained := client.peers["current-owner"] + client.peerMu.Unlock() + if !oldRetained || !currentRetained { + t.Fatalf("stale epoch pruned peers: old=%t current=%t, want both retained", oldRetained, currentRetained) + } + + client.prunePeerConnections(map[string]string{"current-owner": "127.0.0.1:2"}, coord, 1) + if got := oldPeer.conn.GetState(); got != connectivity.Shutdown { + t.Fatalf("retired old owner connection state = %v, want shutdown", got) + } + if currentPeer.conn.GetState() == connectivity.Shutdown { + t.Fatal("current owner connection was closed") + } + + // A delayed completion from the earlier request must not sweep the cache a + // second time after the current snapshot has been reconciled. + client.prunePeerConnections(map[string]string{"old-owner": "127.0.0.1:1"}, coord, 2) + client.peerMu.Lock() + _, currentRetained = client.peers["current-owner"] + client.peerMu.Unlock() + if !currentRetained || client.peerCacheNeedsReconcile(1) { + t.Fatal("stale snapshot changed a cache reconciled for the current epoch") + } +} + +func TestBlockPresencePeerPruningDiscardsUnstableSnapshot(t *testing.T) { + client := newEmbeddedBlockCacheClient(nil) + t.Cleanup(func() { _ = client.Close() }) + + stalePeer, err := client.peerConnection("stale-owner", "127.0.0.1:1") + if err != nil { + t.Fatalf("open stale owner connection: %v", err) + } + client.releasePeerConnection(stalePeer) + currentPeer, err := client.peerConnection("current-owner", "127.0.0.1:2") + if err != nil { + t.Fatalf("open current owner connection: %v", err) + } + client.releasePeerConnection(currentPeer) + + // A ring update during the peer scan invalidates the owner snapshot. The + // prune must not close connections or mark the new epoch as reconciled. + changingCoord := &blockPresenceEpochSequence{epochs: []uint64{17, 8}} + client.prunePeerConnections(map[string]string{"current-owner": "127.0.0.1:2"}, changingCoord, 17) + client.peerMu.Lock() + _, staleRetained := client.peers["stale-owner"] + _, currentRetained := client.peers["current-owner"] + client.peerMu.Unlock() + if !staleRetained || !currentRetained || !client.peerCacheNeedsReconcile(8) { + t.Fatal("unstable owner snapshot pruned peers or marked the new epoch reconciled") + } + + client.prunePeerConnections(map[string]string{"current-owner": "127.0.0.1:2"}, blockPresenceEpochCoordinator(8), 8) + if got := stalePeer.conn.GetState(); got != connectivity.Shutdown { + t.Fatalf("stale owner connection after stable retry = %v, want shutdown", got) + } +} + +func TestBlockPresencePeerConnectionReplacesChangedAddress(t *testing.T) { + client := newEmbeddedBlockCacheClient(nil) + t.Cleanup(func() { _ = client.Close() }) + + coord := blockPresenceEpochCoordinator(1) + oldPeer, err := client.peerConnection("owner", "127.0.0.1:1") + if err != nil { + t.Fatalf("open original owner connection: %v", err) + } + client.releasePeerConnection(oldPeer) + client.prunePeerConnections(map[string]string{"owner": "127.0.0.1:1"}, coord, 1) + if client.peerCacheNeedsReconcile(1) { + t.Fatal("stable owner view was not reconciled") + } + newPeer, err := client.peerConnection("owner", "127.0.0.1:2") + if err != nil { + t.Fatalf("open updated owner connection: %v", err) + } + defer client.releasePeerConnection(newPeer) + + if oldPeer == newPeer { + t.Fatal("address change reused the previous peer connection") + } + if got := oldPeer.conn.GetState(); got != connectivity.Shutdown { + t.Fatalf("old address connection state = %v, want shutdown", got) + } + if newPeer.conn.GetState() == connectivity.Shutdown { + t.Fatal("new address connection is already closed") + } +} + func TestBlockPresenceRequestFallsBackForNonUTF8Keys(t *testing.T) { key := string([]byte{'b', 'l', 'k', '|', 0xff}) if _, err := encodeBlockPresenceRequest("owner", 1, []string{key}); !errors.Is(err, cache.ErrBlockPresenceUnsupported) { From 101ab5376687e907ca66321151b49835a52b154a Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Fri, 2 Oct 2026 03:42:49 +0000 Subject: [PATCH 8/9] fix(cache): prune peers when addresses change Signed-off-by: Perfloop Agent --- cmd/tag/block_cache_client.go | 46 +++---- cmd/tag/block_presence_test.go | 240 ++++++++++++++++++++++++++------- 2 files changed, 209 insertions(+), 77 deletions(-) diff --git a/cmd/tag/block_cache_client.go b/cmd/tag/block_cache_client.go index 3d39208d..a7f11e2e 100644 --- a/cmd/tag/block_cache_client.go +++ b/cmd/tag/block_cache_client.go @@ -23,15 +23,13 @@ import ( type embeddedBlockCacheClient struct { *embedded.Client - dialOptions []grpc.DialOption - peerMu sync.Mutex - peers map[string]*blockPresencePeer - retiredPeers map[*blockPresencePeer]struct{} // detached entries kept only while leased - closed bool - closeOnce sync.Once - closeErr error - reconciledEpoch uint64 - reconciledEpochSet bool + dialOptions []grpc.DialOption + peerMu sync.Mutex + peers map[string]*blockPresencePeer + retiredPeers map[*blockPresencePeer]struct{} // detached entries kept only while leased + closed bool + closeOnce sync.Once + closeErr error } // Peer lifecycle fields are protected by embeddedBlockCacheClient.peerMu. active @@ -87,9 +85,10 @@ func (c *embeddedBlockCacheClient) BlockPresence(ctx context.Context, keys []str if coord != nil { epoch = coord.GetEpoch() localID = coord.GetLocalNodeID() - if ringManager := coord.GetRing(); ringManager != nil && c.peerCacheNeedsReconcile(epoch) { - activeOwners := make(map[string]string) - for _, node := range ringManager.GetActiveNodes() { + if ringManager := coord.GetRing(); ringManager != nil && c.hasCachedPeerConnections() { + nodes := ringManager.GetActiveNodes() + activeOwners := make(map[string]string, len(nodes)) + for _, node := range nodes { if node != nil && node.ID != "" { activeOwners[node.ID] = node.ListenAddress } @@ -315,27 +314,26 @@ func (c *embeddedBlockCacheClient) peerConnection(owner, address string) (*block return peer, nil } -func (c *embeddedBlockCacheClient) peerCacheNeedsReconcile(epoch uint64) bool { +func (c *embeddedBlockCacheClient) hasCachedPeerConnections() bool { c.peerMu.Lock() defer c.peerMu.Unlock() - return !c.closed && (!c.reconciledEpochSet || c.reconciledEpoch != epoch) + return !c.closed && len(c.peers) > 0 } -// prunePeerConnections evicts cached connections for owners no longer active in -// the full ring snapshot. Page membership is deliberately not used: an active -// peer absent from one Range page still belongs in the cache. The epoch is an -// opaque topology fingerprint, so reconciliation is equality-gated and the full -// owner/peer scan stays off the steady-state Range path. A stale snapshot is -// discarded if the coordinator has already observed another epoch; this sweep -// is only connection cleanup, while per-key routing remains authoritative. An -// address change for an owner used by this page is handled by peerConnection. +// prunePeerConnections evicts cached connections for owners no longer active or +// whose address changed in the full ring snapshot. Page membership is deliberately +// not used: an active peer absent from one Range page still belongs in the cache. +// Address-only changes do not affect the ring epoch, so inspect the snapshot on +// each page while peers are cached. Epoch checks discard snapshots made stale by +// membership/state changes; per-key routing and the caller's post-page checks +// remain authoritative for the exchange. This sweep only manages connections. func (c *embeddedBlockCacheClient) prunePeerConnections(activeOwners map[string]string, coord blockPresenceCoordinator, epoch uint64) { if len(activeOwners) == 0 { return } var closeConnections []*grpc.ClientConn c.peerMu.Lock() - if c.closed || (c.reconciledEpochSet && c.reconciledEpoch == epoch) || coord.GetEpoch() != epoch { + if c.closed || coord.GetEpoch() != epoch { c.peerMu.Unlock() return } @@ -359,8 +357,6 @@ func (c *embeddedBlockCacheClient) prunePeerConnections(activeOwners map[string] delete(c.peers, stale.owner) c.retirePeerLocked(stale.peer, &closeConnections) } - c.reconciledEpoch = epoch - c.reconciledEpochSet = true c.peerMu.Unlock() closeBlockPresenceConnections(closeConnections) } diff --git a/cmd/tag/block_presence_test.go b/cmd/tag/block_presence_test.go index 29c44efc..8cdc9ca2 100644 --- a/cmd/tag/block_presence_test.go +++ b/cmd/tag/block_presence_test.go @@ -94,8 +94,8 @@ func TestBlockPresencePeerPruningRetainsActiveOwnerAndLeasedConnection(t *testin "active": "127.0.0.1:2", "new": "127.0.0.1:3", }, coord, 1) - if client.peerCacheNeedsReconcile(1) || !client.peerCacheNeedsReconcile(2) { - t.Fatal("peer cache reconciliation should be complete only for the observed epoch") + if !client.hasCachedPeerConnections() { + t.Fatal("active peer connection is missing from the cache") } client.peerMu.Lock() activePeer := client.peers["active"] @@ -170,8 +170,8 @@ func TestBlockPresencePeerPruningDiscardsStaleEpochSnapshot(t *testing.T) { client.peerMu.Lock() _, currentRetained = client.peers["current-owner"] client.peerMu.Unlock() - if !currentRetained || client.peerCacheNeedsReconcile(1) { - t.Fatal("stale snapshot changed a cache reconciled for the current epoch") + if !currentRetained { + t.Fatal("stale snapshot changed the current peer cache") } } @@ -198,8 +198,8 @@ func TestBlockPresencePeerPruningDiscardsUnstableSnapshot(t *testing.T) { _, staleRetained := client.peers["stale-owner"] _, currentRetained := client.peers["current-owner"] client.peerMu.Unlock() - if !staleRetained || !currentRetained || !client.peerCacheNeedsReconcile(8) { - t.Fatal("unstable owner snapshot pruned peers or marked the new epoch reconciled") + if !staleRetained || !currentRetained || !client.hasCachedPeerConnections() { + t.Fatal("unstable owner snapshot pruned cached peers") } client.prunePeerConnections(map[string]string{"current-owner": "127.0.0.1:2"}, blockPresenceEpochCoordinator(8), 8) @@ -219,9 +219,6 @@ func TestBlockPresencePeerConnectionReplacesChangedAddress(t *testing.T) { } client.releasePeerConnection(oldPeer) client.prunePeerConnections(map[string]string{"owner": "127.0.0.1:1"}, coord, 1) - if client.peerCacheNeedsReconcile(1) { - t.Fatal("stable owner view was not reconciled") - } newPeer, err := client.peerConnection("owner", "127.0.0.1:2") if err != nil { t.Fatalf("open updated owner connection: %v", err) @@ -239,6 +236,93 @@ func TestBlockPresencePeerConnectionReplacesChangedAddress(t *testing.T) { } } +func TestBlockPresenceAddressChangeDefersClosingLeasedPeer(t *testing.T) { + requestReceived := make(chan struct{}) + releaseResponse := make(chan struct{}) + var releaseOnce sync.Once + release := func() { releaseOnce.Do(func() { close(releaseResponse) }) } + defer release() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + server := grpc.NewServer(grpc.UnknownServiceHandler(func(_ any, stream grpc.ServerStream) error { + var request structpb.Struct + if err := stream.RecvMsg(&request); err != nil { + return err + } + close(requestReceived) + <-releaseResponse + response, err := encodeBlockPresenceResponse([]bool{true}, 0) + if err != nil { + return err + } + return stream.SendMsg(response) + })) + go func() { _ = server.Serve(listener) }() + t.Cleanup(func() { + server.Stop() + _ = listener.Close() + }) + + client := newEmbeddedBlockCacheClient(nil) + t.Cleanup(func() { _ = client.Close() }) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + result := make(chan error, 1) + go func() { + _, err := client.remoteBlockPresence(ctx, "owner", listener.Addr().String(), 0, []string{"blk|bucket|object|etag|4|0"}) + result <- err + }() + select { + case <-requestReceived: + case <-ctx.Done(): + t.Fatalf("presence request did not reach peer: %v", ctx.Err()) + } + + client.peerMu.Lock() + peer := client.peers["owner"] + active := 0 + if peer != nil { + active = peer.active + } + client.peerMu.Unlock() + if peer == nil || active != 1 { + t.Fatalf("in-flight peer lease = (%v,%d), want one active lease", peer, active) + } + + // The address changes while the peer RPC is active. Pruning must detach the + // old connection but leave it open until remoteBlockPresence releases its lease. + client.prunePeerConnections(map[string]string{"owner": "127.0.0.1:2"}, blockPresenceEpochCoordinator(1), 1) + client.peerMu.Lock() + _, cached := client.peers["owner"] + _, retired := client.retiredPeers[peer] + client.peerMu.Unlock() + if cached || !retired || peer.conn.GetState() == connectivity.Shutdown { + t.Fatalf("leased peer after prune = (cached=%t, retired=%t, state=%v), want detached and open", cached, retired, peer.conn.GetState()) + } + + release() + select { + case err := <-result: + if err != nil { + t.Fatalf("presence exchange: %v", err) + } + case <-ctx.Done(): + t.Fatalf("presence exchange did not finish after release: %v", ctx.Err()) + } + if got := peer.conn.GetState(); got != connectivity.Shutdown { + t.Fatalf("released retired peer state = %v, want shutdown", got) + } + client.peerMu.Lock() + _, retired = client.retiredPeers[peer] + client.peerMu.Unlock() + if retired { + t.Fatal("released peer lease remained in retired peer set") + } +} + func TestBlockPresenceRequestFallsBackForNonUTF8Keys(t *testing.T) { key := string([]byte{'b', 'l', 'k', '|', 0xff}) if _, err := encodeBlockPresenceRequest("owner", 1, []string{key}); !errors.Is(err, cache.ErrBlockPresenceUnsupported) { @@ -682,7 +766,7 @@ func TestEmbeddedBlockPresenceMultiNodeAndFallback(t *testing.T) { fixture.closeNodes[1]() waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0") - replacement := startEmbeddedBlockRangeReplacementNode(t, fixture, "range-bench-replacement") + replacement, closeReplacement := startEmbeddedBlockRangeReplacementNode(t, fixture, "range-bench-replacement") waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0", "range-bench-replacement") waitForMatchingEmbeddedEpoch(t, fixture.embeddedClient, replacement) @@ -715,9 +799,54 @@ func TestEmbeddedBlockPresenceMultiNodeAndFallback(t *testing.T) { if stalePeer != nil && stalePeer.conn.GetState() != connectivity.Shutdown { t.Fatalf("stale owner connection state = %v, want shutdown", stalePeer.conn.GetState()) } + + // Rejoin the same active node ID with the same token count but a new address. + // OCache's epoch is based on membership/state/token count, so the final epoch + // is unchanged even though the peer connection above points at the old address. + addressOnlyEpoch := coord.GetEpoch() + oldAddress := newOwner.ListenAddress + closeReplacement() + waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0") + replacement, closeReplacement = startEmbeddedBlockRangeReplacementNode(t, fixture, "range-bench-replacement") + defer closeReplacement() + waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0", "range-bench-replacement") + waitForMatchingEmbeddedEpoch(t, fixture.embeddedClient, replacement) + if got := coord.GetEpoch(); got != addressOnlyEpoch { + t.Fatalf("ring epoch after same-ID address change = %d, want unchanged %d", got, addressOnlyEpoch) + } + updatedOwner, err := coord.GetNodeForKey(oldOwnerKey) + if err != nil || updatedOwner.ID != "range-bench-replacement" || updatedOwner.ListenAddress == oldAddress { + t.Fatalf("same-ID owner address = (%v,%v), want %q changed", updatedOwner, err, oldAddress) + } + for _, index := range []int64{2, 3} { + key := cache.MakeBlockKey(bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, index) + owner, err := coord.GetNodeForKey(key) + if err != nil || owner.ID != coord.GetLocalNodeID() { + t.Fatalf("block %d owner = (%v,%v), want local node %q", index, owner, err, coord.GetLocalNodeID()) + } + } + + // This >1-block Range covers only local blocks, so the replacement peer is + // absent from the page. Reconciliation must still retire its old connection. + fixture.stats.reset() + const rangeStart = int64(2 * embeddedBlockRangeBenchmarkBlockSize) + const rangeEnd = int64(3*embeddedBlockRangeBenchmarkBlockSize + 127) + assertEmbeddedRangeBytes(t, fixture.client, fixture.gateway.URL, fixture.key, fixture.body, rangeStart, rangeEnd, fixture.gatewaySigner, fixture.accessKey, fixture.secretKey) + if got := fixture.stats.presenceRequests.Load(); got != 0 { + t.Fatalf("local-only Range made %d remote presence exchanges, want 0", got) + } + fixture.cacheClient.peerMu.Lock() + _, staleAddressRetained := fixture.cacheClient.peers["range-bench-replacement"] + fixture.cacheClient.peerMu.Unlock() + if staleAddressRetained { + t.Fatal("address-only ring change retained the unused old peer connection") + } + if got := replacementPeer.conn.GetState(); got != connectivity.Shutdown { + t.Fatalf("unused old-address connection state = %v, want shutdown", got) + } } -func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBlockRangeBenchmarkFixture, nodeID string) *embedded.Client { +func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBlockRangeBenchmarkFixture, nodeID string) (*embedded.Client, func()) { tb.Helper() diskPath := tb.TempDir() writeEmbeddedBlockRangeRingTokens(tb, diskPath, 1) @@ -745,7 +874,8 @@ func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBloc } presenceServer.client = client var closeOnce sync.Once - tb.Cleanup(func() { closeOnce.Do(func() { _ = client.Close() }) }) + closeClient := func() { closeOnce.Do(func() { _ = client.Close() }) } + tb.Cleanup(closeClient) if err := client.StartGRPCServer(); err != nil { tb.Fatalf("start replacement embedded node: %v", err) } @@ -754,7 +884,51 @@ func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBloc if err := client.WaitReady(readyCtx); err != nil { tb.Fatalf("replacement embedded node did not become ready: %v", err) } - return client + return client, closeClient +} + +func assertEmbeddedRangeBytes(tb testing.TB, client *http.Client, gatewayURL, key string, object []byte, start, end int64, signer *auth.RequestSigner, accessKey, secretKey string) { + tb.Helper() + req, err := http.NewRequest(http.MethodGet, gatewayURL+"/benchmark/"+key, nil) + if err != nil { + tb.Fatal(err) + } + req.Header.Set("Range", fmt.Sprintf("bytes=%d-%d", start, end)) + if signer == nil { + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test/20260101/us-east-1/s3/aws4_request, Signature=deadbeef") + } else { + signed, err := signer.SignRequest(req.Context(), req.Method, req.URL.RequestURI(), req.Body, "", accessKey, secretKey, req.Header) + if err != nil { + tb.Fatalf("sign byte-range request: %v", err) + } + signed.RemoteAddr = req.RemoteAddr + req = signed + } + resp, err := client.Do(req) + if err != nil { + tb.Fatalf("byte-range request: %v", err) + } + body, readErr := io.ReadAll(resp.Body) + closeErr := resp.Body.Close() + if readErr != nil { + tb.Fatalf("read byte-range response: %v", readErr) + } + if closeErr != nil { + tb.Fatalf("close byte-range response: %v", closeErr) + } + wantBody := object[start : end+1] + if resp.StatusCode != http.StatusPartialContent || resp.Header.Get(proxy.XCacheHeader) != proxy.XCacheHit || !bytes.Equal(body, wantBody) { + tb.Fatalf("byte-range response = (status=%d, cache=%q, bytes=%d, exact=%t), want exact cache 206", resp.StatusCode, resp.Header.Get(proxy.XCacheHeader), len(body), bytes.Equal(body, wantBody)) + } + if got, want := resp.Header.Get("Content-Range"), fmt.Sprintf("bytes %d-%d/%d", start, end, len(object)); got != want { + tb.Errorf("Content-Range=%q, want %q", got, want) + } + if got, want := resp.Header.Get("Content-Length"), fmt.Sprint(end-start+1); got != want { + tb.Errorf("Content-Length=%q, want %q", got, want) + } + if got := resp.Header.Get("ETag"); got != `"block-range"` { + tb.Errorf("ETag=%q, want %q", got, `"block-range"`) + } } func waitForMatchingEmbeddedEpoch(tb testing.TB, first, second *embedded.Client) { @@ -909,45 +1083,7 @@ func TestEmbeddedBlockPresenceUsesClusterGRPCAuth(t *testing.T) { func assertEmbeddedRangeResponse(tb testing.TB, client *http.Client, gatewayURL, key string, object []byte, blockCount int, signer *auth.RequestSigner, accessKey, secretKey string) { tb.Helper() rangeLen := blockCount * embeddedBlockRangeBenchmarkBlockSize - req, err := http.NewRequest(http.MethodGet, gatewayURL+"/benchmark/"+key, nil) - if err != nil { - tb.Fatal(err) - } - req.Header.Set("Range", fmt.Sprintf("bytes=0-%d", rangeLen-1)) - if signer == nil { - req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test/20260101/us-east-1/s3/aws4_request, Signature=deadbeef") - } else { - signed, err := signer.SignRequest(req.Context(), req.Method, req.URL.RequestURI(), req.Body, "", accessKey, secretKey, req.Header) - if err != nil { - tb.Fatalf("sign range request: %v", err) - } - signed.RemoteAddr = req.RemoteAddr - req = signed - } - resp, err := client.Do(req) - if err != nil { - tb.Fatalf("range request: %v", err) - } - body, readErr := io.ReadAll(resp.Body) - closeErr := resp.Body.Close() - if readErr != nil { - tb.Fatalf("read range response: %v", readErr) - } - if closeErr != nil { - tb.Fatalf("close range response: %v", closeErr) - } - if resp.StatusCode != http.StatusPartialContent || resp.Header.Get(proxy.XCacheHeader) != proxy.XCacheHit || !bytes.Equal(body, object[:rangeLen]) { - tb.Fatalf("range response = (status=%d, cache=%q, bytes=%d, exact=%t), want exact cache 206", resp.StatusCode, resp.Header.Get(proxy.XCacheHeader), len(body), bytes.Equal(body, object[:rangeLen])) - } - if got, want := resp.Header.Get("Content-Range"), fmt.Sprintf("bytes 0-%d/%d", rangeLen-1, len(object)); got != want { - tb.Errorf("Content-Range=%q, want %q", got, want) - } - if got, want := resp.Header.Get("Content-Length"), fmt.Sprint(rangeLen); got != want { - tb.Errorf("Content-Length=%q, want %q", got, want) - } - if got := resp.Header.Get("ETag"); got != `"block-range"` { - tb.Errorf("ETag=%q, want %q", got, `"block-range"`) - } + assertEmbeddedRangeBytes(tb, client, gatewayURL, key, object, 0, int64(rangeLen-1), signer, accessKey, secretKey) } func TestBlockPresencePeerCancellationIsFatalUnlessSiblingCanceledGroup(t *testing.T) { From f48de426cd3bd3a542d9d3c679c5d5a5b71f2852 Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Fri, 2 Oct 2026 04:26:13 +0000 Subject: [PATCH 9/9] refactor(cache): scope presence connections to exchanges Signed-off-by: Perfloop Agent --- cmd/tag/block_cache_client.go | 235 ++++------------ cmd/tag/block_presence_test.go | 490 +++++++++------------------------ 2 files changed, 187 insertions(+), 538 deletions(-) diff --git a/cmd/tag/block_cache_client.go b/cmd/tag/block_cache_client.go index a7f11e2e..3fd1a51a 100644 --- a/cmd/tag/block_cache_client.go +++ b/cmd/tag/block_cache_client.go @@ -23,23 +23,14 @@ import ( type embeddedBlockCacheClient struct { *embedded.Client - dialOptions []grpc.DialOption - peerMu sync.Mutex - peers map[string]*blockPresencePeer - retiredPeers map[*blockPresencePeer]struct{} // detached entries kept only while leased - closed bool - closeOnce sync.Once - closeErr error -} - -// Peer lifecycle fields are protected by embeddedBlockCacheClient.peerMu. active -// counts RPC leases; retired peers are closed after their last lease is released. -type blockPresencePeer struct { - address string - conn *grpc.ClientConn - active int // outstanding RPC leases, protected by peerMu - retired bool - closed bool + dialOptions []grpc.DialOption + lifecycleMu sync.Mutex + shutdownCtx context.Context + shutdownCancel context.CancelFunc + activeCalls sync.WaitGroup + closed bool + closeOnce sync.Once + closeErr error } type blockPresenceOwnerGroup struct { @@ -52,11 +43,12 @@ type blockPresenceOwnerGroup struct { func newEmbeddedBlockCacheClient(client *embedded.Client, dialOptions ...grpc.DialOption) *embeddedBlockCacheClient { options := append(cacheclient.DefaultDialOptions(), dialOptions...) + shutdownCtx, shutdownCancel := context.WithCancel(context.Background()) return &embeddedBlockCacheClient{ - Client: client, - dialOptions: options, - peers: make(map[string]*blockPresencePeer), - retiredPeers: make(map[*blockPresencePeer]struct{}), + Client: client, + dialOptions: options, + shutdownCtx: shutdownCtx, + shutdownCancel: shutdownCancel, } } @@ -85,18 +77,6 @@ func (c *embeddedBlockCacheClient) BlockPresence(ctx context.Context, keys []str if coord != nil { epoch = coord.GetEpoch() localID = coord.GetLocalNodeID() - if ringManager := coord.GetRing(); ringManager != nil && c.hasCachedPeerConnections() { - nodes := ringManager.GetActiveNodes() - activeOwners := make(map[string]string, len(nodes)) - for _, node := range nodes { - if node != nil && node.ID != "" { - activeOwners[node.ID] = node.ListenAddress - } - } - if len(activeOwners) > 0 { - c.prunePeerConnections(activeOwners, coord, epoch) - } - } } groups := make(map[string]*blockPresenceOwnerGroup) owners := make([]string, len(keys)) @@ -221,34 +201,42 @@ func (c *embeddedBlockCacheClient) remoteBlockPresence(ctx context.Context, owne } return nil, err } - peer, err := c.peerConnection(owner, address) + rpcCtx, finishCall, err := c.beginPresenceCall(ctx) if err != nil { return nil, err } - defer c.releasePeerConnection(peer) - stream, err := peer.conn.NewStream(ctx, &grpc.StreamDesc{ + defer finishCall() + // Keep this ClientConn scoped to one owner exchange. OCache's topology epoch + // omits addresses, so retaining owner channels would require a full-ring scan + // on the Range path to retire an unused owner's old address. + conn, err := grpc.NewClient(address, c.dialOptions...) + if err != nil { + return nil, err + } + defer func() { _ = conn.Close() }() + stream, err := conn.NewStream(rpcCtx, &grpc.StreamDesc{ StreamName: "Check", ClientStreams: true, ServerStreams: true, }, blockPresenceMethod) if err != nil { - return nil, blockPresenceRPCError(ctx, err) + return nil, blockPresenceRPCError(rpcCtx, err) } if err := stream.SendMsg(request); err != nil { // A server-side Unimplemented/FailedPrecondition can arrive before SendMsg // finishes. grpc-go reports io.EOF here and exposes the status on RecvMsg. if !errors.Is(err, io.EOF) { - return nil, blockPresenceRPCError(ctx, err) + return nil, blockPresenceRPCError(rpcCtx, err) } - return receiveBlockPresenceResponse(ctx, stream, epoch, len(keys)) + return receiveBlockPresenceResponse(rpcCtx, stream, epoch, len(keys)) } if err := stream.CloseSend(); err != nil { if !errors.Is(err, io.EOF) { - return nil, blockPresenceRPCError(ctx, err) + return nil, blockPresenceRPCError(rpcCtx, err) } - return receiveBlockPresenceResponse(ctx, stream, epoch, len(keys)) + return receiveBlockPresenceResponse(rpcCtx, stream, epoch, len(keys)) } - return receiveBlockPresenceResponse(ctx, stream, epoch, len(keys)) + return receiveBlockPresenceResponse(rpcCtx, stream, epoch, len(keys)) } func receiveBlockPresenceResponse(ctx context.Context, stream grpc.ClientStream, epoch uint64, keyCount int) ([]bool, error) { @@ -285,153 +273,38 @@ func blockPresenceRPCError(ctx context.Context, err error) error { } } -func (c *embeddedBlockCacheClient) peerConnection(owner, address string) (*blockPresencePeer, error) { - c.peerMu.Lock() +// beginPresenceCall keeps wrapper shutdown authoritative while a one-owner +// exchange owns its temporary ClientConn. Add happens under lifecycleMu so Close +// cannot begin waiting before the final call has registered. +func (c *embeddedBlockCacheClient) beginPresenceCall(parent context.Context) (context.Context, func(), error) { + c.lifecycleMu.Lock() if c.closed { - c.peerMu.Unlock() - return nil, errors.New("embedded cache client is closed") - } - var closeConnections []*grpc.ClientConn - if peer, ok := c.peers[owner]; ok { - if peer.address == address && !peer.retired { - peer.active++ - c.peerMu.Unlock() - return peer, nil - } - delete(c.peers, owner) - c.retirePeerLocked(peer, &closeConnections) - } - conn, err := grpc.NewClient(address, c.dialOptions...) - if err != nil { - c.peerMu.Unlock() - closeBlockPresenceConnections(closeConnections) - return nil, err - } - peer := &blockPresencePeer{address: address, conn: conn, active: 1} - c.peers[owner] = peer - c.peerMu.Unlock() - closeBlockPresenceConnections(closeConnections) - return peer, nil -} - -func (c *embeddedBlockCacheClient) hasCachedPeerConnections() bool { - c.peerMu.Lock() - defer c.peerMu.Unlock() - return !c.closed && len(c.peers) > 0 -} - -// prunePeerConnections evicts cached connections for owners no longer active or -// whose address changed in the full ring snapshot. Page membership is deliberately -// not used: an active peer absent from one Range page still belongs in the cache. -// Address-only changes do not affect the ring epoch, so inspect the snapshot on -// each page while peers are cached. Epoch checks discard snapshots made stale by -// membership/state changes; per-key routing and the caller's post-page checks -// remain authoritative for the exchange. This sweep only manages connections. -func (c *embeddedBlockCacheClient) prunePeerConnections(activeOwners map[string]string, coord blockPresenceCoordinator, epoch uint64) { - if len(activeOwners) == 0 { - return - } - var closeConnections []*grpc.ClientConn - c.peerMu.Lock() - if c.closed || coord.GetEpoch() != epoch { - c.peerMu.Unlock() - return - } - type cachedPeer struct { - owner string - peer *blockPresencePeer - } - var stalePeers []cachedPeer - for owner, peer := range c.peers { - address, active := activeOwners[owner] - if active && address == peer.address { - continue - } - stalePeers = append(stalePeers, cachedPeer{owner: owner, peer: peer}) - } - if coord.GetEpoch() != epoch { - c.peerMu.Unlock() - return - } - for _, stale := range stalePeers { - delete(c.peers, stale.owner) - c.retirePeerLocked(stale.peer, &closeConnections) - } - c.peerMu.Unlock() - closeBlockPresenceConnections(closeConnections) + c.lifecycleMu.Unlock() + return nil, nil, errors.New("embedded cache client is closed") + } + c.activeCalls.Add(1) + rpcCtx, cancel := context.WithCancel(parent) + stopShutdown := context.AfterFunc(c.shutdownCtx, cancel) + c.lifecycleMu.Unlock() + return rpcCtx, func() { + stopShutdown() + cancel() + c.activeCalls.Done() + }, nil } -// retirePeerLocked defers closing a detached connection until no RPC lease uses it. -func (c *embeddedBlockCacheClient) retirePeerLocked(peer *blockPresencePeer, closeConnections *[]*grpc.ClientConn) { - peer.retired = true - if peer.active == 0 { - if !peer.closed { - peer.closed = true - *closeConnections = append(*closeConnections, peer.conn) - } - return - } - c.retiredPeers[peer] = struct{}{} -} - -func (c *embeddedBlockCacheClient) releasePeerConnection(peer *blockPresencePeer) { - var closeConnection *grpc.ClientConn - c.peerMu.Lock() - peer.active-- - if peer.active == 0 && peer.retired && !peer.closed { - peer.closed = true - delete(c.retiredPeers, peer) - closeConnection = peer.conn - } - c.peerMu.Unlock() - if closeConnection != nil { - _ = closeConnection.Close() - } -} - -func closeBlockPresenceConnections(connections []*grpc.ClientConn) { - for _, connection := range connections { - _ = connection.Close() - } -} - -// Close releases the optional peer connections before stopping the embedded -// cache. The embedded cache close remains the caller-visible behavior. +// Close cancels and joins active presence exchanges before closing the embedded +// cache. Each exchange closes its own ClientConn when it returns. func (c *embeddedBlockCacheClient) Close() error { c.closeOnce.Do(func() { - c.peerMu.Lock() + c.lifecycleMu.Lock() c.closed = true - peers := make(map[*blockPresencePeer]struct{}, len(c.peers)+len(c.retiredPeers)) - for _, peer := range c.peers { - peers[peer] = struct{}{} - } - for peer := range c.retiredPeers { - peers[peer] = struct{}{} - } - c.peers = nil - c.retiredPeers = nil - var connections []*grpc.ClientConn - for peer := range peers { - peer.retired = true - if !peer.closed { - peer.closed = true - connections = append(connections, peer.conn) - } - } - c.peerMu.Unlock() - - var closeErrors []error - for _, connection := range connections { - if err := connection.Close(); err != nil { - closeErrors = append(closeErrors, err) - } - } + c.lifecycleMu.Unlock() + c.shutdownCancel() + c.activeCalls.Wait() if c.Client != nil { - if err := c.Client.Close(); err != nil { - closeErrors = append(closeErrors, err) - } + c.closeErr = c.Client.Close() } - c.closeErr = errors.Join(closeErrors...) }) return c.closeErr } diff --git a/cmd/tag/block_presence_test.go b/cmd/tag/block_presence_test.go index 8cdc9ca2..a5ecd6a3 100644 --- a/cmd/tag/block_presence_test.go +++ b/cmd/tag/block_presence_test.go @@ -39,32 +39,6 @@ func (missingBlockPresenceOwnerCoordinator) GetNodeForKey(string) (*ring.NodeInf return nil, nil } -type blockPresenceEpochCoordinator uint64 - -func (c blockPresenceEpochCoordinator) GetEpoch() uint64 { return uint64(c) } -func (blockPresenceEpochCoordinator) GetLocalNodeID() string { return "local" } -func (blockPresenceEpochCoordinator) GetNodeForKey(string) (*ring.NodeInfo, error) { - return nil, nil -} - -type blockPresenceEpochSequence struct { - epochs []uint64 - calls int -} - -func (c *blockPresenceEpochSequence) GetEpoch() uint64 { - if c.calls < len(c.epochs) { - epoch := c.epochs[c.calls] - c.calls++ - return epoch - } - return c.epochs[len(c.epochs)-1] -} -func (*blockPresenceEpochSequence) GetLocalNodeID() string { return "local" } -func (*blockPresenceEpochSequence) GetNodeForKey(string) (*ring.NodeInfo, error) { - return nil, nil -} - func TestCheckBlockPresenceOwnerRetriesWhenNodeIsMissing(t *testing.T) { err := checkBlockPresenceOwner(missingBlockPresenceOwnerCoordinator{}, "block-key", "owner") if !errors.Is(err, cache.ErrBlockPresenceTopologyChanged) { @@ -72,193 +46,89 @@ func TestCheckBlockPresenceOwnerRetriesWhenNodeIsMissing(t *testing.T) { } } -func TestBlockPresencePeerPruningRetainsActiveOwnerAndLeasedConnection(t *testing.T) { - client := newEmbeddedBlockCacheClient(nil) - t.Cleanup(func() { _ = client.Close() }) - - retiredLease, err := client.peerConnection("retired", "127.0.0.1:1") - if err != nil { - t.Fatalf("open retired peer connection: %v", err) - } - activeLease, err := client.peerConnection("active", "127.0.0.1:2") - if err != nil { - t.Fatalf("open active peer connection: %v", err) - } - activeConn := activeLease.conn - client.releasePeerConnection(activeLease) - - // The current ring still contains the active peer even though this page has - // no key for it. The retired owner has a live lease while the ring changes. - coord := blockPresenceEpochCoordinator(1) - client.prunePeerConnections(map[string]string{ - "active": "127.0.0.1:2", - "new": "127.0.0.1:3", - }, coord, 1) - if !client.hasCachedPeerConnections() { - t.Fatal("active peer connection is missing from the cache") - } - client.peerMu.Lock() - activePeer := client.peers["active"] - _, retiredPresent := client.peers["retired"] - _, leaseRetained := client.retiredPeers[retiredLease] - client.peerMu.Unlock() - if activePeer != activeLease || retiredPresent || !leaseRetained { - t.Fatalf("pruned peers: active retained=%t retired present=%t lease retained=%t", activePeer == activeLease, retiredPresent, leaseRetained) - } - if activeConn.GetState() == connectivity.Shutdown || retiredLease.conn.GetState() == connectivity.Shutdown { - t.Fatal("pruning closed an active owner or a leased connection") - } - - newLease, err := client.peerConnection("new", "127.0.0.1:3") - if err != nil { - t.Fatalf("open replacement peer connection: %v", err) - } - client.releasePeerConnection(newLease) - client.releasePeerConnection(retiredLease) - if got := retiredLease.conn.GetState(); got != connectivity.Shutdown { - t.Fatalf("released retired connection state = %v, want shutdown", got) - } - client.peerMu.Lock() - _, activeRetained := client.peers["active"] - _, newRetained := client.peers["new"] - _, retiredLeaseRetained := client.retiredPeers[retiredLease] - client.peerMu.Unlock() - if !activeRetained || !newRetained || retiredLeaseRetained { - t.Fatalf("final peer cache: active=%t new=%t retired lease=%t", activeRetained, newRetained, retiredLeaseRetained) - } -} - -func TestBlockPresencePeerPruningDiscardsStaleEpochSnapshot(t *testing.T) { - client := newEmbeddedBlockCacheClient(nil) - t.Cleanup(func() { _ = client.Close() }) - - oldPeer, err := client.peerConnection("old-owner", "127.0.0.1:1") - if err != nil { - t.Fatalf("open old owner connection: %v", err) - } - client.releasePeerConnection(oldPeer) - currentPeer, err := client.peerConnection("current-owner", "127.0.0.1:2") - if err != nil { - t.Fatalf("open current owner connection: %v", err) - } - client.releasePeerConnection(currentPeer) - - // An older Range took this snapshot before the topology changed, but its - // cleanup reaches the peer cache only after the current epoch is visible. - // Epochs are opaque hashes, so the current value can be numerically smaller. - coord := blockPresenceEpochCoordinator(1) - client.prunePeerConnections(map[string]string{"old-owner": "127.0.0.1:1"}, coord, 2) - client.peerMu.Lock() - _, oldRetained := client.peers["old-owner"] - _, currentRetained := client.peers["current-owner"] - client.peerMu.Unlock() - if !oldRetained || !currentRetained { - t.Fatalf("stale epoch pruned peers: old=%t current=%t, want both retained", oldRetained, currentRetained) - } - - client.prunePeerConnections(map[string]string{"current-owner": "127.0.0.1:2"}, coord, 1) - if got := oldPeer.conn.GetState(); got != connectivity.Shutdown { - t.Fatalf("retired old owner connection state = %v, want shutdown", got) - } - if currentPeer.conn.GetState() == connectivity.Shutdown { - t.Fatal("current owner connection was closed") - } - - // A delayed completion from the earlier request must not sweep the cache a - // second time after the current snapshot has been reconciled. - client.prunePeerConnections(map[string]string{"old-owner": "127.0.0.1:1"}, coord, 2) - client.peerMu.Lock() - _, currentRetained = client.peers["current-owner"] - client.peerMu.Unlock() - if !currentRetained { - t.Fatal("stale snapshot changed the current peer cache") - } -} - -func TestBlockPresencePeerPruningDiscardsUnstableSnapshot(t *testing.T) { - client := newEmbeddedBlockCacheClient(nil) - t.Cleanup(func() { _ = client.Close() }) - - stalePeer, err := client.peerConnection("stale-owner", "127.0.0.1:1") - if err != nil { - t.Fatalf("open stale owner connection: %v", err) - } - client.releasePeerConnection(stalePeer) - currentPeer, err := client.peerConnection("current-owner", "127.0.0.1:2") - if err != nil { - t.Fatalf("open current owner connection: %v", err) - } - client.releasePeerConnection(currentPeer) - - // A ring update during the peer scan invalidates the owner snapshot. The - // prune must not close connections or mark the new epoch as reconciled. - changingCoord := &blockPresenceEpochSequence{epochs: []uint64{17, 8}} - client.prunePeerConnections(map[string]string{"current-owner": "127.0.0.1:2"}, changingCoord, 17) - client.peerMu.Lock() - _, staleRetained := client.peers["stale-owner"] - _, currentRetained := client.peers["current-owner"] - client.peerMu.Unlock() - if !staleRetained || !currentRetained || !client.hasCachedPeerConnections() { - t.Fatal("unstable owner snapshot pruned cached peers") +func TestRemoteBlockPresenceClosesEachAddressConnection(t *testing.T) { + requestSeen := []chan struct{}{make(chan struct{}, 1), make(chan struct{}, 1)} + listeners := make([]net.Listener, len(requestSeen)) + servers := make([]*grpc.Server, len(requestSeen)) + for i := range requestSeen { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + listeners[i] = listener + index := i + servers[i] = grpc.NewServer(grpc.UnknownServiceHandler(func(_ any, stream grpc.ServerStream) error { + var request structpb.Struct + if err := stream.RecvMsg(&request); err != nil { + return err + } + requestSeen[index] <- struct{}{} + response, err := encodeBlockPresenceResponse([]bool{true}, 0) + if err != nil { + return err + } + return stream.SendMsg(response) + })) + go func(i int) { _ = servers[i].Serve(listeners[i]) }(i) + t.Cleanup(func() { + servers[index].Stop() + _ = listeners[index].Close() + }) } - client.prunePeerConnections(map[string]string{"current-owner": "127.0.0.1:2"}, blockPresenceEpochCoordinator(8), 8) - if got := stalePeer.conn.GetState(); got != connectivity.Shutdown { - t.Fatalf("stale owner connection after stable retry = %v, want shutdown", got) + var connMu sync.Mutex + var connections []*grpc.ClientConn + streamInterceptor := func(ctx context.Context, desc *grpc.StreamDesc, conn *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { + connMu.Lock() + connections = append(connections, conn) + connMu.Unlock() + return streamer(ctx, desc, conn, method, opts...) } -} - -func TestBlockPresencePeerConnectionReplacesChangedAddress(t *testing.T) { - client := newEmbeddedBlockCacheClient(nil) + client := newEmbeddedBlockCacheClient(nil, grpc.WithStreamInterceptor(streamInterceptor)) t.Cleanup(func() { _ = client.Close() }) - coord := blockPresenceEpochCoordinator(1) - oldPeer, err := client.peerConnection("owner", "127.0.0.1:1") - if err != nil { - t.Fatalf("open original owner connection: %v", err) - } - client.releasePeerConnection(oldPeer) - client.prunePeerConnections(map[string]string{"owner": "127.0.0.1:1"}, coord, 1) - newPeer, err := client.peerConnection("owner", "127.0.0.1:2") - if err != nil { - t.Fatalf("open updated owner connection: %v", err) - } - defer client.releasePeerConnection(newPeer) - - if oldPeer == newPeer { - t.Fatal("address change reused the previous peer connection") - } - if got := oldPeer.conn.GetState(); got != connectivity.Shutdown { - t.Fatalf("old address connection state = %v, want shutdown", got) + for i, listener := range listeners { + found, err := client.remoteBlockPresence(context.Background(), "same-owner", listener.Addr().String(), 0, []string{"blk|b|k|e|4|0"}) + if err != nil || len(found) != 1 || !found[0] { + t.Fatalf("presence exchange %d = (%v,%v), want one present result", i, found, err) + } + select { + case <-requestSeen[i]: + case <-time.After(2 * time.Second): + t.Fatalf("address %d did not receive a presence request", i) + } + connMu.Lock() + if len(connections) != i+1 { + connMu.Unlock() + t.Fatalf("presence exchanges opened %d ClientConns, want %d", len(connections), i+1) + } + conn := connections[i] + connMu.Unlock() + if got := conn.GetState(); got != connectivity.Shutdown { + t.Fatalf("presence exchange %d retained its ClientConn in state %v", i, got) + } } - if newPeer.conn.GetState() == connectivity.Shutdown { - t.Fatal("new address connection is already closed") + if connections[0] == connections[1] { + t.Fatal("address change reused the previous owner's ClientConn") } } -func TestBlockPresenceAddressChangeDefersClosingLeasedPeer(t *testing.T) { - requestReceived := make(chan struct{}) - releaseResponse := make(chan struct{}) - var releaseOnce sync.Once - release := func() { releaseOnce.Do(func() { close(releaseResponse) }) } - defer release() - +func TestEmbeddedBlockCacheClientCloseCancelsActivePresence(t *testing.T) { listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatal(err) } + requestReceived := make(chan struct{}) + serverCanceled := make(chan struct{}) server := grpc.NewServer(grpc.UnknownServiceHandler(func(_ any, stream grpc.ServerStream) error { var request structpb.Struct if err := stream.RecvMsg(&request); err != nil { return err } close(requestReceived) - <-releaseResponse - response, err := encodeBlockPresenceResponse([]bool{true}, 0) - if err != nil { - return err - } - return stream.SendMsg(response) + <-stream.Context().Done() + close(serverCanceled) + return status.FromContextError(stream.Context().Err()).Err() })) go func() { _ = server.Serve(listener) }() t.Cleanup(func() { @@ -268,58 +138,39 @@ func TestBlockPresenceAddressChangeDefersClosingLeasedPeer(t *testing.T) { client := newEmbeddedBlockCacheClient(nil) t.Cleanup(func() { _ = client.Close() }) - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - result := make(chan error, 1) + requestErr := make(chan error, 1) go func() { - _, err := client.remoteBlockPresence(ctx, "owner", listener.Addr().String(), 0, []string{"blk|bucket|object|etag|4|0"}) - result <- err + _, err := client.remoteBlockPresence(context.Background(), "owner", listener.Addr().String(), 0, []string{"blk|b|k|e|4|0"}) + requestErr <- err }() select { case <-requestReceived: - case <-ctx.Done(): - t.Fatalf("presence request did not reach peer: %v", ctx.Err()) + case <-time.After(2 * time.Second): + t.Fatal("presence request did not reach the owner") } - client.peerMu.Lock() - peer := client.peers["owner"] - active := 0 - if peer != nil { - active = peer.active - } - client.peerMu.Unlock() - if peer == nil || active != 1 { - t.Fatalf("in-flight peer lease = (%v,%d), want one active lease", peer, active) + closeErr := make(chan error, 1) + go func() { closeErr <- client.Close() }() + select { + case <-serverCanceled: + case <-time.After(2 * time.Second): + t.Fatal("client Close did not cancel the active presence RPC") } - - // The address changes while the peer RPC is active. Pruning must detach the - // old connection but leave it open until remoteBlockPresence releases its lease. - client.prunePeerConnections(map[string]string{"owner": "127.0.0.1:2"}, blockPresenceEpochCoordinator(1), 1) - client.peerMu.Lock() - _, cached := client.peers["owner"] - _, retired := client.retiredPeers[peer] - client.peerMu.Unlock() - if cached || !retired || peer.conn.GetState() == connectivity.Shutdown { - t.Fatalf("leased peer after prune = (cached=%t, retired=%t, state=%v), want detached and open", cached, retired, peer.conn.GetState()) + select { + case err := <-requestErr: + if !errors.Is(err, context.Canceled) { + t.Fatalf("presence error after client Close = %v, want context.Canceled", err) + } + case <-time.After(2 * time.Second): + t.Fatal("presence RPC did not return after client Close") } - - release() select { - case err := <-result: + case err := <-closeErr: if err != nil { - t.Fatalf("presence exchange: %v", err) + t.Fatalf("client Close: %v", err) } - case <-ctx.Done(): - t.Fatalf("presence exchange did not finish after release: %v", ctx.Err()) - } - if got := peer.conn.GetState(); got != connectivity.Shutdown { - t.Fatalf("released retired peer state = %v, want shutdown", got) - } - client.peerMu.Lock() - _, retired = client.retiredPeers[peer] - client.peerMu.Unlock() - if retired { - t.Fatal("released peer lease remained in retired peer set") + case <-time.After(2 * time.Second): + t.Fatal("client Close did not join the active presence RPC") } } @@ -749,28 +600,21 @@ func TestEmbeddedBlockPresenceMultiNodeAndFallback(t *testing.T) { t.Fatalf("canceled presence = %v, want context.Canceled", err) } - // A replacement with a fresh ID owns the same deterministic block keys, but - // the old connection must be removed from the batcher's owner cache. + // A replacement with a fresh ID owns the same deterministic block keys. The + // next Range must route its grouped presence exchange to the replacement node. oldOwnerKey := cache.MakeBlockKey(bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, 1) - oldOwner, err := fixture.embeddedClient.Coordinator().GetNodeForKey(oldOwnerKey) + oldOwner, err := coord.GetNodeForKey(oldOwnerKey) if err != nil || oldOwner.ID != "range-bench-1" { t.Fatalf("initial block owner = (%v,%v), want range-bench-1", oldOwner, err) } - fixture.cacheClient.peerMu.Lock() - oldPeer := fixture.cacheClient.peers[oldOwner.ID] - stalePeer := fixture.cacheClient.peers["stale-owner"] - fixture.cacheClient.peerMu.Unlock() - if oldPeer == nil { - t.Fatal("multi-node probes did not cache the old owner's connection") - } fixture.closeNodes[1]() waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0") - replacement, closeReplacement := startEmbeddedBlockRangeReplacementNode(t, fixture, "range-bench-replacement") + replacement := startEmbeddedBlockRangeReplacementNode(t, fixture, "range-bench-replacement") waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0", "range-bench-replacement") waitForMatchingEmbeddedEpoch(t, fixture.embeddedClient, replacement) - newOwner, err := fixture.embeddedClient.Coordinator().GetNodeForKey(oldOwnerKey) + newOwner, err := coord.GetNodeForKey(oldOwnerKey) if err != nil || newOwner.ID != "range-bench-replacement" { t.Fatalf("replacement block owner = (%v,%v), want range-bench-replacement", newOwner, err) } @@ -783,70 +627,9 @@ func TestEmbeddedBlockPresenceMultiNodeAndFallback(t *testing.T) { if got := fixture.stats.presenceRequests.Load(); got != 1 { t.Fatalf("replacement Range made %d presence attempts, want 1", got) } - - fixture.cacheClient.peerMu.Lock() - _, oldRetained := fixture.cacheClient.peers[oldOwner.ID] - _, staleRetained := fixture.cacheClient.peers["stale-owner"] - replacementPeer := fixture.cacheClient.peers["range-bench-replacement"] - peerCount := len(fixture.cacheClient.peers) - fixture.cacheClient.peerMu.Unlock() - if oldRetained || staleRetained || replacementPeer == nil || peerCount != 1 { - t.Fatalf("peer connections after replacement = (old=%t stale=%t replacement=%v count=%d), want only new owner", oldRetained, staleRetained, replacementPeer, peerCount) - } - if oldPeer.conn.GetState() != connectivity.Shutdown { - t.Fatalf("retired owner connection state = %v, want shutdown", oldPeer.conn.GetState()) - } - if stalePeer != nil && stalePeer.conn.GetState() != connectivity.Shutdown { - t.Fatalf("stale owner connection state = %v, want shutdown", stalePeer.conn.GetState()) - } - - // Rejoin the same active node ID with the same token count but a new address. - // OCache's epoch is based on membership/state/token count, so the final epoch - // is unchanged even though the peer connection above points at the old address. - addressOnlyEpoch := coord.GetEpoch() - oldAddress := newOwner.ListenAddress - closeReplacement() - waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0") - replacement, closeReplacement = startEmbeddedBlockRangeReplacementNode(t, fixture, "range-bench-replacement") - defer closeReplacement() - waitForEmbeddedActiveNodeIDs(t, fixture.embeddedClient, "range-bench-0", "range-bench-replacement") - waitForMatchingEmbeddedEpoch(t, fixture.embeddedClient, replacement) - if got := coord.GetEpoch(); got != addressOnlyEpoch { - t.Fatalf("ring epoch after same-ID address change = %d, want unchanged %d", got, addressOnlyEpoch) - } - updatedOwner, err := coord.GetNodeForKey(oldOwnerKey) - if err != nil || updatedOwner.ID != "range-bench-replacement" || updatedOwner.ListenAddress == oldAddress { - t.Fatalf("same-ID owner address = (%v,%v), want %q changed", updatedOwner, err, oldAddress) - } - for _, index := range []int64{2, 3} { - key := cache.MakeBlockKey(bucket, fixture.key, etag, embeddedBlockRangeBenchmarkBlockSize, index) - owner, err := coord.GetNodeForKey(key) - if err != nil || owner.ID != coord.GetLocalNodeID() { - t.Fatalf("block %d owner = (%v,%v), want local node %q", index, owner, err, coord.GetLocalNodeID()) - } - } - - // This >1-block Range covers only local blocks, so the replacement peer is - // absent from the page. Reconciliation must still retire its old connection. - fixture.stats.reset() - const rangeStart = int64(2 * embeddedBlockRangeBenchmarkBlockSize) - const rangeEnd = int64(3*embeddedBlockRangeBenchmarkBlockSize + 127) - assertEmbeddedRangeBytes(t, fixture.client, fixture.gateway.URL, fixture.key, fixture.body, rangeStart, rangeEnd, fixture.gatewaySigner, fixture.accessKey, fixture.secretKey) - if got := fixture.stats.presenceRequests.Load(); got != 0 { - t.Fatalf("local-only Range made %d remote presence exchanges, want 0", got) - } - fixture.cacheClient.peerMu.Lock() - _, staleAddressRetained := fixture.cacheClient.peers["range-bench-replacement"] - fixture.cacheClient.peerMu.Unlock() - if staleAddressRetained { - t.Fatal("address-only ring change retained the unused old peer connection") - } - if got := replacementPeer.conn.GetState(); got != connectivity.Shutdown { - t.Fatalf("unused old-address connection state = %v, want shutdown", got) - } } -func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBlockRangeBenchmarkFixture, nodeID string) (*embedded.Client, func()) { +func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBlockRangeBenchmarkFixture, nodeID string) *embedded.Client { tb.Helper() diskPath := tb.TempDir() writeEmbeddedBlockRangeRingTokens(tb, diskPath, 1) @@ -874,8 +657,7 @@ func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBloc } presenceServer.client = client var closeOnce sync.Once - closeClient := func() { closeOnce.Do(func() { _ = client.Close() }) } - tb.Cleanup(closeClient) + tb.Cleanup(func() { closeOnce.Do(func() { _ = client.Close() }) }) if err := client.StartGRPCServer(); err != nil { tb.Fatalf("start replacement embedded node: %v", err) } @@ -884,51 +666,7 @@ func startEmbeddedBlockRangeReplacementNode(tb testing.TB, fixture *embeddedBloc if err := client.WaitReady(readyCtx); err != nil { tb.Fatalf("replacement embedded node did not become ready: %v", err) } - return client, closeClient -} - -func assertEmbeddedRangeBytes(tb testing.TB, client *http.Client, gatewayURL, key string, object []byte, start, end int64, signer *auth.RequestSigner, accessKey, secretKey string) { - tb.Helper() - req, err := http.NewRequest(http.MethodGet, gatewayURL+"/benchmark/"+key, nil) - if err != nil { - tb.Fatal(err) - } - req.Header.Set("Range", fmt.Sprintf("bytes=%d-%d", start, end)) - if signer == nil { - req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test/20260101/us-east-1/s3/aws4_request, Signature=deadbeef") - } else { - signed, err := signer.SignRequest(req.Context(), req.Method, req.URL.RequestURI(), req.Body, "", accessKey, secretKey, req.Header) - if err != nil { - tb.Fatalf("sign byte-range request: %v", err) - } - signed.RemoteAddr = req.RemoteAddr - req = signed - } - resp, err := client.Do(req) - if err != nil { - tb.Fatalf("byte-range request: %v", err) - } - body, readErr := io.ReadAll(resp.Body) - closeErr := resp.Body.Close() - if readErr != nil { - tb.Fatalf("read byte-range response: %v", readErr) - } - if closeErr != nil { - tb.Fatalf("close byte-range response: %v", closeErr) - } - wantBody := object[start : end+1] - if resp.StatusCode != http.StatusPartialContent || resp.Header.Get(proxy.XCacheHeader) != proxy.XCacheHit || !bytes.Equal(body, wantBody) { - tb.Fatalf("byte-range response = (status=%d, cache=%q, bytes=%d, exact=%t), want exact cache 206", resp.StatusCode, resp.Header.Get(proxy.XCacheHeader), len(body), bytes.Equal(body, wantBody)) - } - if got, want := resp.Header.Get("Content-Range"), fmt.Sprintf("bytes %d-%d/%d", start, end, len(object)); got != want { - tb.Errorf("Content-Range=%q, want %q", got, want) - } - if got, want := resp.Header.Get("Content-Length"), fmt.Sprint(end-start+1); got != want { - tb.Errorf("Content-Length=%q, want %q", got, want) - } - if got := resp.Header.Get("ETag"); got != `"block-range"` { - tb.Errorf("ETag=%q, want %q", got, `"block-range"`) - } + return client } func waitForMatchingEmbeddedEpoch(tb testing.TB, first, second *embedded.Client) { @@ -1083,7 +821,45 @@ func TestEmbeddedBlockPresenceUsesClusterGRPCAuth(t *testing.T) { func assertEmbeddedRangeResponse(tb testing.TB, client *http.Client, gatewayURL, key string, object []byte, blockCount int, signer *auth.RequestSigner, accessKey, secretKey string) { tb.Helper() rangeLen := blockCount * embeddedBlockRangeBenchmarkBlockSize - assertEmbeddedRangeBytes(tb, client, gatewayURL, key, object, 0, int64(rangeLen-1), signer, accessKey, secretKey) + req, err := http.NewRequest(http.MethodGet, gatewayURL+"/benchmark/"+key, nil) + if err != nil { + tb.Fatal(err) + } + req.Header.Set("Range", fmt.Sprintf("bytes=0-%d", rangeLen-1)) + if signer == nil { + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test/20260101/us-east-1/s3/aws4_request, Signature=deadbeef") + } else { + signed, err := signer.SignRequest(req.Context(), req.Method, req.URL.RequestURI(), req.Body, "", accessKey, secretKey, req.Header) + if err != nil { + tb.Fatalf("sign range request: %v", err) + } + signed.RemoteAddr = req.RemoteAddr + req = signed + } + resp, err := client.Do(req) + if err != nil { + tb.Fatalf("range request: %v", err) + } + body, readErr := io.ReadAll(resp.Body) + closeErr := resp.Body.Close() + if readErr != nil { + tb.Fatalf("read range response: %v", readErr) + } + if closeErr != nil { + tb.Fatalf("close range response: %v", closeErr) + } + if resp.StatusCode != http.StatusPartialContent || resp.Header.Get(proxy.XCacheHeader) != proxy.XCacheHit || !bytes.Equal(body, object[:rangeLen]) { + tb.Fatalf("range response = (status=%d, cache=%q, bytes=%d, exact=%t), want exact cache 206", resp.StatusCode, resp.Header.Get(proxy.XCacheHeader), len(body), bytes.Equal(body, object[:rangeLen])) + } + if got, want := resp.Header.Get("Content-Range"), fmt.Sprintf("bytes 0-%d/%d", rangeLen-1, len(object)); got != want { + tb.Errorf("Content-Range=%q, want %q", got, want) + } + if got, want := resp.Header.Get("Content-Length"), fmt.Sprint(rangeLen); got != want { + tb.Errorf("Content-Length=%q, want %q", got, want) + } + if got := resp.Header.Get("ETag"); got != `"block-range"` { + tb.Errorf("ETag=%q, want %q", got, `"block-range"`) + } } func TestBlockPresencePeerCancellationIsFatalUnlessSiblingCanceledGroup(t *testing.T) {