amadan
litestream: replica keys from the full path, not the basename
Tree discovery turns a latent bug into a certainty. The replica URL was derived from filepath.Base, and basenames are not unique: every SQLite-backed tenant here names its database registry.db or data.db, and the box's own control DB is /data/registry.db. Three such databases all resolved to s3://bucket/prefix/registry.db and two litestream replicators writing one prefix interleave their WAL into a replica that restores as neither database. Before treeDBs the collision needed an unusual layout; after it, every tenant tree contains a registry.db. Keys are now the full path with the leading separator dropped — data/registry.db, var/lib/carlos-app/home.kass.training/registry.db — which is unique by construction and still legible to whoever is reading the bucket during a restore, at speed, trying to work out which database a key holds. This is a one-time freedom: no box has ever run a config generated by this verb (the flagship's is hand-written, tito's comes from the legacy `titogo ops litestream`), so there is no live lineage to strand. There will be after the first cutover, and the comment says so.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 files changed,
+65
−1
internal/ops/litestream.go+27 −1internal/ops/ops_test.go+38 −0
diff --git a/internal/ops/litestream.go b/internal/ops/litestream.go| index ae4a60c78..e246f3e1f 100644 |
| --- a/internal/ops/litestream.go |
| +++ b/internal/ops/litestream.go |
| @@ -137,7 +137,7 @@ func LitestreamConfig(dbPaths []string, bucket, prefix, region string) string { |
| for _, p := range sorted { |
| fmt.Fprintf(&b, " - path: %s\n", p) |
| b.WriteString(" replicas:\n") |
| - fmt.Fprintf(&b, " - url: s3://%s/%s/%s\n", bucket, prefix, filepath.Base(p)) |
| + fmt.Fprintf(&b, " - url: s3://%s/%s/%s\n", bucket, prefix, replicaKey(p)) |
| if region != "" { |
| fmt.Fprintf(&b, " region: %s\n", region) |
| } |
| @@ -145,6 +145,32 @@ func LitestreamConfig(dbPaths []string, bucket, prefix, region string) string { |
| return b.String() |
| } |
| +// replicaKey is a database's place in the replica bucket: its full path with |
| +// the leading separator dropped, so /data/registry.db becomes |
| +// data/registry.db. |
| +// |
| +// It is the full path and not the basename because the basename is NOT |
| +// unique, and the consequence of pretending otherwise is severe. Every |
| +// SQLite-backed tenant on this fleet calls its database registry.db or |
| +// data.db, and the box's own control DB is /data/registry.db; a basename key |
| +// collapses all of them onto one S3 prefix, where two litestream replicators |
| +// interleave their WAL into a replica that restores as neither database. |
| +// Tree discovery (treeDBs) turns that from a curiosity into the normal case — |
| +// every tenant tree contains a registry.db. |
| +// |
| +// Keeping the path readable rather than hashing it is deliberate: the person |
| +// reading this bucket is doing a restore, probably at speed, and needs to see |
| +// which database each key is. |
| +// |
| +// Changing this scheme moves every database to a new replica lineage, so it |
| +// is a one-time freedom: no box has ever run a config generated by this verb |
| +// (the flagship's is hand-written, tito's comes from the legacy `titogo ops |
| +// litestream`), so there is no live lineage to break. There will be after the |
| +// first cutover — treat it as fixed then. |
| +func replicaKey(dbPath string) string { |
| + return strings.TrimPrefix(filepath.Clean(dbPath), "/") |
| +} |
| + |
| // ParseLitestreamDBs recovers the database paths from a generated config. |
| // It reads only the exact shape LitestreamConfig writes (the round-trip is |
| // pinned by TestConfigRoundTrip) — it is not a YAML parser. |
diff --git a/internal/ops/ops_test.go b/internal/ops/ops_test.go| index 7aa68fc33..97cea384b 100644 |
| --- a/internal/ops/ops_test.go |
| +++ b/internal/ops/ops_test.go |
| @@ -668,3 +668,41 @@ func TestCoverageShortfall(t *testing.T) { |
| t.Fatalf("a fresh box reported a shortfall: %v", missing) |
| } |
| } |
| + |
| +// Two databases with the same basename must not replicate to the same place. |
| +// |
| +// Every SQLite-backed tenant on the fleet names its database registry.db or |
| +// data.db; the box's own registry is /data/registry.db. Deriving the replica |
| +// URL from filepath.Base collapses all of them onto one S3 key, and two |
| +// litestream replicators writing one prefix interleave their WAL into a |
| +// replica that restores as neither database. Tree discovery makes the |
| +// collision the normal case rather than a curiosity: every tenant tree has a |
| +// registry.db in it. |
| +func TestReplicaURLsAreUniquePerPath(t *testing.T) { |
| + cfg := LitestreamConfig([]string{ |
| + "/data/registry.db", |
| + "/var/lib/carlos-app/home.kass.training/registry.db", |
| + "/var/lib/carlos-app/other.example.com/registry.db", |
| + }, "bucket-x", "fleet", "eu-west-1") |
| + |
| + var urls []string |
| + for _, line := range strings.Split(cfg, "\n") { |
| + if u, ok := strings.CutPrefix(strings.TrimSpace(line), "- url: "); ok { |
| + urls = append(urls, u) |
| + } |
| + } |
| + if len(urls) != 3 { |
| + t.Fatalf("expected 3 replica urls, got %d:\n%s", len(urls), cfg) |
| + } |
| + seen := map[string]bool{} |
| + for _, u := range urls { |
| + if seen[u] { |
| + t.Fatalf("two databases share the replica url %q — their WALs would interleave:\n%s", u, cfg) |
| + } |
| + seen[u] = true |
| + } |
| + // And the key should still be legible to whoever is doing the restore. |
| + if !strings.Contains(cfg, "home.kass.training") { |
| + t.Fatalf("replica keys lost the path that identifies them:\n%s", cfg) |
| + } |
| +} |