Repository navigation
feat(backup): route a database's backups to its own target (#1085) - #1133
Merged
Merged
Conversation
Several named backup targets, with `databases = [...]` on each: those databases' files go to that target and everything else to the default. A run now has one leg per target it touches, each with its own manifest, file sidecar and inventory. Routing keys on the storage-root segment, with _schema/<db>/ and _compaction_state/<tier>/<db>/ unwrapped so a database's field schema anchors and compaction recovery state travel with its data. That is deliberately the same rule a scoped backup uses to decide what a database owns: if the two disagreed, a scoped backup of a routed database would enumerate one set of files and write them to another target. Every leg's copy happens before any leg commits, so a failure during the copy phase leaves no manifest at any destination and the listing shows nothing, exactly as before. A run index at <backup_id>/index.json on the default target, written before the first copy, closes the hole stage B2b-1 recorded: a run that died after a sidecar landed left objects that nothing enumerated. Those runs are now reported in incomplete_runs and a delete by id sweeps them. The listing fans out over every target in parallel and unions by backup id. One unreachable target no longer fails it: the target is named in unreachable_targets, the entry is marked partial_view because its counts are summed over fewer legs than the run has, and the status stays 200. A restore reads every target of the run and refuses before writing anything if one is not configured here, will not answer, or holds no manifest for the id. Restoring only the reachable part would report success over a set it did not restore, and in replace mode would delete the live files of a database whose backup bytes are on the target that is down. The instance-wide state — the SQLite database, the Iceberg catalog, an out-of-root warehouse, arc.toml — is read from the leg whose manifest asserts it rather than from whatever backup.default_target names now, so a backup taken before the default moved still restores. include_config now defaults off when ANY configured target is remote, not only when the default is: arc.toml carries every target's credentials, so a local default plus one remote routed target still means copying it puts the keys to that store inside a backup held there. A target nothing is routed to is inert: it gets a startup warning and no leg. Not a leg on purpose, because a leg is probed before anything is copied, so an unrouted target would otherwise make an unrelated store a precondition of every backup in the instance. Routing is validated at load. A database named on two targets is refused, and the overlap refusal now runs every target against primary storage, against the cold tier, and against every other target pairwise. Renaming a target makes existing multi-target backups unrestorable until the name is restored: a restore has to know which target holds which slice. A single-target backup is unaffected, which keeps stage B2b-1's promise that Manifest.Target is a label and never a lookup. Part of #1085.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Per-database backup routing: several named targets,
databases = [...]on each, every database'sfiles going to its target and everything else to the default. This is the rest of #1085 stage B
apart from the
cold_files_excludedmarker, which lands separately._schema/<db>/and_compaction_state/<tier>/<db>/unwrapped so a database's field schema anchors and compaction recovery state travel with its
data. The rule is deliberately the one a scoped backup already uses (
internal/backup/scope.go),because routing and scoping must agree or a scoped backup of a routed database would enumerate one
set of files and write them to another target. An edge-sync spoke therefore routes by the SPOKE.
and tallies. Copy every leg, then commit every leg, non-default first and the default last, so a
failure during the copy phase leaves no manifest anywhere and the listing shows nothing — exactly
the behaviour before this change.
<backup_id>/index.jsonon the default target, written before any copy. Thiscloses the hole stage B2b-1 recorded: a run that died after a sidecar landed left objects nothing
enumerated. The listing now reports those runs in
incomplete_runs, and a delete by id sweeps them.destination holding a slice. One unreachable target no longer fails the listing: it is named in
unreachable_targets, the entry is markedpartial_view, and the status stays 200. 503 only whenevery target failed.
configured here, will not answer, or holds no manifest for the id. Restoring only the reachable
part would report success over a set it did not restore, and in replace mode would delete the live
files of a database whose backup bytes are on the target that is down.
include_configdefaults off when any configured target is remote, not only when the defaultis:
arc.tomlcarries every target's credentials, so a local default plus one remote routed targetstill means copying it puts the keys to that store inside a backup.
on purpose: a leg is probed before anything is copied, so an unrouted target would otherwise make
an unrelated store a precondition of every backup, including whole-instance ones.
now runs every target against primary storage, against the cold tier, and against every other
target pairwise.
Not in this PR:
cold_files_excluded(the next one, which closes #1085); scheduling, incrementaland cross-cluster backups.
Operator-visible consequence worth stating plainly
Renaming a backup target makes existing multi-target backups unrestorable until the name is
restored. A restore has to know which target holds which slice, and the bytes genuinely are in
several places, so the names in a manifest are resolved against local configuration. Single-target
backups are unaffected: their manifests carry no target set at all, which keeps stage B2b-1's
promise that
Manifest.Targetis a label and never a lookup.Test plan
go build ./cmd/... ./internal/...gofmt -l ./internal ./cmd— emptygo vet ./internal/... ./cmd/...— emptygo test -race ./internal/backup/... ./internal/config/... ./internal/storage/... ./internal/api/... ./cmd/arc/...internal/backup/target_routing_1085_test.go;routeKeyiscross-checked row by row against
scope.ownsPathso the two rules cannot driftconfig.Load()over a table of configs, with onemutation per claimed check site
database, the Azure arm under a key prefix, a scoped run whose scope touches no routed target
while that target is stopped, an unreachable target, a run killed mid-copy, and a run restored
after
default_targetwas re-pointed. 44 assertions pass; the one inconclusive is noted below.databasesexercised at all three spellings a real deployment can use — a TOML array, a TOMLcomma string, and the environment variable — because they do not parse identically
The live run's one inconclusive: the rig cannot make the Iceberg exporter produce outside-root
warehouse files for seeded data, so that arm of the restore fix rests on its unit test rather than on
the rig, and the harness reports it as inconclusive rather than as a pass.
Part of #1085.