diff --git a/cache/s3proxy/artifacts.go b/cache/s3proxy/artifacts.go index c2ccd52..f859b59 100644 --- a/cache/s3proxy/artifacts.go +++ b/cache/s3proxy/artifacts.go @@ -3,6 +3,7 @@ package s3proxy import ( "bytes" "context" + "strings" "github.com/minio/minio-go/v7" ) @@ -41,6 +42,16 @@ const ( // cache read needs to close the breaker. func (c *s3Cache) PutArtifact(ctx context.Context, key string, body []byte) error { bucket := c.bucketForContext(ctx) + // Under the Go backend disconnect, Go-tenant LRU artifacts are skipped + // along with the cache bytes they describe: the artifacts' only consumer + // (the web retention sweep) is paused for Go namespaces during the + // canary, and "Go namespaces produce zero S3 ops" is the cut's whole + // contract. A skipped advisory upload is a successful no-op, not an + // error — failing it would make the flusher log every pass. + if c.goBackendDisconnect && goTenantArtifactKey(key) { + backendUploadsSkipped.WithLabelValues(c.key, skipReasonGoDisconnect).Inc() + return nil + } if !c.breaker.isClosed() { logResponse(c.accessLogger, "LRU_ARTIFACT", bucket, key, errBreakerOpen) return errBreakerOpen @@ -55,6 +66,22 @@ func (c *s3Cache) PutArtifact(ctx context.Context, key string, body []byte) erro return err } +// goTenantArtifactKey reports whether an artifact key sits inside a Go-cache +// tenant namespace: the artifact segment ("lru/", and any future sibling) +// nested directly under a tool segment of "go" (keys are +// ////lru/). Keyed on the key rather +// than the request context because artifact flushes run on timers whose +// contexts carry no tenant identity. +func goTenantArtifactKey(key string) bool { + segments := strings.Split(key, "/") + for i := 1; i < len(segments); i++ { + if segments[i] == "lru" && segments[i-1] == "go" { + return true + } + } + return false +} + // PutArtifact routes to the backend selected on ctx, mirroring the cache // operations' dispatch: missing selector uses the default backend (metered by // backendFor), unknown selector refuses rather than guessing a shard. diff --git a/cache/s3proxy/s3proxy.go b/cache/s3proxy/s3proxy.go index b35d5f6..8f7dc61 100644 --- a/cache/s3proxy/s3proxy.go +++ b/cache/s3proxy/s3proxy.go @@ -59,6 +59,12 @@ type s3Cache struct { metrics Metrics v2mode bool updateTimestamps bool + // goBackendDisconnect severs Go-cache traffic from this backend + // entirely (BAZEL_REMOTE_GO_BACKEND_DISCONNECT=1): Get and Contains + // answer clean misses and Put drops the write-through before the upload + // queue, leaving the local disk cache as the only tier for Go. Canary + // toggle — flipping the env var is the whole roll-in/roll-out. + goBackendDisconnect bool // readDeadline is the overall bound on one read-path call including the // streamed body; defaults to the package-level readDeadline, overridable // via WithReadDeadline. Connection failure is bounded separately and @@ -76,6 +82,15 @@ func WithOperationObserver(observer cache.OperationObserver) Option { } } +// WithGoBackendDisconnect controls whether Go-cache requests bypass this +// backend entirely, for every entry kind and operation (see +// s3Cache.goBackendDisconnect). +func WithGoBackendDisconnect(enabled bool) Option { + return func(c *s3Cache) { + c.goBackendDisconnect = enabled + } +} + var ( cacheHits = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "bazel_remote_s3_cache_hits", @@ -89,6 +104,10 @@ var ( Name: "bazel_remote_s3_backend_lookups_skipped_total", Help: "S3 backend Get/Contains lookups skipped without dialing MinIO.", }, []string{"backend", "reason"}) + backendUploadsSkipped = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "bazel_remote_s3_backend_uploads_skipped_total", + Help: "S3 backend write-through uploads skipped by policy before enqueueing.", + }, []string{"backend", "reason"}) uploadQueueDropped = promauto.NewCounterVec(prometheus.CounterOpts{ Name: "bazel_remote_s3_upload_queue_dropped_total", Help: "Backend uploads dropped because the S3 upload queue was full.", @@ -630,6 +649,16 @@ func classifyUploadOutcome(err error) (status string, reason string) { } func (c *s3Cache) Put(ctx context.Context, kind cache.EntryKind, hash string, logicalSize int64, sizeOnDisk int64, rc io.ReadCloser) { + if c.goBackendDisconnect && isGoRequest(ctx) { + // Drop the write-through before the upload queue. Deliberately NO + // ObserveOperation outcome: the web-side accounting counts the + // dropped/error/rejected statuses as failures (see + // classifyUploadOutcome), and this skip is policy, not a failure — + // the Prometheus counter is the signal. + backendUploadsSkipped.WithLabelValues(c.key, skipReasonGoDisconnect).Inc() + _ = rc.Close() + return + } if c.uploadQueue == nil { _ = rc.Close() return @@ -689,17 +718,15 @@ func (c *s3Cache) UpdateModificationTimestamp(ctx context.Context, bucket string logResponse(c.accessLogger, "COMPOSE", bucket, object, err) } -const skipReasonGoAC = "go_ac" +const ( + skipReasonGoAC = "go_ac" + skipReasonGoDisconnect = "go_disconnect" +) -// skipGoActionCacheBackendLookup reports whether Get/Contains should skip MinIO. -// Go AC Get is dominated by never-stored ActionIDs; checking MinIO cannot hit -// and floods 404s. CAS still hydrates from MinIO. After L1 eviction, Go AC will -// miss instead of filling from L2 — acceptable because successful S3 AC fills -// are ~0.01% of this traffic. -func skipGoActionCacheBackendLookup(ctx context.Context, kind cache.EntryKind) bool { - if kind != cache.AC { - return false - } +// isGoRequest reports whether the request is Go-cache traffic, identified by +// either signal: the forwarded metrics labels carry BuildToolID "go", or the +// tenant storage prefix's last segment (the build tool by convention) is "go". +func isGoRequest(ctx context.Context) bool { if labels, ok := cache.MetricsLabelsFromContext(ctx); ok && labels.BuildToolID == "go" { return true } @@ -710,13 +737,37 @@ func skipGoActionCacheBackendLookup(ctx context.Context, kind cache.EntryKind) b return path.Base(prefix) == "go" } +// skipGoActionCacheBackendLookup reports whether Get/Contains should skip MinIO. +// Go AC Get is dominated by never-stored ActionIDs; checking MinIO cannot hit +// and floods 404s. CAS still hydrates from MinIO. After L1 eviction, Go AC will +// miss instead of filling from L2 — acceptable because successful S3 AC fills +// are ~0.01% of this traffic. +func skipGoActionCacheBackendLookup(ctx context.Context, kind cache.EntryKind) bool { + return kind == cache.AC && isGoRequest(ctx) +} + +// skipBackendLookupReason resolves whether a Get/Contains should skip MinIO, +// and under which backendLookupsSkipped reason: go_disconnect covers every +// entry kind while the disconnect toggle is on, go_ac is the always-on +// AC-only skip. Distinct reasons let dashboards tell the canary's full +// disconnect apart from the steady-state AC optimization. +func (c *s3Cache) skipBackendLookupReason(ctx context.Context, kind cache.EntryKind) (string, bool) { + if c.goBackendDisconnect && isGoRequest(ctx) { + return skipReasonGoDisconnect, true + } + if skipGoActionCacheBackendLookup(ctx, kind) { + return skipReasonGoAC, true + } + return "", false +} + func (c *s3Cache) Get(ctx context.Context, kind cache.EntryKind, hash string, _ int64) (io.ReadCloser, int64, error) { prefix, requestScopedPrefix, requirePrefix := c.prefixForContext(ctx, kind) if requirePrefix && !requestScopedPrefix { c.logMissingRequiredStoragePrefix("DOWNLOAD", kind, hash) } - if skipGoActionCacheBackendLookup(ctx, kind) { - backendLookupsSkipped.WithLabelValues(c.key, skipReasonGoAC).Inc() + if reason, skip := c.skipBackendLookupReason(ctx, kind); skip { + backendLookupsSkipped.WithLabelValues(c.key, reason).Inc() return nil, -1, nil } objectKey := c.objectKeyForPrefix(prefix, hash, kind) @@ -803,8 +854,8 @@ func (c *s3Cache) Contains(ctx context.Context, kind cache.EntryKind, hash strin if requirePrefix && !requestScopedPrefix { c.logMissingRequiredStoragePrefix("CONTAINS", kind, hash) } - if skipGoActionCacheBackendLookup(ctx, kind) { - backendLookupsSkipped.WithLabelValues(c.key, skipReasonGoAC).Inc() + if reason, skip := c.skipBackendLookupReason(ctx, kind); skip { + backendLookupsSkipped.WithLabelValues(c.key, reason).Inc() return false, -1 } objectKey := c.objectKeyForPrefix(prefix, hash, kind) diff --git a/cache/s3proxy/s3proxy_test.go b/cache/s3proxy/s3proxy_test.go index 5bd6902..ab74792 100644 --- a/cache/s3proxy/s3proxy_test.go +++ b/cache/s3proxy/s3proxy_test.go @@ -948,3 +948,278 @@ func TestGoActionCacheSkipDoesNotDialHungBackend(t *testing.T) { t.Fatal("expected hung backend to be dialed for Bazel AC / Go CAS / unscoped AC") } } + +func TestSkipBackendLookupReason(t *testing.T) { + goPrefix := cache.WithStoragePrefix(context.Background(), "prd/10/123/v0/go") + goTool := cache.WithMetricsLabels(context.Background(), cache.MetricsLabels{BuildToolID: "go"}) + bazelPrefix := cache.WithStoragePrefix(context.Background(), "prd/10/123/v0/bazel") + + cases := []struct { + name string + disconnect bool + ctx context.Context + kind cache.EntryKind + reason string + skip bool + }{ + {name: "off go prefix AC", disconnect: false, ctx: goPrefix, kind: cache.AC, reason: skipReasonGoAC, skip: true}, + {name: "off go prefix CAS", disconnect: false, ctx: goPrefix, kind: cache.CAS, skip: false}, + {name: "off go label CAS", disconnect: false, ctx: goTool, kind: cache.CAS, skip: false}, + {name: "off bazel prefix AC", disconnect: false, ctx: bazelPrefix, kind: cache.AC, skip: false}, + {name: "on go prefix AC", disconnect: true, ctx: goPrefix, kind: cache.AC, reason: skipReasonGoDisconnect, skip: true}, + {name: "on go prefix CAS", disconnect: true, ctx: goPrefix, kind: cache.CAS, reason: skipReasonGoDisconnect, skip: true}, + {name: "on go label AC", disconnect: true, ctx: goTool, kind: cache.AC, reason: skipReasonGoDisconnect, skip: true}, + {name: "on go label CAS", disconnect: true, ctx: goTool, kind: cache.CAS, reason: skipReasonGoDisconnect, skip: true}, + {name: "on bazel prefix AC", disconnect: true, ctx: bazelPrefix, kind: cache.AC, skip: false}, + {name: "on bazel prefix CAS", disconnect: true, ctx: bazelPrefix, kind: cache.CAS, skip: false}, + {name: "on unscoped AC", disconnect: true, ctx: context.Background(), kind: cache.AC, skip: false}, + {name: "on unscoped CAS", disconnect: true, ctx: context.Background(), kind: cache.CAS, skip: false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := &s3Cache{goBackendDisconnect: tc.disconnect} + reason, skip := c.skipBackendLookupReason(tc.ctx, tc.kind) + if skip != tc.skip || reason != tc.reason { + t.Fatalf("skipBackendLookupReason = (%q, %v), want (%q, %v)", reason, skip, tc.reason, tc.skip) + } + }) + } +} + +// countingFakeS3Backend is fakeS3Backend with a request counter in front of +// the endpoint — the seam for proving an operation never dialed the backend. +func countingFakeS3Backend(t *testing.T, requests *atomic.Int64, buckets ...string) *s3Cache { + t.Helper() + backend := s3mem.New() + for _, bucket := range buckets { + if err := backend.CreateBucket(bucket); err != nil { + t.Fatal(err) + } + } + inner := gofakes3.New(backend).Server() + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + inner.ServeHTTP(w, r) + })) + t.Cleanup(ts.Close) + + u, err := url.Parse(ts.URL) + if err != nil { + t.Fatal(err) + } + core, err := minio.NewCore(u.Host, &minio.Options{ + Creds: credentials.NewStaticV4("KEY", "SECRET", ""), + Secure: false, + BucketLookup: minio.BucketLookupPath, + }) + if err != nil { + t.Fatal(err) + } + + return &s3Cache{ + key: backendKeyA, + mcore: core, + bucket: "default-bucket", + breaker: newBreaker("test-counting-fake-s3", nil), + objectKey: objectKeyV1, + accessLogger: stdlog.New(&bytes.Buffer{}, "", 0), + } +} + +// TestGoBackendDisconnectSkipsAllKinds pins the canary toggle's read side: +// with the disconnect on, Go traffic (either detection signal) answers clean +// misses for EVERY entry kind without dialing the backend — even for objects +// the backend demonstrably holds — while non-Go traffic is untouched. +func TestGoBackendDisconnectSkipsAllKinds(t *testing.T) { + hash := "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" + var requests atomic.Int64 + c := countingFakeS3Backend(t, &requests, "default-bucket") + c.key = "go-disconnect-lookups" + c.goBackendDisconnect = true + + goPrefix := "prd/10/123/v0/go" + bazelPrefix := "prd/10/123/v0/bazel" + ctxGo := cache.WithStoragePrefix(context.Background(), goPrefix) + ctxGoTool := cache.WithMetricsLabels(context.Background(), cache.MetricsLabels{BuildToolID: "go"}) + ctxBazel := cache.WithStoragePrefix(context.Background(), bazelPrefix) + + seedS3Object(t, c, goPrefix, cache.AC, hash, "goac") + seedS3Object(t, c, goPrefix, cache.CAS, hash, "gocas") + seedS3Object(t, c, bazelPrefix, cache.AC, hash, "bzlac") + seedS3Object(t, c, bazelPrefix, cache.CAS, hash, "bzlcas") + + seeded := requests.Load() + disconnectSkipsBefore := testutil.ToFloat64(backendLookupsSkipped.WithLabelValues(c.key, skipReasonGoDisconnect)) + goACSkipsBefore := testutil.ToFloat64(backendLookupsSkipped.WithLabelValues(c.key, skipReasonGoAC)) + + for _, signal := range []struct { + name string + ctx context.Context + }{ + {"go prefix", ctxGo}, + {"go build tool label", ctxGoTool}, + } { + for _, kind := range []cache.EntryKind{cache.AC, cache.CAS} { + rc, size, err := c.Get(signal.ctx, kind, hash, -1) + if rc != nil || size != -1 || err != nil { + if rc != nil { + _ = rc.Close() + } + t.Fatalf("%s %s Get = (%v, %d, %v), want skip miss", signal.name, kind, rc, size, err) + } + if exists, size := c.Contains(signal.ctx, kind, hash, -1); exists || size != -1 { + t.Fatalf("%s %s Contains = (%v, %d), want skip miss", signal.name, kind, exists, size) + } + } + } + + if got := requests.Load() - seeded; got != 0 { + t.Fatalf("backend dialed %d times for disconnected Go traffic, want 0", got) + } + if got := testutil.ToFloat64(backendLookupsSkipped.WithLabelValues(c.key, skipReasonGoDisconnect)) - disconnectSkipsBefore; got != 8 { + t.Fatalf("backendLookupsSkipped{reason=go_disconnect} delta = %v, want 8", got) + } + // With the toggle on, Go AC skips count under go_disconnect, not go_ac, + // so dashboards can tell the canary apart from the steady-state skip. + if got := testutil.ToFloat64(backendLookupsSkipped.WithLabelValues(c.key, skipReasonGoAC)) - goACSkipsBefore; got != 0 { + t.Fatalf("backendLookupsSkipped{reason=go_ac} delta = %v, want 0", got) + } + + for _, kind := range []cache.EntryKind{cache.AC, cache.CAS} { + rc, _, err := c.Get(ctxBazel, kind, hash, -1) + if err != nil || rc == nil { + t.Fatalf("bazel %s Get = (%v, %v), want hit", kind, rc, err) + } + _ = rc.Close() + if exists, _ := c.Contains(ctxBazel, kind, hash, -1); !exists { + t.Fatalf("bazel %s Contains = false, want true", kind) + } + } + if got := requests.Load() - seeded; got == 0 { + t.Fatal("expected non-Go traffic to dial the backend with the disconnect on") + } +} + +// TestGoBackendDisconnectPutSkipsEnqueue pins the canary toggle's write +// side: with the disconnect on, a Go Put closes the payload and returns +// without enqueueing, counted on backendUploadsSkipped and — critically — +// with NO operation outcome (dropped/error/rejected statuses are consumed +// by the web-side accounting as failures). +func TestGoBackendDisconnectPutSkipsEnqueue(t *testing.T) { + hash := "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" + uploadQueue := make(chan backendproxy.UploadReq, 2) + observer := &recordingObserver{} + c := &s3Cache{ + key: "go-disconnect-put", + uploadQueue: uploadQueue, + observer: observer, + goBackendDisconnect: true, + } + + skipsBefore := testutil.ToFloat64(backendUploadsSkipped.WithLabelValues(c.key, skipReasonGoDisconnect)) + + ctxGoPrefix := cache.WithStoragePrefix(context.Background(), "prd/10/123/v0/go") + ctxGoTool := cache.WithMetricsLabels(context.Background(), cache.MetricsLabels{BuildToolID: "go"}) + + for _, tc := range []struct { + name string + ctx context.Context + kind cache.EntryKind + }{ + {"go prefix CAS", ctxGoPrefix, cache.CAS}, + {"go prefix AC", ctxGoPrefix, cache.AC}, + {"go build tool label CAS", ctxGoTool, cache.CAS}, + {"go build tool label AC", ctxGoTool, cache.AC}, + } { + rc := &closeRecorder{Reader: strings.NewReader("blob")} + c.Put(tc.ctx, tc.kind, hash, 4, 4, rc) + if !rc.closed { + t.Fatalf("%s: expected skipped Put to close the reader", tc.name) + } + select { + case <-uploadQueue: + t.Fatalf("%s: skipped Put reached the upload queue", tc.name) + default: + } + } + + if got := testutil.ToFloat64(backendUploadsSkipped.WithLabelValues(c.key, skipReasonGoDisconnect)) - skipsBefore; got != 4 { + t.Fatalf("backendUploadsSkipped{reason=go_disconnect} delta = %v, want 4", got) + } + if len(observer.outcomes) != 0 { + t.Fatalf("observer outcomes = %+v, want none for policy skips", observer.outcomes) + } + + // Non-Go traffic still write-throughs with the disconnect on. + c.Put(cache.WithStoragePrefix(context.Background(), "prd/10/123/v0/bazel"), cache.CAS, hash, 4, 4, + io.NopCloser(strings.NewReader("blob"))) + select { + case item := <-uploadQueue: + _ = item.Rc.Close() + default: + t.Fatal("expected non-Go Put to enqueue with the disconnect on") + } +} + +// TestGoBackendDisconnectOffGoPutStillEnqueues pins the toggle-off contract: +// without the disconnect, Go traffic write-throughs exactly as before, for +// both entry kinds. +func TestGoBackendDisconnectOffGoPutStillEnqueues(t *testing.T) { + hash := "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" + uploadQueue := make(chan backendproxy.UploadReq, 1) + c := &s3Cache{uploadQueue: uploadQueue} + + ctxGo := cache.WithStoragePrefix(context.Background(), "prd/10/123/v0/go") + ctxGoTool := cache.WithMetricsLabels(context.Background(), cache.MetricsLabels{BuildToolID: "go"}) + + for _, tc := range []struct { + name string + ctx context.Context + kind cache.EntryKind + }{ + {"go prefix CAS", ctxGo, cache.CAS}, + {"go prefix AC", ctxGo, cache.AC}, + {"go build tool label CAS", ctxGoTool, cache.CAS}, + } { + c.Put(tc.ctx, tc.kind, hash, 4, 4, io.NopCloser(strings.NewReader("blob"))) + select { + case item := <-uploadQueue: + _ = item.Rc.Close() + default: + t.Fatalf("%s: toggle-off Go Put did not enqueue", tc.name) + } + } +} + +// Go-tenant LRU artifacts are part of the disconnect's "zero S3 ops" +// contract: skipped as a successful no-op (an error would make the flusher +// log every pass), counted on the skip counter. Key-based, because artifact +// flushes run on timers whose contexts carry no tenant identity. +func TestGoBackendDisconnectSkipsGoTenantArtifacts(t *testing.T) { + c := &s3Cache{key: "go-disconnect-artifacts", goBackendDisconnect: true} + + skipsBefore := testutil.ToFloat64(backendUploadsSkipped.WithLabelValues(c.key, skipReasonGoDisconnect)) + if err := c.PutArtifact(context.Background(), "prd/10/123/go/lru/00000001-x.jsonl", []byte("{}")); err != nil { + t.Fatalf("skipped go artifact must be a successful no-op, got %v", err) + } + if got := testutil.ToFloat64(backendUploadsSkipped.WithLabelValues(c.key, skipReasonGoDisconnect)) - skipsBefore; got != 1 { + t.Fatalf("expected 1 skipped artifact upload, got %v", got) + } +} + +func TestGoTenantArtifactKey(t *testing.T) { + cases := map[string]bool{ + "prd/10/123/go/lru/00000001-x.jsonl": true, + "staging/42/9/v0/go/lru/x.jsonl": true, + "go/lru/x.jsonl": true, + "prd/10/123/bazel/lru/00000001-x.jsonl": false, + "prd/10/123/go/cas.v2/ab/abcd": false, + "lru/x.jsonl": false, + "prd/10/go": false, + } + for key, want := range cases { + if got := goTenantArtifactKey(key); got != want { + t.Errorf("goTenantArtifactKey(%q) = %v, want %v", key, got, want) + } + } +} diff --git a/config/proxy.go b/config/proxy.go index d532ffb..a9576ea 100644 --- a/config/proxy.go +++ b/config/proxy.go @@ -6,6 +6,7 @@ import ( "crypto/x509" "encoding/base64" "fmt" + "log" "net/http" "os" "syscall" @@ -160,6 +161,15 @@ func (c *Config) setProxy() error { } if c.S3CloudStorage != nil { + // Canary toggle, read once at construction and threaded to every + // backend so it covers single-backend and map mode alike: Go-cache + // traffic bypasses the S3 backend entirely (reads answer clean + // misses, write-throughs are dropped). + goBackendDisconnect := os.Getenv("BAZEL_REMOTE_GO_BACKEND_DISCONNECT") == "1" + if goBackendDisconnect { + log.Println("Go-cache S3 backend disconnect enabled (BAZEL_REMOTE_GO_BACKEND_DISCONNECT=1): Go traffic will neither read from nor write to the S3 backend") + } + // Multi-backend mode: an allowlisted selector → backend map, one // s3proxy backend (own minio client, transport, upload queue) per // entry, routed per-request from the validated gRPC metadata @@ -187,7 +197,8 @@ func (c *Config) setProxy() error { c.S3CloudStorage.ConnRecycleInterval, c.StorageMode, c.AccessLogger, c.ErrorLogger, numUploaders, maxQueuedUploads, s3proxy.PrometheusMetrics(), - s3proxy.WithReadDeadline(c.S3CloudStorage.ReadTimeout)) + s3proxy.WithReadDeadline(c.S3CloudStorage.ReadTimeout), + s3proxy.WithGoBackendDisconnect(goBackendDisconnect)) if err != nil { return err } @@ -220,7 +231,8 @@ func (c *Config) setProxy() error { c.S3CloudStorage.ConnRecycleInterval, c.StorageMode, c.AccessLogger, c.ErrorLogger, c.NumUploaders, c.MaxQueuedUploads, s3proxy.PrometheusMetrics(), - s3proxy.WithReadDeadline(c.S3CloudStorage.ReadTimeout)) + s3proxy.WithReadDeadline(c.S3CloudStorage.ReadTimeout), + s3proxy.WithGoBackendDisconnect(goBackendDisconnect)) return nil }