benbjohnson / benbjohnson/litestream

replica: define or remove the public EnforceRetention API after multi-level compaction

Open
#1,487 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.