feat(ops): PostgreSQL WAL archive and PITR restore drill - #1464
feat(ops): PostgreSQL WAL archive and PITR restore drill#1464seonghobae wants to merge 9 commits into
Conversation
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change hardens PostgreSQL Compose services, adds a PITR restore service, automates WAL and recovery validation, and records drill evidence and operational boundaries. ChangesPostgreSQL PITR validation
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟠 High · up to This PR changes the HA primary to archive WAL and adds PITR restore tooling, but it is not merge-ready yet: a partially copied WAL file may be accepted as complete, a reused restore volume may produce stale or incomplete recovery results, and the drill still has unresolved recovery-time and cleanup handling issues that can make restore evidence misleading or leave resources behind. Sequence Diagram(s)sequenceDiagram
participant postgres_pitr_drill.sh
participant db-primary
participant wal_archive
participant base_backup
participant db-restore
postgres_pitr_drill.sh->>db-primary: Start primary and create markers
db-primary->>base_backup: Create physical base backup
db-primary->>wal_archive: Archive WAL segments
postgres_pitr_drill.sh->>db-restore: Start targeted restore
db-restore->>base_backup: Load base backup
db-restore->>wal_archive: Replay WAL to target timestamp
db-restore-->>postgres_pitr_drill.sh: Promote recovered database
postgres_pitr_drill.sh->>db-restore: Verify markers and post-recovery writes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
PR governance metadata gate is not ready for
|
|
Validated and fixed on current head
Validation: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Repaired at exact head |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Repaired the current-head WAL archival finding at exact head |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Fixed the current-head Compose interpolation defect in Validation on current head |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
The remaining open review thread was informational Devin analysis confirming WAL archival ordering; it did not request a code change. I verified the current exact head |
Dismissed as stale after verifying restore_data recreation and atomic WAL archive fixes are present on exact head 5af23f1; all review threads are resolved.
|
Current-head review reconciliation: both CodeRabbit CHANGES_REQUESTED reviews targeted superseded commits ( |
Exact-head maintenance audit
|
OpenCode Review Overview
Pull request overviewOpenCode reviewed the current-head bounded evidence and requested changes before merge. Findings1. P1 docker-compose.postgres-pitr-restore.yml:53 - Repeat
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and requested changes before merge.
Findings
1. P1 docker-compose.postgres-pitr-restore.yml:53 - Repeat db-restore runs can silently reuse the previous recovery point
- Problem: Evidence type: nearby implementation / current-head focused hunk.
docker-compose.postgres-pitr-restore.ymldefinesrestore_dataas a persistent named volume, and the restore entrypoint only rematerializes from/base_backupwhenPG_VERSIONis absent. Re-running the documenteddb-restoreoverlay on the same compose project with a newRECOVERY_TARGET_TIMEtherefore skips the recovery setup and can keep serving the earlier restore target instead of the requested one. - Root cause: The restore flow keys first-run initialization only on
PG_VERSION, but the overlay persists restore state across runs via the namedrestore_datavolume. Nothing compares the requestedRECOVERY_TARGET_TIMEto the previously materialized restore metadata or forces a fresh volume for a new restore. - Fix: Make repeat-use safe: either (a) rematerialize
restore_datawhenever a new PITR target is requested, or (b) persist the last materialized target and fail fast when the operator reuses the same volume with a differentRECOVERY_TARGET_TIME. If the intended contract is one-shot-per-project, document that directly in the overlay header and enforce it instead of silently reusing stale data. - Regression test: Use the PR's documented restore surface and repeat it on the same compose project: first start
db-restorewith oneRECOVERY_TARGET_TIME, then rerundocker compose -p <same-project> -f docker-compose.postgres-ha.yml -f docker-compose.postgres-pitr-restore.yml up -d db-restorewith a different target and withoutdown -v. The second run should either rematerialize the restore or fail with an explicit stale-volume error; it must not silently keep the old target. - Suggested diff: posted in this finding's inline review thread.
Summary
Pull request overview: reviewed current-head bounded evidence plus focused hunks for CHANGELOG.md, docker-compose.postgres-ha.yml, docker-compose.postgres-pitr-restore.yml, docs/operations/postgresql-ha-drill-20260615.md, and docs/operations/postgresql-pitr-drill-2026-08-25.md. One blocking correctness issue remains in the PITR restore flow. Approval sufficiency: not sufficient because the restore overlay can return a previous recovery point on a normal repeat-restore operator path. Verification posture: source-trace review from current-head bounded evidence; direct reads from /home/runner/work/_temp/opencode-pr-head were blocked, so conclusions rely on the authoritative focused hunks and bounded source traces. Linter/static: no completed failed GitHub Checks were present when evidence was collected. TDD/regression: no repo-native regression in the supplied evidence covers repeated db-restore starts without down -v; this needs an explicit repeat-restore guard test or documented verification. Coverage: Coverage execution evidence marked PASS and explicitly says test coverage is not applicable because no supported changed source files or package manifests were found. Docstring coverage: Coverage execution evidence says docstring coverage is not applicable because no supported changed source files or package manifests were found. DAG: source-backed head-flow diagram below maps docker-compose.postgres-pitr-restore.yml to the stale-volume PITR path. PoC/execution: no trusted execution receipt for this restore edge was supplied; review is based on bounded source traces plus the committed drill transcript. DDD/domain: the domain contract is PITR to a requested instant, and silent reuse of an older materialized restore violates that contract. CDD/context: the new overlay is documented for direct operator use via docker compose ... up -d db-restore, so repeatability on the same compose project matters. Similar issues: historical current-head context already shows recent fixes here for UTC targeting, archive idempotence, and cleanup; this remaining stale-volume path is in the same recovery boundary. Claim/concept check: the one-shot drill transcript is internally consistent, but the reusable overlay behavior is not guarded against a changed target on a reused project. Standards search: not required; this is a repository-local compose control-flow issue. Compatibility/convention: changed identifiers (db-primary, db-replica, db-restore, wal_archive, base_backup, restore_data) follow local multi-word naming and no new exposed sequential identifiers were introduced. Breaking-change/backcompat: ops-only change, but the current restore overlay is unsafe for repeat use on the same compose project until it rematerializes or rejects stale data. Performance: no separate performance blocker identified. Developer experience: DX surface is the documented local DR drill and restore overlay; silent reuse of old restore state makes operator debugging and recovery validation unreliable. User experience: UX surface is operator-facing recovery behavior and evidence accuracy; restoring to the wrong point-in-time is high impact. Visual/DOM: non-web surface reviewed—Compose service definitions, shell-driven recovery flow, and ops evidence docs. Accessibility/i18n: non-web ops surface; no UI-specific accessibility or i18n change reviewed. Supply-chain/license: existing pgvector/pgvector:pg16 image usage continues and no new package manifest was changed in current-head evidence. Packaging: Coverage execution evidence says no supported changed source files or package manifests were found; reviewed as YAML/shell/docs operational surfaces. Security/privacy: no-new-privileges, read_only, and explicit tmpfs are good hardening steps, but stale restore-volume reuse is still a recovery-boundary data-integrity risk.
flowchart TD
A["docker-compose.postgres-pitr-restore.yml"] --> B["persistent named volume \"restore_data\""]
B --> C["startup guard only rematerializes when PG_VERSION is absent"]
C --> D["new RECOVERY_TARGET_TIME can be ignored on repeat run"]
D --> E["verify by starting db-restore twice on the same project without down -v"]
Adversarial validation
{"status":"failed","probes":[{"path":"docker-compose.postgres-pitr-restore.yml","line":53,"hypothesis":"A second `db-restore` run on the same compose project can silently reuse the first restore state instead of honoring a new `RECOVERY_TARGET_TIME`.","attack_or_counterexample":"Materialize `db-restore` once, then rerun `docker compose -p <same-project> -f docker-compose.postgres-ha.yml -f docker-compose.postgres-pitr-restore.yml up -d db-restore` with a different `RECOVERY_TARGET_TIME` and without `down -v`.","evidence":"Trusted source trace at docker-compose.postgres-pitr-restore.yml:53 observed `restore_data` is a persistent named volume, while the same file's startup guard only copies `/base_backup` and rewrites `recovery_target_time` when `PG_VERSION` is absent; on a repeat run with an already-populated volume, initialization is skipped and stale restore state is retained. Trusted current-head source binding at docker-compose.postgres-pitr-restore.yml:53; source-line-sha256=80bd7d707f755c9a47b5a667bb0988de3a387302d1c6970882cadb1a8238c530","outcome":"confirmed"},{"path":"docker-compose.postgres-ha.yml","line":3,"hypothesis":"The new `read_only` hardening removes required PostgreSQL write paths and obviously breaks the HA services.","attack_or_counterexample":"Trace the writable paths left after enabling `read_only: true` on `db-primary` and `db-replica`.","evidence":"Trusted source trace at docker-compose.postgres-ha.yml:3 observed the hardening is paired with writable named volumes for `/var/lib/postgresql/data`, `/wal_archive`, and `/base_backup` plus explicit `tmpfs` mounts for `/tmp` and `/run/postgresql`, so the compose definition still preserves the required write targets visible in the current-head hunk. Trusted current-head source binding at docker-compose.postgres-ha.yml:3; source-line-sha256=5a1154c2a38c8c30a8e69971614f36ea799425368df154f0b338917f5d971346","outcome":"falsified"}],"residual_risk":"The main unresolved risk is incorrect PITR target selection on repeated `db-restore` use within the same compose project. Other reviewed current-head changes—the UTC offset, archive idempotence/atomicity, and container hardening—look source-consistent in the bounded evidence, but no trusted execution receipt was supplied for this specific repeat-restore edge."}-
Result: REQUEST_CHANGES
-
Reason: The new PITR restore overlay can silently reuse a stale
restore_datavolume and ignore a laterRECOVERY_TARGET_TIMEon repeateddb-restoreruns. -
Head SHA:
5af23f188d764128636e1f308842bbd9f3329ca7 -
Workflow run: 32883046501
-
Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (2 files)"]
R2 --> V2["docs review"]
| volumes: | ||
| wal_archive: | ||
| base_backup: | ||
| restore_data: |
There was a problem hiding this comment.
P1 Repeat db-restore runs can silently reuse the previous recovery point
- Location:
docker-compose.postgres-pitr-restore.yml:53 - Problem: Evidence type: nearby implementation / current-head focused hunk.
docker-compose.postgres-pitr-restore.ymldefinesrestore_dataas a persistent named volume, and the restore entrypoint only rematerializes from/base_backupwhenPG_VERSIONis absent. Re-running the documenteddb-restoreoverlay on the same compose project with a newRECOVERY_TARGET_TIMEtherefore skips the recovery setup and can keep serving the earlier restore target instead of the requested one. - Root cause: The restore flow keys first-run initialization only on
PG_VERSION, but the overlay persists restore state across runs via the namedrestore_datavolume. Nothing compares the requestedRECOVERY_TARGET_TIMEto the previously materialized restore metadata or forces a fresh volume for a new restore. - Fix: Make repeat-use safe: either (a) rematerialize
restore_datawhenever a new PITR target is requested, or (b) persist the last materialized target and fail fast when the operator reuses the same volume with a differentRECOVERY_TARGET_TIME. If the intended contract is one-shot-per-project, document that directly in the overlay header and enforce it instead of silently reusing stale data. - Regression test: Use the PR's documented restore surface and repeat it on the same compose project: first start
db-restorewith oneRECOVERY_TARGET_TIME, then rerundocker compose -p <same-project> -f docker-compose.postgres-ha.yml -f docker-compose.postgres-pitr-restore.yml up -d db-restorewith a different target and withoutdown -v. The second run should either rematerialize the restore or fail with an explicit stale-volume error; it must not silently keep the old target.
Suggested diff
```diff
+ current_target_file="$${pg_data}/.pitr_recovery_target_time"
if [ ! -s "$${pg_data}/PG_VERSION" ]; then
echo "Materializing restore data directory from base backup."
find "$${pg_data}" -mindepth 1 -delete
cp -a /base_backup/. "$${pg_data}/"
rm -f "$${pg_data}/postmaster.pid" "$${pg_data}/standby.signal"
touch "$${pg_data}/recovery.signal"
{
printf "restore_command = 'cp /wal_archive/%%f %%p'\n"
printf "recovery_target_time = '%s'\n" "$${RECOVERY_TARGET_TIME}"
printf "recovery_target_action = 'promote'\n"
} >> "$${pg_data}/postgresql.auto.conf"
+ printf "%s\n" "$${RECOVERY_TARGET_TIME}" > "$${current_target_file}"
chown -R postgres:postgres "$${pg_data}"
chmod 700 "$${pg_data}"
+ elif [ -f "$${current_target_file}" ] && ! grep -Fxq "$${RECOVERY_TARGET_TIME}" "$${current_target_file}"; then
+ printf >&2 "restore_data was already materialized for a different RECOVERY_TARGET_TIME; remove the volume or use a fresh compose project before rerunning PITR.\n"
+ exit 1
fi
| echo "Restoring again from the same volume targeting ${second_recovery_target_time} UTC." | ||
| export RECOVERY_TARGET_TIME="${second_recovery_target_time}" | ||
| "${full_compose[@]}" up -d db-restore |
There was a problem hiding this comment.
🔍 Re-target depends on compose recreating the container on env change
The second restore assumes up -d db-restore recreates the container so the new RECOVERY_TARGET_TIME takes effect (postgres_pitr_drill.sh). Compose bakes env at create time; a provider that merely restarts the stopped container keeps the old target and the assertion at postgres_pitr_drill.sh fails. This fails loud, not silently, and the stamp logic still re-materializes safely.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Supports #1428
PR 제목 (Title)
feat(ops): PostgreSQL WAL archive and PITR restore drill
목적 (Purpose)
Closes the GA-1 Wave-1 gap "Complete PostgreSQL WAL/PITR/failover/restore evidence" tracked under issue #1428 and the product-technical-gap-baseline program. The Postgres HA stack previously had failover coverage but no WAL archiving and no restore path, so disaster-recovery evidence was incomplete. This PR adds WAL archiving on the primary, a PG16-correct PITR restore overlay, and an automated drill producing redacted evidence.
주요 변경 사항 (Key Changes)
docker-compose.postgres-ha.yml: primary enables WAL archiving (archive_mode=on+ POSIX-safearchive_command) writing completed segments into a dedicatedwal_archivevolume.docker-compose.postgres-pitr-restore.yml(new db-restore overlay): PostgreSQL 16-correct targeted recovery —recovery.signalpluspostgresql.auto.confentries (restore_command,recovery_target_time,recovery_target_action=promote) — replaying archived WAL to the target time and promoting automatically.scripts/postgres_pitr_drill.sh(new): end-to-end PITR drill following the establishedpostgres_ha_drill.shconventions (env-only secrets,ok:assertions,trapcleanup).docs/operations/postgresql-pitr-drill-2026-08-25.md(new): redacted evidence doc from the 2026-08-25 drill run.docs/operations/postgresql-ha-drill-20260615.md: one-sentence Production-boundary update in the existing HA drill evidence doc.Real verification evidence (live drill run, 2026-08-25)
Regression: full HA failover drill re-run passed against the modified compose file.
Focused checks used:
bash -n scripts/postgres_pitr_drill.shandbash -n scripts/postgres_ha_drill.shshellcheck scripts/postgres_pitr_drill.sh(clean)변경 범위 / 영향도 (Scope / Impacted Areas)
wal_archivevolume), new restore overlay, ops drill script, ops evidence docs. Application code, API routes, and frontend are untouched.wal_archivevolume of the primary; all drill traffic stays on throwaway Compose projects bound to drill-only ports (55442–55444), torn down bytrapcleanup.SHOW VERSION, failure ⇒unknown) does not apply here. Recovery/promotion traffic is intentionally pinned to direct primary/restored-instance DSNs rather than any pooled or read-only endpoint — transactional recovery must stay on primary-class connections.POSTGRES_PASSWORDvia environment only (script exits otherwise); evidence doc and transcript are redacted.변경 사항 표 (Change Table)
docker-compose.postgres-ha.ymlarchive_mode=on, idempotentarchive_command=test ! -f /wal_archive/%f && cp %p /wal_archive/%f,wal_archivevolume mount, init step creating/wal_archive+/base_backupdocker-compose.postgres-pitr-restore.ymlpg_datafrom base backup, touchesrecovery.signal, writespostgresql.auto.confwithrestore_command='cp /wal_archive/%f %p',recovery_target_time=${RECOVERY_TARGET_TIME},recovery_target_action=promoterecovery_target_action=promoteends replay exactly at the target and opens the instance for post-restore write validation.scripts/postgres_pitr_drill.shtrap-baseddown -vcleanuppostgres_ha_drill.shconventions (ok:lines, env-only secrets, trap cleanup) so operators apply one mental model across both drills.docs/operations/postgresql-pitr-drill-2026-08-25.mddocs/operations/postgresql-ha-drill-20260615.mdSequence Diagram(s)
sequenceDiagram participant D as Drill script participant P as Primary (compose) participant W as wal_archive volume participant R as Restored instance D->>P: pg_basebackup into /base_backup P-->>D: ok: base backup completed D->>P: commit pre-recovery marker (before target) P->>W: archive_command copies segment 000000010000000000000004 D->>P: commit post-target marker P->>W: archive_command copies segment 000000010000000000000005 D->>R: start restore overlay with RECOVERY_TARGET_TIME R->>W: restore_command cp segments during replay R->>R: reach target -> recovery_target_action=promote D->>R: assert pre-recovery marker present, post-target marker absent D-->>D: ok: restored instance accepts writes after promotionSummary by CodeRabbit
New Features
Bug Fixes
Documentation