From 2e9cc62deda3de859ee389772471734a2f4f9e5d Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 7 Jul 2026 14:20:30 +0900 Subject: [PATCH v3] Fix "missing chunk" errors during heap rewrites and index builds Blah, blah. This problem has been reported recently by Alexander for REPACK, but it is a much older issue (tested down to 9.5 for curiosity's sake, but I suspect that it is much older than that). XXX: This patch includes both TAP and isolation tests. We should remove one pattern at the end. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/18351-f6e06364b3a2e669@postgresql.org Discussion: https://postgr.es/m/19519-fe02d8ff679d834d@postgresql.org --- src/include/access/detoast.h | 9 + src/include/access/heaptoast.h | 13 +- src/include/access/rewriteheap.h | 4 +- src/include/access/tableam.h | 24 +- src/include/access/toast_helper.h | 2 +- src/backend/access/common/detoast.c | 41 ++- src/backend/access/heap/heapam.c | 4 +- src/backend/access/heap/heapam_handler.c | 94 ++++++- src/backend/access/heap/heaptoast.c | 22 +- src/backend/access/heap/rewriteheap.c | 31 ++- src/backend/access/table/toast_helper.c | 17 +- src/test/recovery/meson.build | 1 + src/test/recovery/t/055_rewrite_stale_xmin.pl | 124 +++++++++ contrib/dblink/.gitignore | 2 + contrib/dblink/Makefile | 2 + .../dblink/expected/rewrite_stale_xmin.out | 237 ++++++++++++++++++ contrib/dblink/meson.build | 5 + contrib/dblink/specs/rewrite_stale_xmin.spec | 137 ++++++++++ 18 files changed, 718 insertions(+), 51 deletions(-) create mode 100644 src/test/recovery/t/055_rewrite_stale_xmin.pl create mode 100644 contrib/dblink/expected/rewrite_stale_xmin.out create mode 100644 contrib/dblink/specs/rewrite_stale_xmin.spec diff --git a/src/include/access/detoast.h b/src/include/access/detoast.h index fbd98181a3a1..c5eeed04b011 100644 --- a/src/include/access/detoast.h +++ b/src/include/access/detoast.h @@ -43,6 +43,15 @@ do { \ */ extern varlena *detoast_external_attr(varlena *attr); +/* ---------- + * detoast_external_attr_extended() - + * + * Like detoast_external_attr, but returns NULL if toast chunks + * are missing (instead of raising an error). + * ---------- + */ +extern varlena *detoast_external_attr_extended(varlena *attr); + /* ---------- * detoast_attr() - * diff --git a/src/include/access/heaptoast.h b/src/include/access/heaptoast.h index 631cb1836b96..1be85db6ddd2 100644 --- a/src/include/access/heaptoast.h +++ b/src/include/access/heaptoast.h @@ -92,10 +92,14 @@ * heap_toast_insert_or_update - * * Called by heap_insert() and heap_update(). + * + * If missing_ok is true, returns NULL when failing to fetch external + * TOAST data, instead of throwing an error. * ---------- */ extern HeapTuple heap_toast_insert_or_update(Relation rel, HeapTuple newtup, - HeapTuple oldtup, uint32 options); + HeapTuple oldtup, uint32 options, + bool missing_ok); /* ---------- * heap_toast_delete - @@ -140,10 +144,13 @@ extern HeapTuple toast_build_flattened_tuple(TupleDesc tupleDesc, * heap_fetch_toast_slice * * Fetch a slice from a toast value stored in a heap table. + * If missing_ok is true, returns false when chunks are missing + * instead of raising an error. Returns true on success. * ---------- */ -extern void heap_fetch_toast_slice(Relation toastrel, Oid valueid, +extern bool heap_fetch_toast_slice(Relation toastrel, Oid valueid, int32 attrsize, int32 sliceoffset, - int32 slicelength, varlena *result); + int32 slicelength, varlena *result, + bool missing_ok); #endif /* HEAPTOAST_H */ diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 6ccf7b45c04e..df4fa4fde710 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -25,8 +25,8 @@ 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 void rewrite_heap_tuple(RewriteState state, HeapTuple old_tuple, - HeapTuple new_tuple); +extern bool rewrite_heap_tuple(RewriteState state, HeapTuple old_tuple, + HeapTuple new_tuple, bool missing_ok); extern bool rewrite_heap_dead_tuple(RewriteState state, HeapTuple old_tuple); /* diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index f2c36696bcad..a90b15dd82b3 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -782,12 +782,16 @@ typedef struct TableAmRoutine * This callback is invoked when detoasting a value stored in a toast * table implemented by this AM. See table_relation_fetch_toast_slice() * for more details. + * + * If missing_ok is true, returns false when chunks are missing instead of + * raising an error. Returns true on success. */ - void (*relation_fetch_toast_slice) (Relation toastrel, Oid valueid, + bool (*relation_fetch_toast_slice) (Relation toastrel, Oid valueid, int32 attrsize, int32 sliceoffset, int32 slicelength, - varlena *result); + varlena *result, + bool missing_ok); /* ------------------------------------------------------------------------ @@ -1981,16 +1985,20 @@ table_relation_toast_am(Relation rel) * * result is caller-allocated space into which the fetched bytes should be * stored. + * + * missing_ok: if true, return false when toast chunks are missing instead + * of raising an error. Returns true on success. */ -static inline void +static inline bool table_relation_fetch_toast_slice(Relation toastrel, Oid valueid, int32 attrsize, int32 sliceoffset, - int32 slicelength, varlena *result) + int32 slicelength, varlena *result, + bool missing_ok) { - toastrel->rd_tableam->relation_fetch_toast_slice(toastrel, valueid, - attrsize, - sliceoffset, slicelength, - result); + return toastrel->rd_tableam->relation_fetch_toast_slice(toastrel, valueid, + attrsize, + sliceoffset, slicelength, + result, missing_ok); } diff --git a/src/include/access/toast_helper.h b/src/include/access/toast_helper.h index 2ec92397f267..e548ba5596b4 100644 --- a/src/include/access/toast_helper.h +++ b/src/include/access/toast_helper.h @@ -101,7 +101,7 @@ typedef struct #define TOASTCOL_IGNORE 0x0010 #define TOASTCOL_INCOMPRESSIBLE 0x0020 -extern void toast_tuple_init(ToastTupleContext *ttc); +extern bool toast_tuple_init(ToastTupleContext *ttc, bool missing_ok); extern int toast_tuple_find_biggest_attribute(ToastTupleContext *ttc, bool for_compression, bool check_main); diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c index a6c1f3a734b2..c16b61a1cb1c 100644 --- a/src/backend/access/common/detoast.c +++ b/src/backend/access/common/detoast.c @@ -22,7 +22,7 @@ #include "utils/expandeddatum.h" #include "utils/rel.h" -static varlena *toast_fetch_datum(varlena *attr); +static varlena *toast_fetch_datum(varlena *attr, bool missing_ok); static varlena *toast_fetch_datum_slice(varlena *attr, int32 sliceoffset, int32 slicelength); @@ -51,7 +51,7 @@ detoast_external_attr(varlena *attr) /* * This is an external stored plain value */ - result = toast_fetch_datum(attr); + result = toast_fetch_datum(attr, false); } else if (VARATT_IS_EXTERNAL_INDIRECT(attr)) { @@ -102,6 +102,23 @@ detoast_external_attr(varlena *attr) } +/* ---------- + * detoast_external_attr_extended - + * + * Like detoast_external_attr, but returns NULL if toast chunks are + * missing for external pointers. + * ---------- + */ +varlena * +detoast_external_attr_extended(varlena *attr) +{ + if (VARATT_IS_EXTERNAL_ONDISK(attr)) + return toast_fetch_datum(attr, true); + else + return detoast_external_attr(attr); +} + + /* ---------- * detoast_attr - * @@ -120,7 +137,7 @@ detoast_attr(varlena *attr) /* * This is an externally stored datum --- fetch it back from there */ - attr = toast_fetch_datum(attr); + attr = toast_fetch_datum(attr, false); /* If it's compressed, decompress it */ if (VARATT_IS_COMPRESSED(attr)) { @@ -262,7 +279,7 @@ detoast_attr_slice(varlena *attr, preslice = toast_fetch_datum_slice(attr, 0, max_size); } else - preslice = toast_fetch_datum(attr); + preslice = toast_fetch_datum(attr, false); } else if (VARATT_IS_EXTERNAL_INDIRECT(attr)) { @@ -337,10 +354,12 @@ detoast_attr_slice(varlena *attr, * * Reconstruct an in memory Datum from the chunks saved * in the toast relation + * + * If missing_ok is true, returns NULL when chunks are missing. * ---------- */ static varlena * -toast_fetch_datum(varlena *attr) +toast_fetch_datum(varlena *attr, bool missing_ok) { Relation toastrel; varlena *result; @@ -372,8 +391,14 @@ toast_fetch_datum(varlena *attr) toastrel = table_open(toast_pointer.va_toastrelid, AccessShareLock); /* Fetch all chunks */ - table_relation_fetch_toast_slice(toastrel, toast_pointer.va_valueid, - attrsize, 0, attrsize, result); + if (!table_relation_fetch_toast_slice(toastrel, toast_pointer.va_valueid, + attrsize, 0, attrsize, result, + missing_ok)) + { + pfree(result); + table_close(toastrel, AccessShareLock); + return NULL; + } /* Close toast table */ table_close(toastrel, AccessShareLock); @@ -454,7 +479,7 @@ toast_fetch_datum_slice(varlena *attr, int32 sliceoffset, /* Fetch all chunks */ table_relation_fetch_toast_slice(toastrel, toast_pointer.va_valueid, attrsize, sliceoffset, slicelength, - result); + result, false); /* Close toast table */ table_close(toastrel, AccessShareLock); diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index abfd8e8970a6..1d4018931200 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2236,7 +2236,7 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid, return tup; } else if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD) - return heap_toast_insert_or_update(relation, tup, NULL, options); + return heap_toast_insert_or_update(relation, tup, NULL, options, false); else return tup; } @@ -3871,7 +3871,7 @@ l2: if (need_toast) { /* Note we always use WAL and FSM during updates */ - heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0); + heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0, false); newtupsize = MAXALIGN(heaptup->t_len); } else diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 2268cc277bce..2cd9d0f4aeb9 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -19,6 +19,7 @@ */ #include "postgres.h" +#include "access/detoast.h" #include "access/genam.h" #include "access/heapam.h" #include "access/heaptoast.h" @@ -48,9 +49,10 @@ #include "utils/rel.h" #include "utils/tuplesort.h" -static void reform_and_rewrite_tuple(HeapTuple tuple, +static bool reform_and_rewrite_tuple(HeapTuple tuple, Relation OldHeap, Relation NewHeap, - Datum *values, bool *isnull, RewriteState rwstate); + Datum *values, bool *isnull, RewriteState rwstate, + bool missing_ok); static void heap_insert_for_repack(HeapTuple tuple, Relation OldHeap, Relation NewHeap, Datum *values, bool *isnull, BulkInsertState bistate); @@ -717,6 +719,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, HeapTuple tuple; Buffer buf; bool isdead; + bool recently_dead = false; CHECK_FOR_INTERRUPTS(); @@ -802,6 +805,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, break; case HEAPTUPLE_RECENTLY_DEAD: *tups_recently_dead += 1; + recently_dead = true; pg_fallthrough; case HEAPTUPLE_LIVE: /* Live or recently dead, must copy it */ @@ -835,6 +839,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, RelationGetRelationName(OldHeap)); /* treat as recently dead */ *tups_recently_dead += 1; + recently_dead = true; isdead = false; break; default: @@ -881,8 +886,22 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, int64 ct_val[2]; if (!concurrent) - reform_and_rewrite_tuple(tuple, OldHeap, NewHeap, - values, isnull, rwstate); + { + if (!reform_and_rewrite_tuple(tuple, OldHeap, NewHeap, + values, isnull, rwstate, + recently_dead)) + { + /* + * Missing TOAST chunks for a recently-dead tuple. Treat + * it as dead. + */ + *tups_vacuumed += 1; + *num_tuples -= 1; + if (recently_dead) + *tups_recently_dead -= 1; + continue; + } + } else heap_insert_for_repack(tuple, OldHeap, NewHeap, values, isnull, bistate); @@ -934,10 +953,17 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, n_tuples += 1; if (!concurrent) - reform_and_rewrite_tuple(tuple, - OldHeap, NewHeap, - values, isnull, - rwstate); + { + if (!reform_and_rewrite_tuple(tuple, + OldHeap, NewHeap, + values, isnull, + rwstate, true)) + { + /* TOAST gone for a recently dead tuple */ + n_tuples -= 1; + continue; + } + } else heap_insert_for_repack(tuple, OldHeap, NewHeap, values, isnull, bistate); @@ -1613,6 +1639,43 @@ heapam_index_build_range_scan(Relation heapRelation, continue; } + /* + * For RECENTLY_DEAD tuples, pre-detoast external TOAST values. We do + * this to detect for missing chunks at later stages, feeding inline + * data to the downstream access method code. + */ + if (!tupleIsAlive) + { + TupleDesc tdesc = RelationGetDescr(heapRelation); + bool skip = false; + + slot_getallattrs(slot); + for (int i = 0; i < tdesc->natts; i++) + { + Form_pg_attribute att = TupleDescAttr(tdesc, i); + + if (att->attlen != -1 || att->attisdropped) + continue; + if (slot->tts_isnull[i]) + continue; + if (VARATT_IS_EXTERNAL_ONDISK(DatumGetPointer(slot->tts_values[i]))) + { + varlena *detoasted; + + detoasted = detoast_external_attr_extended( + (varlena *) DatumGetPointer(slot->tts_values[i])); + if (detoasted == NULL) + { + skip = true; + break; + } + slot->tts_values[i] = PointerGetDatum(detoasted); + } + } + if (skip) + continue; + } + /* * For the current heap tuple, extract all the attributes we use in * this index, and note which are null. This also performs evaluation @@ -2347,20 +2410,29 @@ heapam_scan_sample_next_tuple(TableScanDesc scan, SampleScanState *scanstate, * SET WITHOUT OIDS. * * So, we must reconstruct the tuple from component Datums. + * + * Returns true on success, and false on failure if missing_ok is + * true. */ -static void +static bool reform_and_rewrite_tuple(HeapTuple tuple, Relation OldHeap, Relation NewHeap, - Datum *values, bool *isnull, RewriteState rwstate) + Datum *values, bool *isnull, RewriteState rwstate, + bool missing_ok) { HeapTuple newtuple; newtuple = reform_tuple(tuple, OldHeap, NewHeap, values, isnull); /* The heap rewrite module does the rest */ - rewrite_heap_tuple(rwstate, tuple, newtuple); + if (!rewrite_heap_tuple(rwstate, tuple, newtuple, missing_ok)) + { + heap_freetuple(newtuple); + return false; + } heap_freetuple(newtuple); + return true; } /* diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c index 03f885a25b07..38896a0c7492 100644 --- a/src/backend/access/heap/heaptoast.c +++ b/src/backend/access/heap/heaptoast.c @@ -94,7 +94,7 @@ heap_toast_delete(Relation rel, HeapTuple oldtup, bool is_speculative) */ HeapTuple heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup, - uint32 options) + uint32 options, bool missing_ok) { HeapTuple result_tuple; TupleDesc tupleDesc; @@ -154,7 +154,8 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup, ttc.ttc_oldisnull = toast_oldisnull; } ttc.ttc_attr = toast_attr; - toast_tuple_init(&ttc); + if (!toast_tuple_init(&ttc, missing_ok)) + return NULL; /* ---------- * Compress and/or save external until data fits into target length @@ -621,11 +622,14 @@ toast_build_flattened_tuple(TupleDesc tupleDesc, * sliceoffset is the byte offset within the TOAST value from which to fetch. * slicelength is the number of bytes to be fetched from the TOAST value. * result is the varlena into which the results should be written. + * missing_ok if true, return false on missing chunks instead of error. + * + * Returns true on success, false if missing_ok and chunks are missing. */ -void +bool heap_fetch_toast_slice(Relation toastrel, Oid valueid, int32 attrsize, int32 sliceoffset, int32 slicelength, - varlena *result) + varlena *result, bool missing_ok) { Relation *toastidxs; ScanKeyData toastkey[3]; @@ -779,13 +783,23 @@ heap_fetch_toast_slice(Relation toastrel, Oid valueid, int32 attrsize, * Final checks that we successfully fetched the datum */ if (expectedchunk != (endchunk + 1)) + { + if (missing_ok) + { + systable_endscan_ordered(toastscan); + toast_close_indexes(toastidxs, num_indexes, AccessShareLock); + return false; + } ereport(ERROR, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("missing chunk number %d for toast value %u in %s", expectedchunk, valueid, RelationGetRelationName(toastrel)))); + } /* End scan and close indexes. */ systable_endscan_ordered(toastscan); toast_close_indexes(toastidxs, num_indexes, AccessShareLock); + + return true; } diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 5a5398a76ae7..fbb6e6ca1f94 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -211,7 +211,8 @@ typedef struct RewriteMappingDataEntry /* prototypes for internal functions */ -static void raw_heap_insert(RewriteState state, HeapTuple tup); +static bool raw_heap_insert(RewriteState state, HeapTuple tup, + bool missing_ok); /* internal logical remapping prototypes */ static void logical_begin_heap_rewrite(RewriteState state); @@ -309,7 +310,7 @@ 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); + raw_heap_insert(state, unresolved->tuple, false); } /* Write the last page, if any */ @@ -338,9 +339,10 @@ end_heap_rewrite(RewriteState state) * old_tuple original tuple in the old heap * new_tuple new, rewritten tuple to be inserted to new heap */ -void +bool rewrite_heap_tuple(RewriteState state, - HeapTuple old_tuple, HeapTuple new_tuple) + HeapTuple old_tuple, HeapTuple new_tuple, + bool missing_ok) { MemoryContext old_cxt; ItemPointerData old_tid; @@ -437,7 +439,7 @@ rewrite_heap_tuple(RewriteState state, * tuple will be written. */ MemoryContextSwitchTo(old_cxt); - return; + return true; } } @@ -455,7 +457,13 @@ rewrite_heap_tuple(RewriteState state, ItemPointerData new_tid; /* Insert the tuple and find out where it's put in new_heap */ - raw_heap_insert(state, new_tuple); + if (!raw_heap_insert(state, new_tuple, missing_ok)) + { + if (free_new) + heap_freetuple(new_tuple); + MemoryContextSwitchTo(old_cxt); + return false; + } new_tid = new_tuple->t_self; logical_rewrite_heap_tuple(state, old_tid, new_tuple); @@ -532,6 +540,7 @@ rewrite_heap_tuple(RewriteState state, } MemoryContextSwitchTo(old_cxt); + return true; } /* @@ -593,8 +602,8 @@ rewrite_heap_dead_tuple(RewriteState state, HeapTuple old_tuple) * tuple is invalid on entry, it's replaced with the new TID as well (in * the inserted data only, not in the caller's copy). */ -static void -raw_heap_insert(RewriteState state, HeapTuple tup) +static bool +raw_heap_insert(RewriteState state, HeapTuple tup, bool missing_ok) { Page page; Size pageFreeSpace, @@ -628,7 +637,9 @@ raw_heap_insert(RewriteState state, HeapTuple tup) options |= HEAP_INSERT_NO_LOGICAL; heaptup = heap_toast_insert_or_update(state->rs_new_rel, tup, NULL, - options); + options, missing_ok); + if (heaptup == NULL) + return false; } else heaptup = tup; @@ -702,6 +713,8 @@ raw_heap_insert(RewriteState state, HeapTuple tup) /* If heaptup is a private copy, release it. */ if (heaptup != tup) heap_freetuple(heaptup); + + return true; } /* ------------------------------------------------------------------------ diff --git a/src/backend/access/table/toast_helper.c b/src/backend/access/table/toast_helper.c index 2f2022d99510..7e98c01a5eee 100644 --- a/src/backend/access/table/toast_helper.c +++ b/src/backend/access/table/toast_helper.c @@ -36,9 +36,12 @@ * toast_flags is just a single uint8, but toast_attr is a caller-provided * array with a length equal to tupleDesc->natts. The caller need not * perform any initialization of the array before calling this function. + * + * If missing_ok is true and an external TOAST value cannot be fetched, + * returns false. Returns true on success. */ -void -toast_tuple_init(ToastTupleContext *ttc) +bool +toast_tuple_init(ToastTupleContext *ttc, bool missing_ok) { TupleDesc tupleDesc = ttc->ttc_rel->rd_att; int numAttrs = tupleDesc->natts; @@ -137,7 +140,13 @@ toast_tuple_init(ToastTupleContext *ttc) if (VARATT_IS_EXTERNAL(new_value)) { ttc->ttc_attr[i].tai_oldexternal = new_value; - if (att->attstorage == TYPSTORAGE_PLAIN) + if (missing_ok && VARATT_IS_EXTERNAL_ONDISK(new_value)) + { + new_value = detoast_external_attr_extended(new_value); + if (new_value == NULL) + return false; + } + else if (att->attstorage == TYPSTORAGE_PLAIN) new_value = detoast_attr(new_value); else new_value = detoast_external_attr(new_value); @@ -159,6 +168,8 @@ toast_tuple_init(ToastTupleContext *ttc) ttc->ttc_attr[i].tai_colflags |= TOASTCOL_IGNORE; } } + + return true; } /* diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index ad0d85f41897..7e583ee39a10 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -63,6 +63,7 @@ tests += { 't/052_checkpoint_segment_missing.pl', 't/053_standby_login_event_trigger.pl', 't/054_unlogged_sequence_promotion.pl', + 't/055_rewrite_stale_xmin.pl', ], }, } diff --git a/src/test/recovery/t/055_rewrite_stale_xmin.pl b/src/test/recovery/t/055_rewrite_stale_xmin.pl new file mode 100644 index 000000000000..dad95c0cf967 --- /dev/null +++ b/src/test/recovery/t/055_rewrite_stale_xmin.pl @@ -0,0 +1,124 @@ +# Copyright (c) 2025-2026, PostgreSQL Global Development Group +# +# Test that heap rewrites (CLUSTER, VACUUM FULL, REPACK) and index builds +# work in the case when the backend's own xmin is held back by a transaction +# in another database. + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('primary'); +$node->init; +# No Autovacuum, no random interferences. +$node->append_conf( + 'postgresql.conf', qq[ +autovacuum = off +]); +$node->start; + +# Create two databases. +$node->safe_psql('postgres', 'CREATE DATABASE db_holder'); +$node->safe_psql('postgres', 'CREATE DATABASE db_target'); + +# Set up a table with TOAST data. Returns the toast table name. +sub setup_table +{ + my ($tblname, $extra_ddl) = @_; + $extra_ddl //= ''; + + $node->safe_psql( + 'db_target', qq{ + DROP TABLE IF EXISTS $tblname; + CREATE TABLE $tblname (id int PRIMARY KEY, data text) + WITH (autovacuum_enabled = off); + ALTER TABLE $tblname ALTER COLUMN data SET STORAGE EXTERNAL; + INSERT INTO $tblname VALUES (1, repeat('x', 2500)); + INSERT INTO $tblname VALUES (2, repeat('z', 2500)); + $extra_ddl + }); + + my $toast = $node->safe_psql( + 'db_target', qq{ + SELECT 'pg_toast.' || relname FROM pg_class + WHERE oid = (SELECT reltoastrelid FROM pg_class + WHERE relname = '$tblname'); + }); + chomp $toast; + return $toast; +} + +# Create precondition state. Requires that $holder is already running with +# a write XID. +sub create_missing_toast_state +{ + my ($tblname, $toast_table) = @_; + + # Hold xmin in same DB to prevent main tuple removal. + my $xmin_holder = $node->background_psql('db_target', on_error_stop => 1); + $xmin_holder->query_safe("BEGIN ISOLATION LEVEL REPEATABLE READ"); + $xmin_holder->query_safe("SELECT 1"); + + # Delete the row with TOAST data. + $node->safe_psql('db_target', "DELETE FROM $tblname WHERE id = 1"); + + # VACUUM main table only. The xmin_holder keeps the dead tuple. + $node->safe_psql('db_target', "VACUUM (PROCESS_TOAST false) $tblname"); + + # Release same-DB xmin. + $xmin_holder->query_safe("COMMIT"); + $xmin_holder->quit; + + # Vacuum toast table: toast chunks now DEAD and removed. + $node->safe_psql('db_target', "VACUUM $toast_table"); +} + +# Open a transaction holding an XID before any deletes. This XID will hold +# back GetSnapshotData() globally, on a different database than the one +# running the various rewrite operations. +my $holder = $node->background_psql('db_holder', on_error_stop => 1); +$holder->query_safe("BEGIN"); +$holder->query_safe("CREATE TEMP TABLE xid_holder(x int)"); + +# Test 1: CLUSTER +my $toast = setup_table('rewrite_test'); +create_missing_toast_state('rewrite_test', $toast); + +my ($ret, $stdout, $stderr) = + $node->psql('db_target', 'CLUSTER rewrite_test USING rewrite_test_pkey'); +is($ret, 0, "CLUSTER succeeds without missing-chunk error"); +unlike($stderr, qr/missing chunk/, "CLUSTER: no missing-chunk error"); + +# Test 2: VACUUM FULL +$toast = setup_table('rewrite_test'); +create_missing_toast_state('rewrite_test', $toast); + +($ret, $stdout, $stderr) = + $node->psql('db_target', 'VACUUM FULL rewrite_test'); +is($ret, 0, "VACUUM FULL succeeds without missing-chunk error"); +unlike($stderr, qr/missing chunk/, "VACUUM FULL: no missing-chunk error"); + +# Test 3: REPACK +$toast = setup_table('rewrite_test'); +create_missing_toast_state('rewrite_test', $toast); + +($ret, $stdout, $stderr) = $node->psql('db_target', 'REPACK rewrite_test'); +is($ret, 0, "REPACK succeeds without missing-chunk error"); +unlike($stderr, qr/missing chunk/, "REPACK: no missing-chunk error"); + +# Test 4: CREATE INDEX on toasted column +$toast = setup_table('rewrite_test'); +create_missing_toast_state('rewrite_test', $toast); + +($ret, $stdout, $stderr) = $node->psql('db_target', + 'CREATE INDEX rewrite_test_data_idx ON rewrite_test (data)'); +is($ret, 0, "CREATE INDEX succeeds without missing-chunk error"); +unlike($stderr, qr/missing chunk/, "CREATE INDEX: no missing-chunk error"); + +# Cleanup. +$holder->query_safe("COMMIT"); +$holder->quit; + +$node->stop; +done_testing(); diff --git a/contrib/dblink/.gitignore b/contrib/dblink/.gitignore index 5dcb3ff97235..33bc8d8ffeb3 100644 --- a/contrib/dblink/.gitignore +++ b/contrib/dblink/.gitignore @@ -2,3 +2,5 @@ /log/ /results/ /tmp_check/ +/output_iso/ +/tmp_check_iso/ diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile index fde0b49ddbbd..a0ba41280813 100644 --- a/contrib/dblink/Makefile +++ b/contrib/dblink/Makefile @@ -13,6 +13,8 @@ PGFILEDESC = "dblink - connect to other PostgreSQL databases" REGRESS = dblink REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress +ISOLATION = rewrite_stale_xmin +ISOLATION_OPTS = --load-extension=dblink TAP_TESTS = 1 ifdef USE_PGXS diff --git a/contrib/dblink/expected/rewrite_stale_xmin.out b/contrib/dblink/expected/rewrite_stale_xmin.out new file mode 100644 index 000000000000..2e5a7940ea26 --- /dev/null +++ b/contrib/dblink/expected/rewrite_stale_xmin.out @@ -0,0 +1,237 @@ +Parsed test spec with 3 sessions + +starting permutation: s1_hold_xmin s2_begin s3_delete s3_vacuum_main_only s2_commit s3_vacuum_toast s3_cluster 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_delete: + DELETE FROM rewrite_test 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_cluster: + CLUSTER rewrite_test USING rewrite_test_pkey; + +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_delete 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_delete: + DELETE FROM rewrite_test 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_delete 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_delete: + DELETE FROM rewrite_test 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) + + +starting permutation: s1_hold_xmin s2_begin s3_delete s3_vacuum_main_only s2_commit s3_vacuum_toast s3_create_index 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_delete: + DELETE FROM rewrite_test 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_create_index: + CREATE INDEX rewrite_test_data_idx ON rewrite_test (data); + +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/meson.build b/contrib/dblink/meson.build index e2489f41229f..86fbd372b880 100644 --- a/contrib/dblink/meson.build +++ b/contrib/dblink/meson.build @@ -36,6 +36,11 @@ tests += { ], 'regress_args': ['--dlpath', meson.project_build_root() / 'src/test/regress'], }, + 'isolation': { + 'specs': [ + 'rewrite_stale_xmin', + ], + }, 'tap': { 'tests': [ 't/001_auth_scram.pl', diff --git a/contrib/dblink/specs/rewrite_stale_xmin.spec b/contrib/dblink/specs/rewrite_stale_xmin.spec new file mode 100644 index 000000000000..fe88e789f854 --- /dev/null +++ b/contrib/dblink/specs/rewrite_stale_xmin.spec @@ -0,0 +1,137 @@ +# Test that heap rewrites work when the rewriting backend's own xmin is +# held back by a transaction in another database. + +setup +{ + CREATE DATABASE regress_other_db; +} + +setup +{ + CREATE EXTENSION IF NOT EXISTS dblink; + + CREATE TABLE rewrite_test (id int PRIMARY KEY, data text) + WITH (autovacuum_enabled = off); + ALTER TABLE rewrite_test ALTER COLUMN data SET STORAGE EXTERNAL; + INSERT INTO rewrite_test VALUES (1, repeat('x', 2500)); + INSERT INTO rewrite_test VALUES (2, repeat('y', 2500)); + + -- Rename the toast table to a known name so we can VACUUM it directly. + SET allow_system_table_mods TO true; + DO $$DECLARE r record; + BEGIN + SELECT INTO r reltoastrelid::regclass::text AS table_name + FROM pg_class WHERE oid = 'rewrite_test'::regclass; + EXECUTE 'ALTER TABLE ' || r.table_name || ' RENAME TO rewrite_test_toast;'; + END$$; + RESET allow_system_table_mods; +} + +teardown +{ + DROP DATABASE regress_other_db; +} + +# Session s1: use dblink to hold a transaction in another database. +# This must start before the DELETE so its XID is older, ensuring +# GetSnapshotData() holds back any rewrites. +session s1 +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)'); +} +step s1_release +{ + SELECT dblink_exec('holder', 'COMMIT'); + SELECT dblink_disconnect('holder'); +} + +# Session s2: holds xmin in the SAME database temporarily, so the +# main-table vacuum preserves the dead tuple as RECENTLY_DEAD. +session s2 +step s2_begin +{ + BEGIN ISOLATION LEVEL REPEATABLE READ; + SELECT 1; +} +step s2_commit { COMMIT; } + +# Session s3: performs DML, split vacuum, and the rewrite. +session s3 +step s3_delete +{ + DELETE FROM rewrite_test WHERE id = 1; +} +step s3_vacuum_main_only +{ + VACUUM (PROCESS_TOAST false) rewrite_test; +} +step s3_vacuum_toast +{ + VACUUM pg_toast.rewrite_test_toast; +} +step s3_cluster +{ + CLUSTER rewrite_test USING rewrite_test_pkey; +} +step s3_vacuum_full +{ + VACUUM FULL rewrite_test; +} +step s3_repack +{ + REPACK rewrite_test; +} +step s3_create_index +{ + CREATE INDEX rewrite_test_data_idx ON rewrite_test (data); +} + +teardown { DROP TABLE IF EXISTS rewrite_test; } + +# CLUSTER +permutation + s1_hold_xmin + s2_begin + s3_delete + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_cluster + s1_release + +# VACUUM FULL +permutation + s1_hold_xmin + s2_begin + s3_delete + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_vacuum_full + s1_release + +# REPACK +permutation + s1_hold_xmin + s2_begin + s3_delete + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_repack + s1_release + +# CREATE INDEX +permutation + s1_hold_xmin + s2_begin + s3_delete + s3_vacuum_main_only + s2_commit + s3_vacuum_toast + s3_create_index + s1_release -- 2.55.0