Skip to content

fix(postgresql): make PITR restore_command replayable and timestamp comparison numeric#3072

Draft
weicao wants to merge 1 commit into
mainfrom
fix/pg-pitr-replayable-restore
Draft

fix(postgresql): make PITR restore_command replayable and timestamp comparison numeric#3072
weicao wants to merge 1 commit into
mainfrom
fix/pg-pitr-replayable-restore

Conversation

@weicao

@weicao weicao commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Problem

Deep-review finding 三.20 (issue #3071):

  1. restore_command consumed segments with mv: PostgreSQL re-requests earlier WAL segments through restore_command whenever recovery restarts mid-way (pod eviction, OOM, node restart — routine on Kubernetes). With mv, those segments are gone from the staging dir and the restore can never complete — the PITR attempt is burned. The .history branch already used cp, conceding the point.
  2. Lexicographic timestamp comparison in fetch-wal-log's stop condition ([[ $ts > $rt ]]): both operands are epoch seconds of currently-equal width, so it is accidentally correct today — a latent trap.

Changes

  • restore_command='cp ${PITR_DIR}/%f %p' for all files (PostgreSQL's own documented example shape). Disk boundary: fetch-wal-log stages the whole window up front, so the peak is already paid; cp adds only transient pg_wal duplication bounded by recovery checkpoint recycling.
  • -gt for the epoch comparison.
  • New spec (3 examples) that executes the same concatenation the ActionSet builds (set -e + common-scripts + fetch-wal-log + pitr-restore): pins the cp restore_command, the staging + idempotent-retry behavior, and the timestamp fix via a 9-digit/10-digit epoch pair that discriminates lexicographic from numeric comparison.

Evidence boundary

Static + concatenated-form shellspec (suite 23/23 locally, shellcheck clean at CI severity). The mid-recovery-restart replay scenario is not runtime-reproduced; it will be covered in the draft-batch integration round (T3 PITR window) and/or the #postgresql team's kubeblocks-tests negative-path cases (task #29 alignment).

Opened as draft per the batch process (tests pass → ready). Will be added to integration/pg-review-batch.

🤖 Generated with Claude Code

…omparison numeric

- restore_command moved WAL segments out of the staging dir as
  recovery consumed them ('mv ${PITR_DIR}/%f %p'); a mid-recovery
  restart (pod eviction, OOM — routine on Kubernetes) re-requests
  earlier segments through restore_command and they are gone, so the
  restore can never complete. The .history branch already used cp,
  conceding the hazard. Now cp for all files, matching PostgreSQL's
  own documented restore_command examples. Disk note: fetch-wal-log
  stages the entire window in PITR_DIR up front, so the peak is
  already paid; cp adds only transient duplication in pg_wal bounded
  by recovery checkpoint recycling.
- fetch-wal-log's stop condition used [[ $timestamp > $restore_time ]]
  — lexicographic. Both operands are epoch seconds (currently equal
  width, so accidentally correct); now -gt.
- new pitr_restore_spec.sh (3 examples): runs the same concatenation
  the ActionSet builds (set -e + common + fetch + restore) and pins
  the cp restore_command + staging/idempotent-retry behavior; the
  timestamp case uses a 9-digit vs 10-digit epoch pair that fails
  under lexicographic comparison and passes under -gt.

Fixes #3071

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (35e0ded) to head (f215a27).

Files with missing lines Patch % Lines
...ns/postgresql/scripts-ut-spec/pitr_restore_spec.sh 0.00% 67 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main   #3072   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        144     145    +1     
  Lines      23013   23080   +67     
=====================================
- Misses     23013   23080   +67     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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