Add pre-flip catalog backup for reshard operations#939
Merged
Conversation
Before a reshard mutates a tenant's DuckLake metadata catalog, dump the SOURCE catalog to the org's own S3 data bucket under a reserved _reshard_catalog_backups/ prefix so recovery is a one-command pg_restore. This is the "safety net" layer C: the artifact survives even if both live copies are damaged, and covers all directions. - New backup_catalog step in the reshard runner, after drain + pause-compaction and before the flip, for every direction. - Gate by direction: destructive cnpg->external HARD-fails the op before the flip on a backup failure; non-destructive directions (source left intact) log a warning and continue. - pg_dump --format=custom --no-owner --schema=public; password via PGPASSWORD (never argv). Upload assumes the org's own IAM role via the existing STS broker (AssumeRoleFunc injected from multitenant.go to avoid the import cycle). No worker required. - Record the artifact s3:// URI on the op row (new backup_s3_uri column, migration 000021) + op log, with the exact pg_restore recovery command. - Retention: tagged/prefixed objects expire via an S3 lifecycle rule (30d suggested); backups kept on success, no in-app GC. - postgresql-client-18 (PGDG repo) added to both Dockerfile and Dockerfile.controlplane so pg_dump/pg_restore ship in the CP image. - Tests: runner unit tests (backup-before-flip, destructive hard-fail, non-destructive continue), migration column assert, ext->cnpg e2e backup assertion. Docs: resharding.md + CLAUDE.md.
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: neutral or increased No coverage-reduction warnings detected. |
benben
added a commit
that referenced
this pull request
Jul 13, 2026
) #939 added migration 000021 (backup_s3_uri on duckgres_reshard_operations) and updated TestConfigStoreRunsVersionedSQLMigrations, but missed the downgrade round-trip test: TestConfigStoreSQLMigrationsUpgradeVersion8Schema still deleted only versions 9-20 from goose_db_version and asserted the re-upgraded latest version as 20, so every configstore-integration run on main fails with "latest goose migration version = {21 true}, want 8". Add 21 to the downgrade DELETE list and assert it is re-recorded after the upgrade. No schema reversal is needed for 000021 itself: its column lives on duckgres_reshard_operations, which the downgrade block already drops.
benben
added a commit
that referenced
this pull request
Jul 13, 2026
…ck s3:PutObjectTagging) Root cause of the e2e 'pre-flip catalog backup did not complete' failure — PRE-EXISTING on main, not introduced by the pod model: PutObject with an x-amz-tagging header requires s3:PutObjectTagging, which the per-org duckling IAM roles do not grant, so the tagged upload 403s (AccessDenied on s3:PutObjectTagging). PR #939's own merged-commit e2e run failed with the identical error from the old temp-file PutObject path; the assertion never passed against the real cluster. Fix: upload untagged. Lifecycle retention keys on the reserved _reshard_catalog_backups/ prefix (already the documented suggested rule), which needs no extra permission. Unit test now pins that the upload carries NO Tagging; docs updated (resharding.md retention, CLAUDE.md).
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
1 participant
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.
What
Before a reshard mutates a tenant's DuckLake metadata catalog, dump the source catalog to the org's own S3 data bucket under a reserved
_reshard_catalog_backups/prefix so recovery is a one-commandpg_restore. This is the safety net (layer C): it survives even if both live copies are damaged, and it covers all directions. Only the Postgres catalog metadata is ever at risk in a reshard (the S3 parquet data is never touched) — this backs up exactly that. Would have turned a recent multi-hour, RDS-PITR data-recovery into a one-liner.How
backup_catalogrunner step (controlplane/provisioner/reshard_runner.go), placed after drain + pause-compaction and BEFORE the flip, for every direction.pg_dump --format=custom --no-owner --schema=public(restorable with stockpg_restore); the DB password goes viaPGPASSWORDenv, never argv. Source DSN is the same the copy step uses (external →sslmode=require, cnpg →sslmode=disablevia the pooler).s3://<org-data-bucket>/_reshard_catalog_backups/op-<id>-<UTC-ts>.dump. Bucket + region come from the duckling CR status.AssumeRoleFuncfrommultitenant.go(the STS broker lives in thecontrolplanepackage; the func indirection avoids the import cycle). New impl:controlplane/provisioner/catalog_backup.go.backup_s3_uricolumn (goose migration000021) + the op log, with the exactpg_restorerecovery command.postgresql-client-18(PGDG apt repo, pinned to PG 18 to match the cnpg shard major) added to bothDockerfileandDockerfile.controlplane. The final CP image isdebian:bookworm-slim(not distroless), and mw-dev/e2e runs the all-in-oneDockerfileimage as its control plane, so the client must live in both.duckgres-reshard-catalog-backup=1) and prefixed so a single S3 lifecycle rule (30d suggested) expires them; backups are kept on success. Documented indocs/design/resharding.md.Restore
The op log carries a ready-to-run command (host/user/db name the source; password is a
<PASSWORD>placeholder):Tests
reshard_runner_test.go): backup recorded before the flip with the right dest bucket/key/role; destructive cnpg→external backup failure fails the op BEFORE any flip (source untouched, warehouse back to ready); non-destructive backup failure logs a warning and continues. FakeCatalogBackuperseam — no real S3/pg_dump in unit tests.tests/configstore/migrations_postgres_test.go): the newbackup_s3_uricolumn + version bump to 21.tests/mw-dev/e2e/harness.sh): the ext→cnpg positive path asserts the pre-flip backup completed and recorded abackup_s3_uriunder_reshard_catalog_backups/. The in-cluster Job has noawsCLI, so it asserts the recorded URI + restore command in the op log rather than an S3 list (same limitation as the tenant-isolation object-store half).Verification
go build ./...andgo build -tags kubernetes ./...both pass.go test -tags kubernetes ./controlplane/provisioner ./controlplane/admin -run TestReshard -count=1passes.bash -n tests/mw-dev/e2e/harness.shpasses.Coordination
Kept additive/localized (new step method + one line of wiring into the sequence) to minimize the merge conflict with the in-flight orphan-adopt branch that also edits the step machine.
🤖 Generated with Claude Code