Skip to content

BLA-4006 Route Bazel cache by request storage prefix#1

Open
shreyas-blacksmith wants to merge 5 commits into
mainfrom
shreyas/bla-4006-bazel-cache-prefixing
Open

BLA-4006 Route Bazel cache by request storage prefix#1
shreyas-blacksmith wants to merge 5 commits into
mainfrom
shreyas/bla-4006-bazel-cache-prefixing

Conversation

@shreyas-blacksmith
Copy link
Copy Markdown
Contributor

@shreyas-blacksmith shreyas-blacksmith commented May 29, 2026

Summary

  • add a request-scoped storage prefix context API for Blacksmith-owned bazel-remote integrations
  • route Action Cache and CAS S3 Put/Get/Contains plus async uploads through the request-scoped prefix when FA attaches one
  • scope the local disk AC/CAS index/path by a stable hash of the request prefix so repo/generation namespaces do not hit each other locally
  • keep RAW/default callers on the configured process-wide prefix, preserving Buck2/default behavior
  • document the FA/BLA-4013 integration contract in BLACKSMITH.md

Contract

FA should attach Bazel request context with:

cache.WithStoragePrefix(ctx, "<MINIO_PREFIX>/bazel/<environment>/<region>/<model_installation_id>/<repository_id>/<generation>")
cache.WithRequiredStoragePrefix(ctx)

WithStoragePrefix affects Action Cache and CAS. AC still uses bazel-remote existing logical instance_name hash remapping, and the physical prefix adds a visible repo/generation boundary for broad deletion/cache clearing. Buck2 should not set the required-prefix marker and continues using the configured process-wide prefix.

Tests

  • env GOCACHE=/private/tmp/bazel-remote-go-cache go test ./... -count=1
  • git diff --check

Notes

  • This does not wire FA to the new API; BLA-4013 owns attaching the prefix in FA auth/interceptor code.
  • This does not change the client-facing REAPI contract or require new Bazel flags beyond the existing remote cache and instance name config.

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 29, 2026

BLA-4006

shreyas-blacksmith and others added 2 commits May 29, 2026 16:01
Co-authored-by: Codesmith Staging <codesmith-bot@users.noreply.github.com>
@shreyas-blacksmith shreyas-blacksmith changed the title BLA-4006 Route Bazel CAS by request storage prefix BLA-4006 Route Bazel cache by request storage prefix May 29, 2026
@shreyas-blacksmith
Copy link
Copy Markdown
Contributor Author

Ready for review.

Review focus:

  • cache/storage_prefix.go and cache/cache.go: request-scoped prefix API and scoped local lookup keys for AC/CAS.
  • cache/s3proxy/s3proxy.go: AC/CAS S3 object keys use the request-scoped prefix when present; RAW/default callers use the configured process prefix.
  • cache/disk/*: local disk AC/CAS entries are isolated by StoragePrefixID(prefix), including reload/eviction/FindMissing behavior.
  • utils/backendproxy/backendproxy.go: async S3 uploads capture the prefix before request context disappears.
  • BLACKSMITH.md: FA/BLA-4013 integration contract.

Expected storage shapes:

<prefix>/ac/<hh>/<sha256(action_digest + instance_name)>
<prefix>/cas.v2/<hh>/<content_digest>

Important boundary: this PR provides the bazel-remote storage primitive. FA still needs to attach cache.WithStoragePrefix after VM/instance-name authorization in BLA-4013. Buck2/default behavior is preserved when no request-scoped prefix is attached.

Validation run locally:

env GOCACHE=/private/tmp/bazel-remote-go-cache go test ./... -count=1
git diff --check

Comment thread cache/disk/disk.go Outdated
parts := strings.Split(key, "/")
if len(parts) >= 2 {
hash = parts[1]
if len(parts) >= 4 && parts[2] == "storage_prefix" {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This magical string does a lot of heavy lifting but surprisingly does not appear in the PR description, commit message, or any comment I can find. Want to hoist it to the top level and refer to the identifier, and make sure that its role is discussed in BLACKSMITH.md

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in dc64b97. I removed the literal marker from the generic cache lookup key shape (ac/<hash>/<prefixID> / cas/<hash>/<prefixID> now), hoisted the remaining local disk marker to scopedStorageRootDir, and documented in BLACKSMITH.md that storage_prefix/<sha256(prefix)>/... is local-disk-only. S3/MinIO keys still use the real request-scoped prefix directly for broad repo/generation deletion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Killed the magical string - it was a way of delineating the changes that we had made from bazel-remote, but honestly there's no good name for it and it's tough to pick a good location. So I killed it.

@shreyas-blacksmith
Copy link
Copy Markdown
Contributor Author

Follow-up pushed in 8b2d34f: removed the scoped disk marker entirely instead of documenting it. Local disk scoped entries now live directly under <sha256(prefix)>/{ac.v2,cas.v2}/..., BLACKSMITH.md no longer names the removed marker, and rg finds no remaining storage_prefix/scopedStorageRootDir references. Verified with go test ./cache ./cache/disk ./cache/s3proxy -count=1 and a full go test ./... -count=1 run.

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.

2 participants