Repository navigation
Conversation
robinnsc
requested review from
LeeroyHannigan,
amrith,
c33howard,
jcshepherd,
pdf-amzn and
yesyayen
as code owners
October 13, 2026 03:25
6 of 8 tasks
This was referenced Oct 13, 2026
…t code Revision of docs/design/15-backup-restore.md after reviewing the draft against main 9f43d03 and PRs #384, #335, #336, #337, measuring the read, compression, and bulk-load paths on PostgreSQL 16 and SQLite with a 1M-item fixture, and two review rounds against the companion PRs #393, #394 and #395. The revision log at the end of the document lists each change; the ones that alter behavior: - SnapshotEngine returns the table definition together with the items from one fenced operation (TableSnapshot). The items-only signature in the draft allowed a GSI change or DeleteTable between the definition read and the data snapshot. Indexes are reported in whatever build state they had and only an index being deleted is dropped; an ACTIVE-only filter would have left a GSI mid-backfill out of the backup for good. - The snapshot is opened in the CreateBackup request so the backup represents the request time, as DynamoDB documents; backup_creation_date_time is the instant the snapshot pinned. - Per-job claims with fencing replace the single-instance lease the draft assumed; PostgreSQL and MongoDB have no such lease. The claim is an advisory lock on a dedicated catalog connection on PostgreSQL (the seed #384's restore lock already uses), the row itself on MongoDB, an in-process slot on SQLite; no row ever exists unowned. Heartbeats are a timer, staleness is measured on the database's clock, and a failed heartbeat aborts the job. The reconciler resolves a dead CREATING row at once under a fresh generation conditioned on status = CREATING and deletes its prefix only after the stale window. - DeleteBackup tombstones the row as DELETING, deletes the manifest first, and marks DELETED only when the prefix is empty; backup sync is scoped to the configured store and never adopts a CREATING, DELETING, DELETED or FAILED prefix. Failed creations and deleted backups are BackupNotFound, as on the service, and share one retention. - Restore returns CREATING from the request (table and restore_jobs row created atomically; ResourceInUseException on a name race, LimitExceededException over the job limit), spools and verifies every data file before writing any item, and records progress in a way a crash cannot split from the data: a data-database row per file on PostgreSQL (data migration 005 reserved), the same transaction per 10k-item batch on SQLite, idempotent re-application of one file on MongoDB followed by a document-count check against the manifest. Duplicate detection for unordered backups is the target's primary key, not an in-memory key set. The ACTIVE flip is conditional; DeleteTable on a restore target fails the job. - The manifest self-checksum is replaced by a sidecar over the stored bytes; the draft's canonical form depended on serde_json formatting. backup_arn is the backup's identity and source_table must agree with it. N key values are normalized before key text or key order is computed; the SQL ORDER BY recipe includes COLLATE "C" on PostgreSQL. - Index population on restore stays per backend via bulk_load; the shared restore-backfill worker the draft described exists only on MongoDB. - Format additions: item_order, per-file key_range, md5 and uncompressed_bytes, explicit nulls, three-state vector_indexes of creation specs, optional etag, dataFileS3Key as the full object key, partition-aware BackupArn as the single ARN implementation. - Filesystem store: owner-only root re-checked by every operation, owner-only rule on every descended directory including list and delete_prefix, ancestor ownership rule, objects 0600 and directories 0700 at creation, put returns object metadata; the consequence for backup pickup agents is stated. The S3 conformance suite is mandatory in CI against moto (MinIO no longer publishes images). - ListBackups paging and filtering, the restore override contract, sqlite-memory behavior, extenddb destroy leaving the store intact, a restore spool outside the store root, max_concurrent_jobs, heartbeat_interval_secs and a concrete pool-size rule are specified. - Phase 1 is rewritten as the PR series with acceptance criteria and GitHub numbers. Migration numbers are reserved up front so the PITR PR is not on the critical path. Co-authored-by: Lee Hannigan <lhnng@amazon.com>
robinnsc
force-pushed
the
docs/backup-restore-design-revised
branch
from
October 13, 2026 06:50
d843f33 to
95fec9a
Compare
This branch has not been deployed
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.
Backup/restore series (design: #396): #384 restore fidelity (merged first) · #393 store · #394 format · #395 SnapshotEngine · next: PITR surface (#335 rebased), catalog index, backup job, restore job.
What
Revision of
docs/design/15-backup-restore.mdfrom #348, written against main9f43d03, #384, and the companion PRs #393, #394, #395 as they stand after two adversarial review rounds. One commit, squashed with #348's work (Lee is co-author); supersedes #348. The document carries a revision log (items 1–14); the entries that change behavior:Snapshot semantics.
SnapshotEnginereturns the table definition together with the items from one operation (TableSnapshot). The items-only signature in the draft allowed a GSI change or a DeleteTable to commit between the definition read and the data snapshot, which #384 had already closed on PostgreSQL. The snapshot is opened in the CreateBackup request, so the backup represents the request time, as the service documents;backup_creation_date_timeis the instant the snapshot pinned. Indexes are reported in whatever build state they had and only an index being deleted is dropped; a restore builds them from the items, and an ACTIVE-only filter would have left a GSI mid-backfill out of the backup for good.Coordination. Per-job claims with fencing tokens replace the single-instance lease the draft assumed (PostgreSQL and MongoDB have none; readiness P0-7 is open). The claim is an advisory lock on a dedicated catalog connection on PostgreSQL (the seed #384's restore lock already uses), the row itself on MongoDB (born owned), an in-process slot on SQLite; no row ever exists unowned. Heartbeats are written on a timer, not per uploaded part (the store trait exposes no part boundary); staleness is measured on the database's clock on every backend; a failed heartbeat aborts the job. The reconciler resolves a dead CREATING row at once under a fresh generation, conditioned on
status = CREATING, and deletes its prefix only after the stale window.DeleteBackup. The row is tombstoned DELETING, the manifest is deleted first, and the row goes to DELETED when the prefix lists empty.
backup syncis scoped to the configured store and never adopts a CREATING, DELETING, DELETED or FAILED prefix. Failed creations and deleted backups are BackupNotFound, as on the service, and share one retention.Restore. The request creates the target table and a
restore_jobsrow atomically and returns CREATING (ResourceInUseExceptionon a name race,LimitExceededExceptionover the job limit); the job spools and verifies every data file (with.verifiedmarkers in a per-host spool) before writing any item; progress is recorded in a way a crash cannot split from the data: a data-database row per file on PostgreSQL (data migration 005 reserved), the same transaction per 10k-item batch on SQLite, idempotent re-application of one file on MongoDB followed by a document-count check against the manifest. Duplicate detection for unordered backups is the target's primary key, not an in-memory key set, so restore memory does not grow with the table.Nvalues are normalized before key text or key order is computed. The ACTIVE flip is conditional on the table still being a CREATING restore target; DeleteTable on a restore target fails the job. Index population stays per backend behindbulk_load; the shared restore-backfill worker the draft described exists only on MongoDB.Format. The manifest self-checksum is replaced by a sidecar over stored bytes.
backup_arnis the backup's identity andsource_tablemust agree with it.item_orderwith a per-filekey_range(normalizedNendpoints; none on an unordered manifest); the SQLORDER BYrecipe includesCOLLATE "C"on PostgreSQL;md5anduncompressed_bytesper file; explicit nulls; three-statevector_indexesof creation specifications; optionaletag;dataFileS3Keyas the full object key; one partition-awareBackupArn.Store. Owner-only root re-checked by every operation, the owner-only rule on every descended directory including
listanddelete_prefix, an ancestor ownership rule, 0600/0700 at creation; the consequence for backup pickup agents is stated. The S3 conformance suite is mandatory in CI against moto.Also specified: ListBackups paging and filtering; the restore override contract;
sqlite-memoryrefusal;extenddb destroyleaves the store intact andbackup purgeis scoped to this instance's accounts; a restore spool outside the store root;max_concurrent_jobsas a semaphore shared by backups and restores;heartbeat_interval_secs; a concrete pool-size rule (pool_size >= max_concurrent_jobs + 10);SnapshotExpiredcodes (PostgreSQL 40001 and 72000, MongoDB 239 and 246);SnapshotInfoas a re-export once #394 merges.Phasing is rewritten as the PR series with acceptance criteria and GitHub numbers. Migration numbers (catalog 004/005, data 005) are reserved up front so the PITR PR is off the critical path.
Why
The draft was written on 2026-09-10 against
4fd2c5f8. Since then #384 fixed two of its seven motivating defects, and a review of the draft against the code found three design-level problems (snapshot/definition atomicity, delete ordering, non-idempotent resume) and several assumptions the code does not satisfy (the lease, the backfill workers, a job queue). Two further review rounds against this revision and the companion PRs found internal contradictions (the checklist still described the pre-revision restore order; "heartbeat per part" could not be built against the store trait; "claim before row" is impossible on MongoDB; step 6 resolved rows "immediately" while step 4 forbade deleting for a stale window; an unordered-backup key set contradicted the memory formula) and undefined pieces (the restore job's row, claim and retention; where "verified" is recorded; the ACTIVE flip's condition), all closed in this revision. The implementation PRs in this series follow the revised text, so it should land with or before them.Testing done
Documentation only.
cargo test -p extenddb-storage-postgres --test doc_version_literalspasses (the two forward-looking catalog version literals carryversion-literal-okmarkers). Measurements cited in the text (read, compression, bulk-load throughput; vacuum and WAL growth under a held snapshot) were taken on a 16-core host with PostgreSQL 16 and SQLite against a 1M-item fixture; each number carries its host, item size, transaction granularity, and tool caveats in the document.Not changed here
Checklist
cargo test --workspace)cargo fmt --check)cargo clippy -- -W clippy::pedantic)Storagetrait, auth model, on-disk format, or public CLI surface, an RFC has been accepted or is linked below. Otherwise, an ADR captures the decision (link below).ADR / RFC: this document is the design record for the series.
Breaking changes
None (documentation).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0 and I agree to the Developer Certificate of Origin (DCO). See CONTRIBUTING.md for details.