Skip to content

fix(postgres): restore must not report ACTIVE before the data copy co… - #246

Merged
amrith merged 1 commit into
mainfrom
fix/restore-active-implies-copy-complete
Aug 10, 2026
Merged

fix(postgres): restore must not report ACTIVE before the data copy co…#246
amrith merged 1 commit into
mainfrom
fix/restore-active-implies-copy-complete

Conversation

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator

PR body: fix/restore-active-implies-copy-complete

What

Fixes a race in the Postgres restore path where a restored table could report ACTIVE in DescribeTable before its data copy finished. create_table_impl gains a defer_active flag: the restore path passes true, writing the table row CREATING with no scheduled transition, so the explicit ACTIVE update at the end of the row copy is now the only status flip. The public TableEngine::create_table path passes false and is unchanged.

Why

DynamoDB's contract is that the first ACTIVE observation on a restore target implies the restored data is fully present. Our restore created the target through the normal create-table flow, which schedules the CREATING to ACTIVE transition on the creation-delay timer. Copying a large backup outlasts that timer, so a client that waits-for-ACTIVE can observe an empty or partial table. Reproduced live on baseline: ACTIVE at 515/40000 items. This is the same defect class the MongoDB backend in #207 fixed on its side, and it is what makes that PR's dual-target restore test red on the Postgres runner.

Closes # (no standing issue; surfaced by the restore-completeness test in #207's CI)

Testing done

Live against Postgres 16, TLS, SigV4 auth:

  • New integration test restore_active_completeness::restored_table_has_all_items_when_first_active: an observer task spawns before the restore, polls DescribeTable, and counts items via a paginated Select=COUNT scan at the first ACTIVE observation. Transient describe errors are retried; scan errors fail the test rather than truncating the count.
  • Negative control on baseline (fix reverted): FAILED with ACTIVE at 515/40000 items (112s), confirming the test discriminates.
  • With the fix: the test passes (129s), and the full Rust integration suite with --test-threads=1 is 418/421 passed, 0 filtered out. The 3 failures are capacity_throttling tests that require settings set throttling_enabled true, which my ad-hoc harness omitted; CI sets it in integration.yml.
  • cargo fmt --check exit 0, cargo clippy clean on the touched crate.

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, 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: n/a (internal bug fix; create_table_impl is pub(crate), no contract surface changed)


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.

…mpletes

DescribeTable's contract is that the first ACTIVE observation on a restore
target implies the restored data is fully present. The Postgres restore
path created the target through the normal create-table flow, which
schedules a CREATING->ACTIVE transition on the creation-delay timer. The
row-by-row copy of a large backup outlasts that timer, so a client that
waits-for-ACTIVE can observe an empty or partial table (reproduced live:
ACTIVE at 515/40000 items on baseline).

Fix: create_table_impl gains a defer_active flag. The restore path passes
true, writing the row CREATING with no scheduled transition; the existing
explicit ACTIVE update at the end of the copy is now the only flip. The
public TableEngine::create_table path passes false and is unchanged.

Verification (live, Postgres 16 at 127.0.0.1:5433):
- New test restore_active_completeness: observer spawns before restore,
  polls DescribeTable, counts items via paginated Select=COUNT scan at
  first ACTIVE. Transient describe errors are retried, and scan errors
  fail the test rather than truncating the count (avoids the oracle bug
  that makes the equivalent upstream test flaky).
- Negative control on baseline: FAILED with ACTIVE at 515/40000 (112s).
- With fix: single test PASS (129s); full integration suite
  --test-threads=1: 418/421 passed, 0 filtered. The 3 reds are
  capacity_throttling tests requiring 'settings set throttling_enabled
  true' which the ad-hoc harness omitted; CI sets it (integration.yml).
@amrith
amrith added this pull request to the merge queue Aug 10, 2026
@amrith

amrith commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

approved and merging when ready

Merged via the queue into main with commit cf5d497 Aug 10, 2026
14 checks passed
diegotoledano95 added a commit to diegotoledano95/extenddb that referenced this pull request Aug 10, 2026
 Pull in ExtendDB#239 (GSI pagination tiebreaker), ExtendDB#245 (SQLite stale GSI delay),
 and ExtendDB#246 (Postgres restore race) — the three pre-existing backend bugs the
 dual-target suite surfaced. Resolve the add/add conflict on
 restore_active_completeness.rs in favor of this branch version, which
 carries the reviewer-requested retry-on-scan-error oracle fix and a bounded
 observer; main branch Postgres restore fix lands via its own source change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants