Skip to content

perf(cache): batch range block presence checks - #235

Open
perfloop[bot] wants to merge 9 commits into
mainfrom
perfloop-pr-open-nbvk1nvcw7
Open

perfloop[bot] wants to merge 9 commits into
mainfrom
perfloop-pr-open-nbvk1nvcw7

Conversation

@perfloop

@perfloop perfloop Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Probe-first Range reads used a separate one-byte cache stream for every covering block before committing a 206. TAG now checks bounded pages grouped by cache owner. Each remote presence exchange opens and closes its own connection, so an unused connection cannot remain tied to an old owner address.

Change

The cache package adds Cache.BlockExistsBatchErr, ErrBlockPresenceUnsupported, and ErrBlockPresenceTopologyChanged. This optional capability lets the proxy request presence without extending OCache's required CacheClient interface. The embedded client checks local keys directly and sends one ordered presence exchange per remote owner in each page of up to 32 keys.

The server verifies the owner and ring epoch before and after reading the byte-zero probes. Unsupported peers and topology changes use the existing per-key fallback; routing, storage, and cancellation failures remain errors, not missing blocks. Range preflight retains the existing 32-missing-block fall-through, coalesced and validated per-block origin fills, and payload reads after the 206 commit. Full-object probes are unchanged. RFC 0001 documents the paged preflight. No external S3 endpoint or configuration is added.

Presence connections are closed after each owner exchange rather than retained across Range pages. OCache's topology epoch does not include addresses, so per-exchange ownership avoids retaining an unused connection when an owner's address changes without changing its identity.

Measured

The native benchmark primes auth and cached object data, then serves a warmed 32-block, 32 MiB Range through the regular transparent-mode handler on two embedded nodes. Metadata is local and the fixed block placement sends half of the covering blocks to a remote owner. The timer starts at handler entry and stops at the 206 header commit, before the response body is streamed; presence connection setup and close are included. This is not full-response latency. Production branch frequency and live latency remain unknown. The CPU and heap profiles include fixture setup and post-timer body writes, so they do not isolate CPU for the measured interval; no CPU or allocation improvement is claimed.

On workload 32-block warm transparent-mode Range, median metric ns/op changed from 7000386 to 2335478; paired median delta -4664908 (-66.6% of baseline; at least 19/20 confidence interval -4913757 to -4304525 from 10 pairs).

Reproduce the measurement with bash -o pipefail -c "set -euo pipefail\n\"$PERFLOOP_BUILD_OUTPUT_DIR/bench.test\" -test.run='^$' -test.bench='^BenchmarkEmbeddedBlockRangePresenceTransparent$/^32_blocks$' -test.count=1 -test.benchtime=10x | perfloop-go-bench-json 'BenchmarkEmbeddedBlockRangePresenceTransparent/32_blocks' 'ns/op'".

Validation

Embedded-node and fallback coverage exercises 2, 4, 8, and 32 blocks, sparse cap-crossing ranges, cache RPC and probe-byte counts, zero-byte misses, owner changes, coalesced fills, cancellation, transient failures, and exact 206 bytes and headers. make lint-ci, make build, make test, make test-integration, make test-race, and make test-coverage passed, along with the cmd/tag race suite.

Target CI owns S3 Compatibility Tests (s3-compat-tests default and tiered), S3 Compatibility Tests (Cluster) (s3-compat-tests-cluster default), and the Run SDK tests steps.

Target CI must run these checks.

make s3-tests (not run here): No AWS credential pair is available to start the local Tigris-backed TAG target for the required block-mode single-node and cluster profiles.


Generated by Perfloop. Measurements and checks.

@greptile-apps

greptile-apps Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[High risk] Adds optional batched block presence checks to cache layer.

The PR appears safe to merge based on the reviewed changes.

Summary

The PR batches Range block-presence checks by cache owner and falls back to per-key probes when a peer lacks the optional exchange or topology changes. Since the previous review, it replaces retained peer connections with connections scoped to each exchange, addressing the stale-address finding, and updates lifecycle tests.

Reviews (3) · Last reviewed commit: "refactor(cache): scope presence connecti..."

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 62c1573. Configure here.

Comment thread cmd/tag/block_presence.go
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
@perfloop

perfloop Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in 432ea5bffc6c. Response to this feedback.

The shared owner helper now handles a missing node as a topology change, so local preflight and peer handling avoid the panic and use the existing retry path. The nil-lookup regression test asserts that error.

Comment thread cmd/tag/block_cache_client.go Outdated
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant