From b48d56fb1944e0bfb2df6b5b3f9b87c5007472fd Mon Sep 17 00:00:00 2001 From: Rui Zhao Date: Wed, 21 Oct 2026 23:11:16 +0800 Subject: [PATCH 1/2] Fix handling of skipped prior versions during heap rewrites A live tuple can resolve a stashed prior version whose TOAST data is missing. Returning false for that prior version makes the caller treat the live tuple as skipped, triggering Assert(recently_dead) and charging the removal to the wrong tuple. Return false only when the caller's tuple is skipped. Count skipped stashed tuples separately, including those processed at the end of the rewrite, and use that count to correct the rewrite statistics. Add UPDATE tests for VACUUM FULL and REPACK, including a three-version chain that leaves an older tuple for end_heap_rewrite(). --- .../dblink/expected/rewrite_stale_xmin.out | 302 ++++++++++++++++++ contrib/dblink/specs/rewrite_stale_xmin.spec | 60 ++++ src/backend/access/heap/heapam_handler.c | 9 +- src/backend/access/heap/rewriteheap.c | 29 +- src/include/access/rewriteheap.h | 2 +- 5 files changed, 395 insertions(+), 7 deletions(-) diff --git a/contrib/dblink/expected/rewrite_stale_xmin.out b/contrib/dblink/expected/rewrite_stale_xmin.out index 2e5a7940ea2..c1162dfcc3f 100644 --- a/contrib/dblink/expected/rewrite_stale_xmin.out +++ b/contrib/dblink/expected/rewrite_stale_xmin.out @@ -235,3 +235,305 @@ dblink_disconnect OK (1 row) + +starting permutation: s1_hold_xmin s2_begin s3_update s3_vacuum_main_only s2_commit s3_vacuum_toast s3_vacuum_full s3_check_rows s1_release +step s1_hold_xmin: + SELECT dblink_connect('holder', + 'dbname=regress_other_db port=' || current_setting('port')); + SELECT dblink_exec('holder', 'BEGIN'); + SELECT dblink_exec('holder', 'CREATE TEMP TABLE xid_holder(x int)'); + +dblink_connect +-------------- +OK +(1 row) + +dblink_exec +----------- +BEGIN +(1 row) + +dblink_exec +------------ +CREATE TABLE +(1 row) + +step s2_begin: + BEGIN ISOLATION LEVEL REPEATABLE READ; + SELECT 1; + +?column? +-------- + 1 +(1 row) + +step s3_update: + UPDATE rewrite_test SET data = repeat('z', 2500) WHERE id = 1; + +step s3_vacuum_main_only: + VACUUM (PROCESS_TOAST false) rewrite_test; + +step s2_commit: COMMIT; +step s3_vacuum_toast: + VACUUM pg_toast.rewrite_test_toast; + +step s3_vacuum_full: + VACUUM FULL rewrite_test; + +step s3_check_rows: + SELECT id, length(data), left(data, 1) FROM rewrite_test ORDER BY id; + SELECT reltuples FROM pg_class WHERE oid = 'rewrite_test'::regclass; + +id|length|left +--+------+---- + 1| 2500|z + 2| 2500|y +(2 rows) + +reltuples +--------- + 2 +(1 row) + +step s1_release: + SELECT dblink_exec('holder', 'COMMIT'); + SELECT dblink_disconnect('holder'); + +dblink_exec +----------- +COMMIT +(1 row) + +dblink_disconnect +----------------- +OK +(1 row) + + +starting permutation: s1_hold_xmin s2_begin s3_update s3_vacuum_main_only s2_commit s3_vacuum_toast s3_repack s3_check_rows s1_release +step s1_hold_xmin: + SELECT dblink_connect('holder', + 'dbname=regress_other_db port=' || current_setting('port')); + SELECT dblink_exec('holder', 'BEGIN'); + SELECT dblink_exec('holder', 'CREATE TEMP TABLE xid_holder(x int)'); + +dblink_connect +-------------- +OK +(1 row) + +dblink_exec +----------- +BEGIN +(1 row) + +dblink_exec +------------ +CREATE TABLE +(1 row) + +step s2_begin: + BEGIN ISOLATION LEVEL REPEATABLE READ; + SELECT 1; + +?column? +-------- + 1 +(1 row) + +step s3_update: + UPDATE rewrite_test SET data = repeat('z', 2500) WHERE id = 1; + +step s3_vacuum_main_only: + VACUUM (PROCESS_TOAST false) rewrite_test; + +step s2_commit: COMMIT; +step s3_vacuum_toast: + VACUUM pg_toast.rewrite_test_toast; + +step s3_repack: + REPACK rewrite_test; + +step s3_check_rows: + SELECT id, length(data), left(data, 1) FROM rewrite_test ORDER BY id; + SELECT reltuples FROM pg_class WHERE oid = 'rewrite_test'::regclass; + +id|length|left +--+------+---- + 1| 2500|z + 2| 2500|y +(2 rows) + +reltuples +--------- + 2 +(1 row) + +step s1_release: + SELECT dblink_exec('holder', 'COMMIT'); + SELECT dblink_disconnect('holder'); + +dblink_exec +----------- +COMMIT +(1 row) + +dblink_disconnect +----------------- +OK +(1 row) + + +starting permutation: s1_hold_xmin s2_begin s3_update s3_update_again s3_vacuum_main_only s2_commit s3_vacuum_toast s3_vacuum_full s3_check_rows s1_release +step s1_hold_xmin: + SELECT dblink_connect('holder', + 'dbname=regress_other_db port=' || current_setting('port')); + SELECT dblink_exec('holder', 'BEGIN'); + SELECT dblink_exec('holder', 'CREATE TEMP TABLE xid_holder(x int)'); + +dblink_connect +-------------- +OK +(1 row) + +dblink_exec +----------- +BEGIN +(1 row) + +dblink_exec +------------ +CREATE TABLE +(1 row) + +step s2_begin: + BEGIN ISOLATION LEVEL REPEATABLE READ; + SELECT 1; + +?column? +-------- + 1 +(1 row) + +step s3_update: + UPDATE rewrite_test SET data = repeat('z', 2500) WHERE id = 1; + +step s3_update_again: + UPDATE rewrite_test SET data = repeat('w', 2500) WHERE id = 1; + +step s3_vacuum_main_only: + VACUUM (PROCESS_TOAST false) rewrite_test; + +step s2_commit: COMMIT; +step s3_vacuum_toast: + VACUUM pg_toast.rewrite_test_toast; + +step s3_vacuum_full: + VACUUM FULL rewrite_test; + +step s3_check_rows: + SELECT id, length(data), left(data, 1) FROM rewrite_test ORDER BY id; + SELECT reltuples FROM pg_class WHERE oid = 'rewrite_test'::regclass; + +id|length|left +--+------+---- + 1| 2500|w + 2| 2500|y +(2 rows) + +reltuples +--------- + 2 +(1 row) + +step s1_release: + SELECT dblink_exec('holder', 'COMMIT'); + SELECT dblink_disconnect('holder'); + +dblink_exec +----------- +COMMIT +(1 row) + +dblink_disconnect +----------------- +OK +(1 row) + + +starting permutation: s1_hold_xmin s2_begin s3_update s3_update_again s3_vacuum_main_only s2_commit s3_vacuum_toast s3_repack s3_check_rows s1_release +step s1_hold_xmin: + SELECT dblink_connect('holder', + 'dbname=regress_other_db port=' || current_setting('port')); + SELECT dblink_exec('holder', 'BEGIN'); + SELECT dblink_exec('holder', 'CREATE TEMP TABLE xid_holder(x int)'); + +dblink_connect +-------------- +OK +(1 row) + +dblink_exec +----------- +BEGIN +(1 row) + +dblink_exec +------------ +CREATE TABLE +(1 row) + +step s2_begin: + BEGIN ISOLATION LEVEL REPEATABLE READ; + SELECT 1; + +?column? +-------- + 1 +(1 row) + +step s3_update: + UPDATE rewrite_test SET data = repeat('z', 2500) WHERE id = 1; + +step s3_update_again: + UPDATE rewrite_test SET data = repeat('w', 2500) WHERE id = 1; + +step s3_vacuum_main_only: + VACUUM (PROCESS_TOAST false) rewrite_test; + +step s2_commit: COMMIT; +step s3_vacuum_toast: + VACUUM pg_toast.rewrite_test_toast; + +step s3_repack: + REPACK rewrite_test; + +step s3_check_rows: + SELECT id, length(data), left(data, 1) FROM rewrite_test ORDER BY id; + SELECT reltuples FROM pg_class WHERE oid = 'rewrite_test'::regclass; + +id|length|left +--+------+---- + 1| 2500|w + 2| 2500|y +(2 rows) + +reltuples +--------- + 2 +(1 row) + +step s1_release: + SELECT dblink_exec('holder', 'COMMIT'); + SELECT dblink_disconnect('holder'); + +dblink_exec +----------- +COMMIT +(1 row) + +dblink_disconnect +----------------- +OK +(1 row) + diff --git a/contrib/dblink/specs/rewrite_stale_xmin.spec b/contrib/dblink/specs/rewrite_stale_xmin.spec index fe88e789f85..4fd82116f4c 100644 --- a/contrib/dblink/specs/rewrite_stale_xmin.spec +++ b/contrib/dblink/specs/rewrite_stale_xmin.spec @@ -65,6 +65,19 @@ step s3_delete { DELETE FROM rewrite_test WHERE id = 1; } +step s3_update +{ + UPDATE rewrite_test SET data = repeat('z', 2500) WHERE id = 1; +} +step s3_update_again +{ + UPDATE rewrite_test SET data = repeat('w', 2500) WHERE id = 1; +} +step s3_check_rows +{ + SELECT id, length(data), left(data, 1) FROM rewrite_test ORDER BY id; + SELECT reltuples FROM pg_class WHERE oid = 'rewrite_test'::regclass; +} step s3_vacuum_main_only { VACUUM (PROCESS_TOAST false) rewrite_test; @@ -135,3 +148,50 @@ permutation s3_vacuum_toast s3_create_index s1_release + +# Preserve the live successor when a stashed prior version loses its TOAST. +permutation + s1_hold_xmin + s2_begin + s3_update + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_vacuum_full + s3_check_rows + s1_release + +permutation + s1_hold_xmin + s2_begin + s3_update + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_repack + s3_check_rows + s1_release + +permutation + s1_hold_xmin + s2_begin + s3_update + s3_update_again + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_vacuum_full + s3_check_rows + s1_release + +permutation + s1_hold_xmin + s2_begin + s3_update + s3_update_again + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_repack + s3_check_rows + s1_release diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index b1df7d19863..30b070e903b 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -1072,7 +1072,14 @@ normal_put: /* Write out any remaining tuples, and fsync if needed */ if (rwstate) - end_heap_rewrite(rwstate); + { + double tuples_dropped = end_heap_rewrite(rwstate); + + /* Stashed tuples with missing TOAST were scanned as recently dead. */ + *tups_vacuumed += tuples_dropped; + *num_tuples -= tuples_dropped; + *tups_recently_dead -= tuples_dropped; + } if (bistate) FreeBulkInsertState(bistate); diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 0d57b3f5709..0ba2f202dd8 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -150,6 +150,7 @@ typedef struct RewriteStateData XLogRecPtr rs_begin_lsn; /* XLogInsertLsn when starting the rewrite */ HTAB *rs_unresolved_tups; /* unmatched A tuples */ HTAB *rs_old_new_tid_map; /* unmatched B tuples */ + double rs_tuples_dropped; /* stashed tuples with missing TOAST */ HTAB *rs_logical_mappings; /* logical remapping files */ uint32 rs_num_rewrite_mappings; /* # in memory mappings */ } RewriteStateData; @@ -296,12 +297,14 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm * End a rewrite. * * state and any other resources are freed. + * Returns the number of stashed tuples skipped because their TOAST is missing. */ -void +double end_heap_rewrite(RewriteState state) { HASH_SEQ_STATUS seq_status; UnresolvedTup unresolved; + double tuples_dropped; /* * Write any remaining tuples in the UnresolvedTups table. If we have any @@ -312,7 +315,8 @@ end_heap_rewrite(RewriteState state) while ((unresolved = hash_seq_search(&seq_status)) != NULL) { ItemPointerSetInvalid(&unresolved->tuple->t_data->t_ctid); - raw_heap_insert(state, unresolved->tuple, unresolved->recently_dead); + if (!raw_heap_insert(state, unresolved->tuple, unresolved->recently_dead)) + state->rs_tuples_dropped += 1; } /* Write the last page, if any */ @@ -326,8 +330,12 @@ end_heap_rewrite(RewriteState state) logical_end_heap_rewrite(state); + tuples_dropped = state->rs_tuples_dropped; + /* Deleting the context frees everything */ MemoryContextDelete(state->rs_cxt); + + return tuples_dropped; } /* @@ -462,10 +470,21 @@ rewrite_heap_tuple(RewriteState state, /* Insert the tuple and find out where it's put in new_heap */ if (!raw_heap_insert(state, new_tuple, recently_dead)) { - if (free_new) - heap_freetuple(new_tuple); + if (!free_new) + { + MemoryContextSwitchTo(old_cxt); + return false; + } + + /* + * The caller's tuple was already written. Count this skipped + * prior version separately; end_heap_rewrite() will handle any + * still older versions waiting for it. + */ + state->rs_tuples_dropped += 1; + heap_freetuple(new_tuple); MemoryContextSwitchTo(old_cxt); - return false; + return true; } new_tid = new_tuple->t_self; diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 67467c2ddbe..ec9d64112ad 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -24,7 +24,7 @@ typedef struct RewriteStateData *RewriteState; extern RewriteState begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xmin, TransactionId freeze_xid, MultiXactId cutoff_multi); -extern void end_heap_rewrite(RewriteState state); +extern double end_heap_rewrite(RewriteState state); /* See toast_helper.h for the values of "flags" */ extern bool rewrite_heap_tuple(RewriteState state, HeapTuple old_tuple, -- 2.43.7