benbjohnson / benbjohnson/litestream
replica: define or remove the public EnforceRetention API after multi-level compaction
- Dominant language
- Go
- Stars
- 14.4k
- Forks
- 414
- Avg merge
- 7d 1h
- Merged PRs (30d)
- 21
Description
## Problem
`Replica.EnforceRetention(context.Context)` is a public method whose implementation on `main` unconditionally panics:
```go
func (r *Replica) EnforceRetention(ctx context.Context) (err error) {
panic("TODO(ltx): Re-implement after multi-level compaction")
}
```
This is reachable by library callers and is also adjacent to the one-shot `replicate -enforce-retention` path. The previous single-level implementation is retained only as commented code.
## Current code path
- `replica.go:316-350`: public `Replica.EnforceRetention` panics.
- `store.go:833-853`: current retention is implemented at the Store/DB boundary by enforcing snapshot retention and cascading a TXID floor through configured non-L0 levels.
- `compactor.go:238-336`: level-aware snapshot and TXID retention primitives.
- `cmd/litestream/replicate.go:422-428`: `-once -enforce-retention` calls `Store.EnforceSnapshotRetention`, not the panicking Replica method.
## Reproduction
A caller using the public library API can trigger the panic with a configured `Replica`:
```go
var r *litestream.Replica = ...
_ = r.EnforceRetention(context.Background())
```
The method cannot be repaired by restoring the old body mechanically: the current architecture has no retention configuration on `Replica`, and retention must account for configured multi-level compaction, snapshot floors, in-flight restore plans, local-file cleanup, and the `RetentionEnabled` policy.
## Design questions
1. Should `Replica.EnforceRetention` be removed in a breaking release, deprecated, or retained as a compatibility wrapper?
2. If retained, should it delegate to a Store/DB-level API, and where should retention configuration and compaction levels come from?
3. What behavior is expected for standalone `Replica` library callers and for retention-disabled configurations?
## Proposed follow-up
After the API/ownership decision, add a focused regression test proving the chosen behavior and update the public API documentation. The implementation should preserve the existing Store/Compactor multi-level retention semantics rather than reintroducing the obsolete single-level algorithm.
This issue is intentionally a design discussion before code changes because the correct fix is not mechanically safe.
Contributor guide
Research direction
Read replica.go:316-350 alongside store.go:833-853 and compactor.go:238-336 to understand the panicking API and current multi-level retention behavior. Check cmd/litestream/replicate.go:422-428 for the existing one-shot path, then resolve the API ownership and compatibility decision. Done means a focused regression test proves the chosen behavior and the public API documentation is updated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend-api-design, databases
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100