Isolate S3 storage metrics from the relay

[ravarora2]

Why

The relay lists the media bucket once an hour to publish S3 storage metrics. The production bucket now exceeds the relay's one million object cap, so each sweep fails and the last good per-community values remain stale.

Raising that cap inside the serving relay would move a larger, memory-heavy scan into the process that owns live connections. A large scan could raise relay memory use, trigger restarts, and drop connections. We need to run the scan outside the serving path while preserving the existing Datadog metric names.

What

• Add buzz-admin storage-snapshot, a run-once worker that scans S3 and saves one complete accounting snapshot in PostgreSQL.
• Add inline, external, and off relay modes. The default remains the current inline mode.
• Add an opt-in Helm CronJob for the worker. The 10 million object cap exists only on this Job.
• Keep the current fleet and per-community gauges, and add snapshot age, configured cap, and cap utilization gauges.

How

Complete snapshot handoff

The worker takes a deployment-wide PostgreSQL advisory lock before it lists S3. This prevents scheduled and manual workers from overlapping, even if Kubernetes starts both.

The final database replacement runs through that same lock-owning session. If the session dies during the scan, publication fails instead of letting the stale worker overwrite its successor.

It reads S3 in pages of 1,000 objects and folds each page into physical totals, logical per-community usage, and anomaly counts. It stops before folding a page that would exceed --max-objects and emits structured progress every 100,000 objects.

The worker replaces the singleton database row only after the full scan succeeds. A listing error, malformed page, cap breach, serialization error, or database error cannot publish a partial snapshot. The last completed snapshot stays available. Each stored result includes its completion time, duration, cap, and image or source revision.

Relay behavior

BUZZ_STORAGE_METRICS=inline keeps the existing in-process sweep and its one million object default. external loads the newest completed worker snapshot on each leader metrics tick. off emits no storage-family metrics. An unknown value fails closed to off.

External mode republishes the existing metric names, so current queries remain valid. It also exports buzz_storage_sweep_age_seconds, buzz_storage_sweep_max_objects, and buzz_storage_sweep_cap_utilization. If a community disappears, changes host, or leaves the configured emission scope, the relay zeroes the old labeled series instead of leaving stale values behind.

Per-community bytes are logical referenced media usage. If two communities reference the same blob, each community receives the blob's bytes. Fleet physical totals still count each S3 object once.

Deployment boundary

The CronJob is disabled by default. When enabled, it uses concurrencyPolicy: Forbid, no retry, a configurable active deadline, bounded history, explicit CPU and memory resources, a read-only root filesystem, and no Istio sidecar by default.

The worker receives only DATABASE_URL, S3 settings, optional S3 credential references, and the code revision. It does not receive the relay private key, Git hook HMAC secret, Redis URL, or the full relay secret as environment variables. Operators may reuse the Buzz service account or select a dedicated one.

Risk

The application and chart defaults do not enable the new worker or raise the serving relay's cap. The database change is an additive deployment-global singleton table.

The main rollout risk is worker memory use because the fold retains state for distinct blob hashes and community bindings. The first production-sized run must confirm peak memory and duration before the schedule is enabled. External mode also needs a stale-snapshot monitor because the relay intentionally keeps publishing the last complete result after a failed job.

Testing

No production S3 scan was run from this PR.

After deployment, create one manual Job from the CronJob during a staffed window. Require a completed Job, exactly one new snapshot, fewer objects than the configured cap, acceptable duration and peak memory, no relay restart regression, restored Datadog points, and no OpenMetrics sample truncation.

Bigger picture

The follow-up deployment is squareup/builderbot-platform-core-infrastructure#244. After this PR merges and publishes its chart and image, that PR must pin the published artifacts, rerender, pass CI, and run the manual rollout gate above. Only then should the daily schedule and stale/failure monitors be enabled.

Related issue: N/A. No matching issue or pull request exists in block/buzz.

Generated with Codex

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is cec5c8fd9280d30f56effac701e1e19d5cfe6fea...57ede126827501a95b49d2c04b6f6de12e80010d.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 57ede126827501a95b49d2c04b6f6de12e80010d to authorize a new review.
Any previous review applies only to its recorded range.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes for one P2 correctness defect: a worker that loses its advisory-lock session can still publish, replacing its successor’s snapshot with stale contents and a fresh completion timestamp. The inline finding gives the failure sequence and a narrow exit criterion.

Reviewed head 6cb30a1f29a8e5369c7156ce74213f041706f834 against base 82656ffea080cc28cec9163ebbf7d1c6be327e43. Traced worker/fold, serialization, singleton persistence, relay modes/leadership/metric lifecycle, and deployment wiring; all delegated review lanes are complete.

Validation: existing exact-head Rust/PostgreSQL/relay CI and Helm checks passed. Independently reproduced the lost-lock overwrite on PostgreSQL 17.11 in an isolated scratch database using this head’s production upsert SQL. This was a SQL-mechanism reproduction, not an end-to-end S3/CLI run; broad local suites were not duplicated.

Nonblocking rollout gates remain as documented: pin the newly published compatible chart/image, explicitly select external relay mode at cutover, and validate production-sized memory/duration, restored metrics and OpenMetrics sample budget before enabling the schedule and monitors. I am not requesting coupling the independent worker/mode controls or redesigning the release mechanism.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

[P2] Publish through the session that owns the storage-worker lock

This acquires a different pooled connection from the detached session held by StorageAccountingLeader (lines 35–47). The CLI keeps that guard while scanning S3 but never uses its session for publication. If A’s lock backend dies while its process continues scanning, PostgreSQL releases the lock; a manually started worker B can acquire it and publish. A can then finish and unconditionally overwrite B through a healthy pooled connection, despite no longer owning the lock. concurrencyPolicy: Forbid does not fence manual workers.

I reproduced this on PostgreSQL 17.11 using the exact upsert below: live A excluded B; terminating only A’s scratch backend let B acquire and publish B-fresh (2 objects); A’s subsequent independent write replaced it with A-stale (1 object) while B still held the lock. A’s completed_at was newer, because transaction_timestamp() is evaluated during the final write. Thus the relay can publish stale totals with an apparently fresh age; a monotonic timestamp predicate would not fix this. This is stale complete content, not a partial-snapshot write.

Please make publication execute on the original lock-owning session, for example as a method on StorageAccountingLeader, so session loss fails publication closed. A liveness check followed by a write on another connection still races. Add a production-DB regression that terminates A’s owned scratch lock backend, lets B acquire/publish, then asserts A cannot publish and B’s snapshot remains. Preserve the ordinary exclusion and failed-fold tests; no lease subsystem is needed.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Re-review: the prior P2 is resolved; no blocking findings remain. Reviewed head 57ede126827501a95b49d2c04b6f6de12e80010d against the previous reviewed head 6cb30a1f29a8e5369c7156ce74213f041706f834; PR base remains 82656ffea080cc28cec9163ebbf7d1c6be327e43. Only the two fix files changed, so the earlier integrated review and rollout boundaries remain applicable.

StorageAccountingLeader::save_snapshot now executes the atomic upsert directly on its private lock-owning PgConnection. The worker moves that leader into the persistence closure and retains it while the S3 fold runs. If that physical session dies, publication fails instead of acquiring another pooled connection; the error propagates to a failed worker result. Successful fold/serialization remains a prerequisite for any save.

The added PostgreSQL regression terminates A’s actual lock backend, lets B acquire and publish, then verifies A cannot save and B’s payload and metadata remain unchanged. Existing overlap exclusion/replacement coverage remains. The independent DB re-review agrees that this closes the reported failure without a lease redesign.

Validation: GitHub Actions run 34510338856, associated with this head, is successful. The PostgreSQL job checked out synthetic merge 187f9b0f1d53d034849b71131b27b26b56677d94 (this head merged into cec5c8fd9280d30f56effac701e1e19d5cfe6fea); both changed files have identical Git blob IDs at that checkout and the reviewed head. I checked the actual PostgreSQL job log: both lost_lock_session_cannot_overwrite_successor_snapshot and complete_snapshot_replaces_atomically_and_worker_lock_excludes_overlap passed; the lane reports 380/380 passing. Relevant Rust/relay and Helm checks passed. Local final source review was re-read from immutable Git objects and restored worktree bytes were verified identical. I ran no local package suites or production S3 scan. A collaborator reported additional local mutation checks, which are not relied on here; the verified CI log is the runtime evidence.

The documented manual rollout gates still apply: pin compatible published artifacts and verify production-sized memory/duration, restored metrics, sample budget and stale/failure monitoring. These are not additional code blockers. This is a comment review, not approval.

Carl, an automated reviewer, commenting via Wes’s GitHub account. The sole P2 is resolved at 57ede12; verified re-review: #7543 (review). Dismissing the obsolete blocking review, not approving the PR.