Add physical differential oracle test for incremental backups#79
Draft
NikolayS wants to merge 2 commits into
Draft
Add physical differential oracle test for incremental backups#79NikolayS wants to merge 2 commits into
NikolayS wants to merge 2 commits into
Conversation
Incremental backups depend on the WAL summarizer, which only learns about relation blocks that WAL records properly register. Any code path that modifies a page without registering its buffer (or without WAL-logging the change at all) is a summarizer blind spot: pg_basebackup omits the block from the incremental backup and pg_combinebackup silently reconstructs a stale page from an older backup. Bugs of this class - most recently unregistered visibility map buffers, fixed in ed62d26 - were previously only detectable by enumerating each affected operation (as 012_vm_consistency.pl does). This adds a generic physical oracle that catches the whole class instead of known instances. The new test runs a mixed workload (heap with indexes, TOAST, VM bit set/clear cycles, bulk COPY, TRUNCATE and re-extension, index build/drop, table drop, an unlogged table for init fork coverage) with churn running concurrently with a chain of rate-limited incremental backups F0 <- I1 <- I2. It then quiesces the cluster (VACUUM, a hint-bit settle sweep over catalogs and user tables, CHECKPOINT) and takes a final incremental I3 plus a reference full backup R back to back. pg_waldump audits the WAL window spanning both final backups: if any record in it carries a block reference, the two backups may not describe the same cluster state and the pair is retaken (bounded retries), so the comparison is deterministic by construction rather than by luck. pg_combinebackup(F0, I1, I2, I3) must then be physically identical to R, block by block, for every main, _vm and _init fork under base/ and global/. FSM forks are skipped (not fully WAL-logged by design). Only pd_lsn and pd_checksum are masked; hint bits are compared strictly, which is sound because data checksums (on by default) force hint-bit changes to be WAL-logged. In practice even the masked bytes match; the mask exists as documented insurance. Any divergence is reported with file, relation name, block number and differing byte ranges. Closes #74 Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP
11 tasks
Review feedback on the physical differential oracle raised three points;
this addresses all of them without changing the oracle's verdict logic.
1. Framing. The header no longer implies the final incremental and the
reference full backup share a stop LSN -- they cannot, since each emits
its own checkpoint/backup-end/switch records. Instead the comment now
states the actual equivalence argument: the two backups are comparable
because no relation block changes on disk between them, which is audited
(pg_waldump over the window spanning both backups must show no block
references), not assumed. The audit is conservative-sufficient because
with data checksums on, a relation page can only reach disk modified if
some WAL record referencing it was inserted first; the hint-bit FPI
loophole is closed by having I3's own start checkpoint inside the window.
The alternative reviewer-suggested design (restore both paths, replay to
a common recovery_target_lsn, then compare) was considered and rejected:
WAL replay applies full-page images that would repair exactly the stale
blocks this oracle exists to catch, so the zero-replay comparison is the
stronger oracle for the summarizer-blind-spot class.
2. Normalization structure. Blanket masking is replaced by a per-block
normalizer dispatched on fork and access method ("fork:am" -> "fork:*"
-> "*:*"), with the relation's relkind/AM resolved from pg_class and
included in divergence reports. The dispatch currently maps everything
to the page-header mask (pd_lsn + pd_checksum), which is sufficient
under this test's documented preconditions (quiesced audited window,
checksums on, heap/btree/sequence/toast only); the comment now spells
out per fork and AM what could legitimately diverge under relaxed
conditions (heap infomask hint bits and pd_prune_xid, btree LP_DEAD
hints and btpo_cycleid) so that AM-aware maskers slot in locally
instead of requiring a comparator rewrite.
3. Committed kill test. The previously ad-hoc corruption check is now
part of the test: after the main comparison, one byte is flipped
(outside the masked header) in copies of a heap main-fork block and a
visibility-map block from the combined output, and the comparator must
report exactly one divergent block each, attributed to the correct
file, block and byte offset. This keeps the oracle honest against
future masking or file-collection regressions that would otherwise
turn the main comparison into a trivial pass.
Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP
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.
Physical oracle for incremental backup completeness
New TAP test
src/bin/pg_combinebackup/t/013_physical_differential.pl(registered inmeson.build; the Makefile picks it up via thet/*.plglob).Why
The WAL summarizer only learns about relation blocks that WAL records register. Any code path that modifies a page without registering its buffer (or without WAL-logging at all) is invisible to it: the incremental backup omits the block and
pg_combinebackupsilently reconstructs a stale page from an older backup. Bugs of this class (most recently the unregistered VM buffers fixed in ed62d26) were previously only detectable by enumerating known operations —012_vm_consistency.plchecks INSERT/UPDATE/DELETE/LOCK/COPY explicitly. This test is the complementary generic oracle: it doesn't need to know which operation is broken; a stale block of any kind, in any compared fork, in any relation, fails the byte comparison.Design choice: audited-quiescent-window comparison, not replay-to-target-LSN
Reviewer feedback correctly noted that separately taken backups do not share a stop LSN — each emits its own checkpoint/backup-end/WAL-switch records — and suggested restoring both paths and replaying to one target LSN. After weighing both, this PR deliberately keeps the zero-replay design, and the test header now states the actual equivalence argument instead of implying stop-LSN equality:
pg_waldumpscans the WAL window from just before I3's start checkpoint to just after R completes; if any record in the window carries a block reference, the pair is rejected and retaken (≤3 attempts) before failing loudly.data_checksums=on(asserted, fails fast otherwise) and on the audit window's coverage, which is itself verified by a post-windowtxid_current()sentinel record provingpg_waldumpread through the window end.Test structure
F0, then incrementalsI1,I2taken while abackground_psqlsession runs bounded DO-block churn (backups rate-limited--max-rate=4Mso they genuinely overlap: I1 ≈ 1.2 s, I2 ≈ 2.6 s vs ≈ 2.4 s churn).VACUUM, hint-bit sweep over catalogs and user tables,CHECKPOINT), then I3 and reference full R back to back, accepted only if the audited WAL window is free of block references (retake procedure above).pg_combinebackup(F0,I1,I2,I3)must be physically identical toR— block-by-block over every main,_vm,_initfork underbase/andglobal/(923 files / 3344 blocks per run). File-set equality asserted both directions; anti-triviality guards require >20 files and >500 blocks compared.Normalization: per-fork/per-AM hook, page-header mask in effect
Per review, blanket masking was restructured into a normalizer dispatch keyed by fork and access method (
"fork:am"→"fork:*"→"*:*"), with relkind/AM resolved frompg_classand included in every divergence report. The dispatch currently routes everything to the page-header mask —pd_lsn+pd_checksumonly — which is sufficient under this test's documented preconditions (audited-quiescent window, checksums on, heap/btree/sequence/toast relations only). The comparator comment documents, per fork/AM, what could legitimately diverge under relaxed conditions and where the corresponding masker would slot in as a local change:HEAP_XMIN/XMAX_COMMITTED…) andpd_prune_xid(cf. heap'srm_mask) — not needed here because checksums=on forces hint changes through WAL;LP_DEADline-pointer hints,btpo_cycleid— same reasoning;_vm: pure bitmap payload behind the header, no legitimate unlogged content when heap VM operations are WAL-correct (which is what the oracle checks);_init: written once, no legitimate divergence;_fsm: intentionally not WAL-logged — excluded entirely rather than masked.Empirical result: across all runs, 0 blocks even needed the pd_lsn/pd_checksum mask (counter reported via
notewhen nonzero) — the combined output is bit-identical to the reference.Kill test (committed, deterministic)
The corruption-injection check is now part of the test (subtests 16–21), not a prose procedure: after the main comparison, one byte is flipped — outside the masked header bytes — in copies of a heap main-fork block (block 3, offset 4000) and a VM block (block 0, offset 100) from the combined output; the comparator must report exactly one divergent block each with correct file/block/offset attribution:
This also guards the main comparison against rotting into a trivial pass via a future masking or file-collection regression.
Divergence findings on master
None. On current master (post-ed62d26) the combined backup is bit-identical to the reference full backup in every run. No new summarizer blind spot detected by this workload.
Provenance & run evidence
d374280(v19devel master tip; ed62d26 in ancestry). PR head:52e346d(test) on8177c62(initial version).meson setup build --buildtype=debug -Dcassert=true -Dtap_tests=enabled; gcc, Linux x86_64; TAP drivermeson test --num-processes 1 pg_combinebackup/013_physical_differential, run as an unprivileged user.summarize_wal=on,autovacuum=off,wal_keep_size=1GB(prevents segment recycling within the ~16-segment test lifetime so the audit window stays readable),allows_streamingnode init, data checksums at initdb default (on — asserted as subtest 1). Combine mode--copy(env-selectable viaPG_TEST_PG_COMBINEBACKUP_MODElike sibling tests).pg_combinebackupsuite green (13 tests, 013 at 10.08 s). Earlier iterations of the same design: 8/8 green at 9–12 s. Every run accepted the final backup pair on attempt 1; the retake path has never been exercised outside fault injection.build/testrun/pg_combinebackup/013_physical_differential/log/regress_log_013_physical_differential(TAP output incl. LSN window notes and any DIVERGENCE reports) and.../log/013_physical_differential_primary.log(server log);build/meson-logs/testlog.txt(suite summary).blkrefrecords frompg_waldump(first 10), retakes settle+I3+R up to 3 attempts, and aborts loudly if none is clean — so a genuine quiescence breaker is characterized, not averaged away. On comparison failure, reports carry relpath, relname, relkind, AM, fork, absolute block number (segment-aware) and differing byte ranges with hex from both sides (capped at 50 reports / 5 ranges each) — enough to minimize straight to the responsible relation and operation.Caveats
--no-data-checksumsthe strict hint-bit comparison would be unsound; the test fails fast on the checksums assertion rather than producing false positives.pg_waldumpquirk worked around: an--endLSN sitting exactly at a fresh segment's first-record position triggers a spurious prev-link error from a clamped page read (stale read-buffer bytes; reproducible on static files) — the test therefore runs without--endand verifies coverage via the sentinel record. Possibly worth an independent upstream report.Closes #74
🤖 Generated with Claude Code
https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP