block / block/buzz

Git conformance probe writes non-pack objects into `packs/` on every startup, so the retention-policy cleanup it delegates to cannot be written safely

Open
#5,241 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
32.7k
Forks
4.3k
Avg merge
1d 13h
Merged PRs (30d)
253

Description

### Summary

Phase 1 of `GitObjectStore::run_conformance_probe` writes its probe objects into the `packs/` prefix, which is where authoritative git pack data lives. Those objects are never deleted, so every relay startup permanently adds `race_rounds` objects (3 by default) to the production namespace.

The probe is explicit that accumulation is intentional and that cleanup is the bucket's job, in `crates/buzz-relay/src/api/git/store.rs`:

```rust
// Cleanup pointer (immutable probe writes accumulate by design; the
// bucket's retention policy handles them, not the probe).
let _ = self.bucket.delete_object(&pointer_key).await;
```

That is a reasonable design, but it does not hold for phase 1, because phase 1 is the one probe write that is not namespaced. The retention policy the comment delegates to cannot be written safely.

### Detail

Every other probe write stays under `probe/`:

- Phase 2 pointer: `probe/pointer-{nonce}`, and it is explicitly deleted at the end of the probe.
- Phase 3: `Self::content_key("probe/inm-race", &body)`.

Phase 1 instead goes through `put_pack`:

```rust
let key = self.put_pack(&body).await?;
```

and `put_pack` is hardcoded to the production prefix:

```rust
pub async fn put_pack(&self, bytes: &[u8]) -> Result {
self.put_immutable("packs", bytes, "application/x-git-pack")
.await
}
```

So a lifecycle rule on `probe/` would cleanly handle phases 2 and 3, but there is no equivalent rule for phase 1. Any age-based or size-based rule broad enough to catch the probe litter in `packs/` is also broad enough to delete real pack data, and the keys are content-addressed SHA-256 either way, so there is nothing in the key to distinguish them.

Two things make it a little worse:

- The phase 1 bodies are `probe-sequential-{nonce}-{round}`, which are not valid git packs. `packs/` therefore accumulates objects that are not packs, and anything enumerating or validating that prefix has to tolerate them.
- Growth is proportional to restart count, not uptime. A crash-looping relay multiplies it quickly. We saw roughly 52 restarts in a row while a single-node S3 backend was shedding load under the probe's default 32-way concurrency, so one bad deploy can leave well over a hundred stray objects.

Net effect for an operator: the `packs/` prefix cannot be cleaned up automatically, and cannot be audited by prefix either, so reconciling real git content against probe residue means hand-inspecting object bodies.

### Suggested fix

Either would resolve it:

1. Give phase 1 its own namespace, e.g. write through `put_immutable("probe/pack", ...)` instead of `put_pack`, so the documented retention mitigation actually applies to all probe writes. This keeps the phase honest as long as it still exercises the same immutable create-only path.
2. Delete the phase 1 keys at the end of the probe, the way `pointer_key` already is. The keys are returned by `put_pack`, so they are available to collect.

Option 1 seems closer to the existing intent, since it preserves "probe writes accumulate, retention handles them" as a single coherent rule rather than making phase 1 an exception.

### Version

Verified on `main` at `2b873cf208bf2143bfdb77dbe34b04edcdb723a1`. `ProbeConfig::default()` is `race_width: 32, race_rounds: 3`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.