From 687708781e8f43a09a2a6bcd55b6d8cbb3b10d53 Mon Sep 17 00:00:00 2001 From: Rui Zhao Date: Thu, 20 Aug 2026 00:10:23 +0800 Subject: [PATCH] Write stashed update-chain tuples with their own TOAST flags rewrite_heap_tuple() stashes the prior version of an update pair in rs_unresolved_tups and writes it out when the successor arrives, but the write reused the successor's flags: a live successor resolves a recently-dead prior version with flags = 0, and detoasting the prior version's already-reclaimed chunks fails with "missing chunk". The same applied to the leftovers end_heap_rewrite() writes, which passed a hardcoded 0. Keep the flags a tuple was scanned with in its UnresolvedTupData and use them at both write-out sites. A failed insert of a stashed tuple cannot be reported through rewrite_heap_tuple()'s return value -- the caller's tuple has already been written, and the caller would adjust the counters for the wrong tuple -- so drop the tuple on the spot and have end_heap_rewrite() return the number of such drops, which the caller folds into tups_vacuumed / num_tuples / tups_recently_dead. Add UPDATE permutations for VACUUM FULL and REPACK to the rewrite_stale_xmin isolation spec; they fail with "missing chunk" without this fix. --- .../dblink/expected/rewrite_stale_xmin.out | 118 ++++++++++++++++++ contrib/dblink/specs/rewrite_stale_xmin.spec | 27 ++++ src/backend/access/heap/heapam_handler.c | 15 ++- src/backend/access/heap/rewriteheap.c | 44 ++++++- src/include/access/rewriteheap.h | 2 +- 5 files changed, 199 insertions(+), 7 deletions(-) diff --git a/contrib/dblink/expected/rewrite_stale_xmin.out b/contrib/dblink/expected/rewrite_stale_xmin.out index 2e5a7940ea2..ff600edd68d 100644 --- a/contrib/dblink/expected/rewrite_stale_xmin.out +++ b/contrib/dblink/expected/rewrite_stale_xmin.out @@ -235,3 +235,121 @@ 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 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 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 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 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..b670c6b4435 100644 --- a/contrib/dblink/specs/rewrite_stale_xmin.spec +++ b/contrib/dblink/specs/rewrite_stale_xmin.spec @@ -65,6 +65,10 @@ 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_vacuum_main_only { VACUUM (PROCESS_TOAST false) rewrite_test; @@ -135,3 +139,26 @@ permutation s3_vacuum_toast s3_create_index s1_release + +# VACUUM FULL after an UPDATE: the prior version of the update pair reaches +# the new heap through rs_unresolved_tups +permutation + s1_hold_xmin + s2_begin + s3_update + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_vacuum_full + s1_release + +# REPACK after an UPDATE +permutation + s1_hold_xmin + s2_begin + s3_update + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_repack + s1_release diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 8c7a67ec3e1..0444bb53ace 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -1009,7 +1009,20 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, /* Write out any remaining tuples, and fsync if needed */ if (rwstate) - end_heap_rewrite(rwstate); + { + double tuples_dropped; + + tuples_dropped = end_heap_rewrite(rwstate); + + /* + * Tuples the rewrite dropped because their TOAST data was already + * reclaimed were all scanned as recently dead: count them as + * vacuumed instead. + */ + *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 c68333ed03f..467a9f8d089 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -149,6 +149,8 @@ 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 dropped because their + * TOAST data was already reclaimed */ HTAB *rs_logical_mappings; /* logical remapping files */ uint32 rs_num_rewrite_mappings; /* # in memory mappings */ } RewriteStateData; @@ -173,6 +175,7 @@ typedef struct TidHashKey key; /* expected xmin/old location of B tuple */ ItemPointerData old_tid; /* A's location in the old heap */ HeapTuple tuple; /* A's tuple contents */ + uint32 flags; /* raw_heap_insert flags A was scanned with */ } UnresolvedTupData; typedef UnresolvedTupData *UnresolvedTup; @@ -294,23 +297,31 @@ 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 tuples the rewrite dropped because their TOAST + * data had already been reclaimed. They were all scanned as recently + * dead, so the caller should count them as vacuumed rather than kept. */ -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 * left, they should in fact be dead, but let's err on the safe side. + * Each of them was scanned as recently dead, so write it with the flags + * it was scanned with and drop it if its TOAST data is gone. */ hash_seq_init(&seq_status, state->rs_unresolved_tups); while ((unresolved = hash_seq_search(&seq_status)) != NULL) { ItemPointerSetInvalid(&unresolved->tuple->t_data->t_ctid); - raw_heap_insert(state, unresolved->tuple, 0); + if (!raw_heap_insert(state, unresolved->tuple, unresolved->flags)) + state->rs_tuples_dropped += 1; } /* Write the last page, if any */ @@ -324,8 +335,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; } /* @@ -433,6 +448,7 @@ rewrite_heap_tuple(RewriteState state, unresolved->old_tid = old_tuple->t_self; unresolved->tuple = heap_copytuple(new_tuple); + unresolved->flags = flags; /* * We can't do anything more now, since we don't know where the @@ -459,10 +475,27 @@ 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, flags)) { - if (free_new) - heap_freetuple(new_tuple); + if (!free_new) + { + /* The caller's own tuple was skipped: report it */ + MemoryContextSwitchTo(old_cxt); + return false; + } + + /* + * A stashed prior version. The tuple this call was made for has + * already been written, so the failure cannot be reported to the + * caller; drop the stashed tuple here instead. Only tuples + * scanned as recently dead are stashed, so this is the same + * treatment the caller gives its own tuple, and the drop is + * counted for end_heap_rewrite() to report. If a still older + * version is waiting for this one in rs_unresolved_tups, it will + * be written out by end_heap_rewrite(). + */ + state->rs_tuples_dropped += 1; + heap_freetuple(new_tuple); MemoryContextSwitchTo(old_cxt); - return false; + return true; } new_tid = new_tuple->t_self; @@ -502,6 +535,7 @@ rewrite_heap_tuple(RewriteState state, if (free_new) heap_freetuple(new_tuple); new_tuple = unresolved->tuple; + flags = unresolved->flags; free_new = true; old_tid = unresolved->old_tid; new_tuple->t_data->t_ctid = new_tid; diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 09110f6337a..aba8dd462df 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