public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Imran Zaheer <[email protected]>
Cc: Srinath Reddy Sadipiralla <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19519: REPACK can fail due to missing chunk for toast value
Date: Mon, 6 Jul 2026 15:36:39 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+UBfan1QukfNDPvb2YpPG-ME4vUduBfbbe5uWptXDryZbtwVA@mail.gmail.com>
References: <[email protected]>
<CAFC+b6rLNvDzzYxrfLdq7CWbbouHcuOxi-mjEOW1r3qy2uAPMQ@mail.gmail.com>
<CA+UBfan1QukfNDPvb2YpPG-ME4vUduBfbbe5uWptXDryZbtwVA@mail.gmail.com>
On Fri, Jul 03, 2026 at 11:45:42AM +0500, Imran Zaheer wrote:
> Can we safely use the same fix in the index build path too? Can we use
> GetOldestNonRemovableTransactionIdforRewrite or something similar
> here? Normal serial index builds use SnapShotAny and concurrent index
> builds use MVCC, but the bug only exists in the serial index build
> path.
Yeah, I think that we should be able to safely reuse your ForRewrite()
API for the index build case (SnapshotAny part) as well to make the
horizon computations more conservative, exclusing one own XID as we
do in the lazy VACUUM case.
> Other than that, after applying your patch, the bug was not
> reproducible with either this repro or the other report's repro [1] in
> the rewrite path. However, the create index bug is still there. You
> can use the following repro as mentioned in the thread [1].
The assymetry between the global xmin computation and the per-database
horizon is what's killing us here. All these patterns are complicated
enough that they warrant some tests, even if it is necessary to have
multiple databases to trigger the buggy horizon computations. TAP
tests would be an obvious choice for that. Now I have a set of tricks
in my sleeves to make an isolation test fully deterministic:
- dblink() that opens a transaction to a different database than the
one of the isolation regression database.
- Rename of a TOAST table using allow_system_table_mods, for a VACUUM.
The second one is one of the dirtiest tricks I've done in the past for
a REINDEX CONCURRENTLY test with TOAST. Quite useful. If the backend
side change is reverted, the tests fail.
The attached includes both test suites for all four cases reported
(REPACK, VACUUM, CLUSTER, non-MVCC index builds). We'll need to
remove one of them, just keeping both posted as I am not sure if the
isolation tests would be entirely stable in the buildfarm..
I still need to dive more into the code, but for now this bit stands
out:
vacuum_get_cutoffs(OldHeap, ¶ms, &cutoffs);
+ /*
+ * vacuum_get_cutoffs() folds our own backend's xmin into OldestXmin. For
+ * a rewrite that is too conservative: the snapshot we hold exists only to
+ * evaluate index expressions against other relations, not to read
+ * OldHeap's historical rows. If our xmin is held back by a transaction
+ * that cannot even see OldHeap (e.g. one in another database), we would
+ * preserve a recently-dead tuple whose TOAST chunks a concurrent or prior
+ * lazy vacuum was free to remove, and then fail with "missing chunk" while
+ * copying it. Recompute OldestXmin ignoring our own backend so it matches
+ * the horizon lazy vacuum uses. This can only move OldestXmin forward, so
+ * the freeze cutoffs derived above remain valid.
+ */
+ cutoffs.OldestXmin = GetOldestNonRemovableTransactionIdForRewrite(OldHeap);
In copy_table_data() (for the data copy with repack, cluster, vacuum
full), I think that this is incorrect. For one, this breaks the
FreezeLimit which should always be older than OldestXmin, and this
patch enforces a new recomputation of OldestXmin ignoring most of the
internals of vacuum_get_cutoffs(). That's brittle, to say the least
if we change the way the vacuum cutoffs are calculated in the future.
Thoughts and comments from others are welcome for now. My day is
almost out, at least I got all these scenarios working some tests.
--
Michael
From 36fdec282e5c48866bf915f767502ccc3c6bc0b2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 6 Jul 2026 15:27:54 +0900
Subject: [PATCH v2] Fix "missing chunk" errors during heap rewrites and index
builds
When performing a heap rewrite (CLUSTER, VACUUM FULL, REPACK) or a
non-concurrent index build, the backend holds an MVCC snapshot whose
xmin can be held back by a transaction in another database, since
GetSnapshotData() considers XIDs from all databases.
However, ComputeXidHorizons() only considers same-database backends for
data_oldest_nonremovable. This asymmetry means a concurrent VACUUM
can compute a more aggressive horizon and remove TOAST chunks for a
tuple that the rewrite/index-build backend still considers as recently
dead, leading to "missing chunk number 0 for toast value" errors when
attempting to copy or index a tuple.
This issue is fixed by introducing a ForRewrite() equivalent of
GetOldestNonRemovableTransactionId(). It calls ComputeXidHorizons()
with a new option called "excludeMyself", mirroring how PROC_IN_VACUUM
excludes a lazy VACUUM own's XID to make rewrites more conservative with
the computation of the XID horizon.
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).
Srinath has authored the C portion of the patch, while I got some tests
working for the four problematic cases reported.
XXX: This patch includes both TAP and isolation tests. We should remove
one pattern at the end.
Reported-by: Alexander Lakhin <[email protected]>
Author: Srinath Reddy Sadipiralla <[email protected]>
Co-authored-by: Michael Paquier <[email protected]>
Reviewed-by: Imran Zaheer <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 14
---
src/include/storage/procarray.h | 1 +
src/backend/access/heap/heapam_handler.c | 2 +-
src/backend/commands/repack.c | 15 ++
src/backend/storage/ipc/procarray.c | 78 +++++-
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 ++++++++++
11 files changed, 596 insertions(+), 8 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/storage/procarray.h b/src/include/storage/procarray.h
index d718a5b542f0..ccfc31cdb5d1 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -51,6 +51,7 @@ extern RunningTransactions GetRunningTransactionData(void);
extern bool TransactionIdIsInProgress(TransactionId xid);
extern TransactionId GetOldestNonRemovableTransactionId(Relation rel);
+extern TransactionId GetOldestNonRemovableTransactionIdForRewrite(Relation rel);
extern TransactionId GetOldestTransactionIdConsideredRunning(void);
extern TransactionId GetOldestActiveTransactionId(bool inCommitOnly,
bool allDbs);
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 2268cc277bce..fb1540784236 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -1213,7 +1213,7 @@ heapam_index_build_range_scan(Relation heapRelation,
/* okay to ignore lazy VACUUMs here */
if (!IsBootstrapProcessingMode() && !indexInfo->ii_Concurrent)
- OldestXmin = GetOldestNonRemovableTransactionId(heapRelation);
+ OldestXmin = GetOldestNonRemovableTransactionIdForRewrite(heapRelation);
if (!scan)
{
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index faa07d1a118b..1585d67f0b34 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -71,6 +71,7 @@
#include "storage/lmgr.h"
#include "storage/predicate.h"
#include "storage/proc.h"
+#include "storage/procarray.h"
#include "utils/acl.h"
#include "utils/fmgroids.h"
#include "utils/guc.h"
@@ -1390,6 +1391,20 @@ copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex,
memset(¶ms, 0, sizeof(VacuumParams));
vacuum_get_cutoffs(OldHeap, ¶ms, &cutoffs);
+ /*
+ * vacuum_get_cutoffs() folds our own backend's xmin into OldestXmin. For
+ * a rewrite that is too conservative: the snapshot we hold exists only to
+ * evaluate index expressions against other relations, not to read
+ * OldHeap's historical rows. If our xmin is held back by a transaction
+ * that cannot even see OldHeap (e.g. one in another database), we would
+ * preserve a recently-dead tuple whose TOAST chunks a concurrent or prior
+ * lazy vacuum was free to remove, and then fail with "missing chunk"
+ * while copying it. Recompute OldestXmin ignoring our own backend so it
+ * matches the horizon lazy vacuum uses. This can only move OldestXmin
+ * forward, so the freeze cutoffs derived above remain valid.
+ */
+ cutoffs.OldestXmin = GetOldestNonRemovableTransactionIdForRewrite(OldHeap);
+
/*
* FreezeXid will become the table's new relfrozenxid, and that mustn't go
* backwards, so take the max.
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 60336b318036..a11c01eb9d42 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -1671,7 +1671,7 @@ TransactionIdIsInProgress(TransactionId xid)
* code doesn't expect (breaking HOT).
*/
static void
-ComputeXidHorizons(ComputeXidHorizonsResult *h)
+ComputeXidHorizons(ComputeXidHorizonsResult *h, bool excludeMyself)
{
ProcArrayStruct *arrayP = procArray;
TransactionId kaxmin;
@@ -1770,6 +1770,22 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
if (statusFlags & (PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING))
continue;
+ /*
+ * Optionally ignore our own backend's xmin. A relation rewrite holds
+ * an MVCC snapshot only so that index expressions can be evaluated
+ * against other relations; it never reads the historical rows of the
+ * relation being rewritten through that snapshot. Letting our own
+ * xmin hold back that relation's removal horizon makes the rewrite
+ * more conservative than the lazy vacuum that may already have
+ * removed a recently-dead tuple's TOAST chunks, which would then fail
+ * with "missing chunk" while copying the tuple. This mirrors how
+ * PROC_IN_VACUUM excludes a lazy vacuum's own xmin.
+ *
+ * See also GetOldestNonRemovableTransactionIdForRewrite().
+ */
+ if (excludeMyself && proc == MyProc)
+ continue;
+
/* shared tables need to take backends in all databases into account */
h->shared_oldest_nonremovable =
TransactionIdOlder(h->shared_oldest_nonremovable, xmin);
@@ -1898,8 +1914,15 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
TransactionIdPrecedesOrEquals(h->oldest_considered_running,
h->slot_catalog_xmin));
- /* update approximate horizons with the computed horizons */
- GlobalVisUpdateApply(h);
+ /*
+ * Update approximate horizons with the computed horizons. Skip this when
+ * excluding our own xmin: the resulting horizons are more aggressive than
+ * what is globally safe and are only valid for the caller's own rewrite
+ * of a single relation, so they must not be published to the shared
+ * GlobalVisState.
+ */
+ if (!excludeMyself)
+ GlobalVisUpdateApply(h);
}
/*
@@ -1945,7 +1968,48 @@ GetOldestNonRemovableTransactionId(Relation rel)
{
ComputeXidHorizonsResult horizons;
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
+
+ switch (GlobalVisHorizonKindForRel(rel))
+ {
+ case VISHORIZON_SHARED:
+ return horizons.shared_oldest_nonremovable;
+ case VISHORIZON_CATALOG:
+ return horizons.catalog_oldest_nonremovable;
+ case VISHORIZON_DATA:
+ return horizons.data_oldest_nonremovable;
+ case VISHORIZON_TEMP:
+ return horizons.temp_oldest_nonremovable;
+ }
+
+ /* just to prevent compiler warnings */
+ return InvalidTransactionId;
+}
+
+/*
+ * GetOldestNonRemovableTransactionIdForRewrite -- variant of
+ * GetOldestNonRemovableTransactionId() that ignores the calling backend's
+ * own xmin.
+ *
+ * A rewrite must preserve recently-dead tuples of the relation being
+ * rewritten so that other backends' snapshots still see them afterwards.
+ *
+ * It is not, however, a reader of that relation's
+ * historical rows itself: the MVCC snapshot it holds exists only to evaluate
+ * index expressions against *other* relations. Excluding our own xmin yields
+ * the same horizon a lazy vacuum would use (PROC_IN_VACUUM does the equivalent
+ * for lazy vacuum). That matters for TOAST relations to prevent the removal of
+ * recently-dead tuple's TOAST chunks.
+ *
+ * The caller must hold a lock on rel strong enough that the set of backends
+ * that could read its rows cannot change underneath us.
+ */
+TransactionId
+GetOldestNonRemovableTransactionIdForRewrite(Relation rel)
+{
+ ComputeXidHorizonsResult horizons;
+
+ ComputeXidHorizons(&horizons, true);
switch (GlobalVisHorizonKindForRel(rel))
{
@@ -1974,7 +2038,7 @@ GetOldestTransactionIdConsideredRunning(void)
{
ComputeXidHorizonsResult horizons;
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
return horizons.oldest_considered_running;
}
@@ -1987,7 +2051,7 @@ GetReplicationHorizons(TransactionId *xmin, TransactionId *catalog_xmin)
{
ComputeXidHorizonsResult horizons;
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
/*
* Don't want to use shared_oldest_nonremovable here, as that contains the
@@ -4214,7 +4278,7 @@ GlobalVisUpdate(void)
ComputeXidHorizonsResult horizons;
/* updates the horizons as a side-effect */
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
}
/*
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
Attachments:
[text/plain] v2-0001-Fix-missing-chunk-errors-during-heap-rewrites-and.patch (24.1K, ../[email protected]/2-v2-0001-Fix-missing-chunk-errors-during-heap-rewrites-and.patch)
download | inline diff:
From 36fdec282e5c48866bf915f767502ccc3c6bc0b2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 6 Jul 2026 15:27:54 +0900
Subject: [PATCH v2] Fix "missing chunk" errors during heap rewrites and index
builds
When performing a heap rewrite (CLUSTER, VACUUM FULL, REPACK) or a
non-concurrent index build, the backend holds an MVCC snapshot whose
xmin can be held back by a transaction in another database, since
GetSnapshotData() considers XIDs from all databases.
However, ComputeXidHorizons() only considers same-database backends for
data_oldest_nonremovable. This asymmetry means a concurrent VACUUM
can compute a more aggressive horizon and remove TOAST chunks for a
tuple that the rewrite/index-build backend still considers as recently
dead, leading to "missing chunk number 0 for toast value" errors when
attempting to copy or index a tuple.
This issue is fixed by introducing a ForRewrite() equivalent of
GetOldestNonRemovableTransactionId(). It calls ComputeXidHorizons()
with a new option called "excludeMyself", mirroring how PROC_IN_VACUUM
excludes a lazy VACUUM own's XID to make rewrites more conservative with
the computation of the XID horizon.
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).
Srinath has authored the C portion of the patch, while I got some tests
working for the four problematic cases reported.
XXX: This patch includes both TAP and isolation tests. We should remove
one pattern at the end.
Reported-by: Alexander Lakhin <[email protected]>
Author: Srinath Reddy Sadipiralla <[email protected]>
Co-authored-by: Michael Paquier <[email protected]>
Reviewed-by: Imran Zaheer <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 14
---
src/include/storage/procarray.h | 1 +
src/backend/access/heap/heapam_handler.c | 2 +-
src/backend/commands/repack.c | 15 ++
src/backend/storage/ipc/procarray.c | 78 +++++-
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 ++++++++++
11 files changed, 596 insertions(+), 8 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/storage/procarray.h b/src/include/storage/procarray.h
index d718a5b542f0..ccfc31cdb5d1 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -51,6 +51,7 @@ extern RunningTransactions GetRunningTransactionData(void);
extern bool TransactionIdIsInProgress(TransactionId xid);
extern TransactionId GetOldestNonRemovableTransactionId(Relation rel);
+extern TransactionId GetOldestNonRemovableTransactionIdForRewrite(Relation rel);
extern TransactionId GetOldestTransactionIdConsideredRunning(void);
extern TransactionId GetOldestActiveTransactionId(bool inCommitOnly,
bool allDbs);
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 2268cc277bce..fb1540784236 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -1213,7 +1213,7 @@ heapam_index_build_range_scan(Relation heapRelation,
/* okay to ignore lazy VACUUMs here */
if (!IsBootstrapProcessingMode() && !indexInfo->ii_Concurrent)
- OldestXmin = GetOldestNonRemovableTransactionId(heapRelation);
+ OldestXmin = GetOldestNonRemovableTransactionIdForRewrite(heapRelation);
if (!scan)
{
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index faa07d1a118b..1585d67f0b34 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -71,6 +71,7 @@
#include "storage/lmgr.h"
#include "storage/predicate.h"
#include "storage/proc.h"
+#include "storage/procarray.h"
#include "utils/acl.h"
#include "utils/fmgroids.h"
#include "utils/guc.h"
@@ -1390,6 +1391,20 @@ copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex,
memset(¶ms, 0, sizeof(VacuumParams));
vacuum_get_cutoffs(OldHeap, ¶ms, &cutoffs);
+ /*
+ * vacuum_get_cutoffs() folds our own backend's xmin into OldestXmin. For
+ * a rewrite that is too conservative: the snapshot we hold exists only to
+ * evaluate index expressions against other relations, not to read
+ * OldHeap's historical rows. If our xmin is held back by a transaction
+ * that cannot even see OldHeap (e.g. one in another database), we would
+ * preserve a recently-dead tuple whose TOAST chunks a concurrent or prior
+ * lazy vacuum was free to remove, and then fail with "missing chunk"
+ * while copying it. Recompute OldestXmin ignoring our own backend so it
+ * matches the horizon lazy vacuum uses. This can only move OldestXmin
+ * forward, so the freeze cutoffs derived above remain valid.
+ */
+ cutoffs.OldestXmin = GetOldestNonRemovableTransactionIdForRewrite(OldHeap);
+
/*
* FreezeXid will become the table's new relfrozenxid, and that mustn't go
* backwards, so take the max.
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 60336b318036..a11c01eb9d42 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -1671,7 +1671,7 @@ TransactionIdIsInProgress(TransactionId xid)
* code doesn't expect (breaking HOT).
*/
static void
-ComputeXidHorizons(ComputeXidHorizonsResult *h)
+ComputeXidHorizons(ComputeXidHorizonsResult *h, bool excludeMyself)
{
ProcArrayStruct *arrayP = procArray;
TransactionId kaxmin;
@@ -1770,6 +1770,22 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
if (statusFlags & (PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING))
continue;
+ /*
+ * Optionally ignore our own backend's xmin. A relation rewrite holds
+ * an MVCC snapshot only so that index expressions can be evaluated
+ * against other relations; it never reads the historical rows of the
+ * relation being rewritten through that snapshot. Letting our own
+ * xmin hold back that relation's removal horizon makes the rewrite
+ * more conservative than the lazy vacuum that may already have
+ * removed a recently-dead tuple's TOAST chunks, which would then fail
+ * with "missing chunk" while copying the tuple. This mirrors how
+ * PROC_IN_VACUUM excludes a lazy vacuum's own xmin.
+ *
+ * See also GetOldestNonRemovableTransactionIdForRewrite().
+ */
+ if (excludeMyself && proc == MyProc)
+ continue;
+
/* shared tables need to take backends in all databases into account */
h->shared_oldest_nonremovable =
TransactionIdOlder(h->shared_oldest_nonremovable, xmin);
@@ -1898,8 +1914,15 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
TransactionIdPrecedesOrEquals(h->oldest_considered_running,
h->slot_catalog_xmin));
- /* update approximate horizons with the computed horizons */
- GlobalVisUpdateApply(h);
+ /*
+ * Update approximate horizons with the computed horizons. Skip this when
+ * excluding our own xmin: the resulting horizons are more aggressive than
+ * what is globally safe and are only valid for the caller's own rewrite
+ * of a single relation, so they must not be published to the shared
+ * GlobalVisState.
+ */
+ if (!excludeMyself)
+ GlobalVisUpdateApply(h);
}
/*
@@ -1945,7 +1968,48 @@ GetOldestNonRemovableTransactionId(Relation rel)
{
ComputeXidHorizonsResult horizons;
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
+
+ switch (GlobalVisHorizonKindForRel(rel))
+ {
+ case VISHORIZON_SHARED:
+ return horizons.shared_oldest_nonremovable;
+ case VISHORIZON_CATALOG:
+ return horizons.catalog_oldest_nonremovable;
+ case VISHORIZON_DATA:
+ return horizons.data_oldest_nonremovable;
+ case VISHORIZON_TEMP:
+ return horizons.temp_oldest_nonremovable;
+ }
+
+ /* just to prevent compiler warnings */
+ return InvalidTransactionId;
+}
+
+/*
+ * GetOldestNonRemovableTransactionIdForRewrite -- variant of
+ * GetOldestNonRemovableTransactionId() that ignores the calling backend's
+ * own xmin.
+ *
+ * A rewrite must preserve recently-dead tuples of the relation being
+ * rewritten so that other backends' snapshots still see them afterwards.
+ *
+ * It is not, however, a reader of that relation's
+ * historical rows itself: the MVCC snapshot it holds exists only to evaluate
+ * index expressions against *other* relations. Excluding our own xmin yields
+ * the same horizon a lazy vacuum would use (PROC_IN_VACUUM does the equivalent
+ * for lazy vacuum). That matters for TOAST relations to prevent the removal of
+ * recently-dead tuple's TOAST chunks.
+ *
+ * The caller must hold a lock on rel strong enough that the set of backends
+ * that could read its rows cannot change underneath us.
+ */
+TransactionId
+GetOldestNonRemovableTransactionIdForRewrite(Relation rel)
+{
+ ComputeXidHorizonsResult horizons;
+
+ ComputeXidHorizons(&horizons, true);
switch (GlobalVisHorizonKindForRel(rel))
{
@@ -1974,7 +2038,7 @@ GetOldestTransactionIdConsideredRunning(void)
{
ComputeXidHorizonsResult horizons;
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
return horizons.oldest_considered_running;
}
@@ -1987,7 +2051,7 @@ GetReplicationHorizons(TransactionId *xmin, TransactionId *catalog_xmin)
{
ComputeXidHorizonsResult horizons;
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
/*
* Don't want to use shared_oldest_nonremovable here, as that contains the
@@ -4214,7 +4278,7 @@ GlobalVisUpdate(void)
ComputeXidHorizonsResult horizons;
/* updates the horizons as a side-effect */
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
}
/*
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
[application/pgp-signature] signature.asc (833B, ../[email protected]/3-signature.asc)
download
view thread (21+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19519: REPACK can fail due to missing chunk for toast value
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox