agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedREPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
28+ messages / 8 participants
[nested] [flat]
* REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
@ 2026-09-23 00:23 Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 15:06 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Melanie Plageman <melanieplageman@gmail.com>
0 siblings, 2 replies; 28+ messages in thread
From: Thom Brown @ 2026-09-23 00:23 UTC (permalink / raw)
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Hi,
Whilst stress-testing REPACK (CONCURRENTLY) I managed to get it to silently
throw away committed updates to a TOASTed column. There's no error, and both
verify_heapam() and bt_index_check() seem to think everything is fine.
I had claude analyse what's going on and:
-------
It happens at the very start of REPACK. The decoding worker grabs the
toast relation's relfilenode in repack_setup_logical_decoding() and immediately
lets go of the lock, but the backend doesn't lock the toast table until
copy_table_data(). In between sits get_initial_snapshot(), which waits for all
running XIDs to finish, so an open transaction widens the gap nicely. During
that gap you can rewrite the toast table by name (VACUUM FULL / REPACK /
CLUSTER of it) and give it a fresh relfilenode - the parent is only held with
ShareUpdateExclusiveLock, so that's allowed.
-------
Reproducible steps:
-- session A: hold a transaction open so REPACK blocks building its snapshot
BEGIN;
SELECT pg_current_xact_id();
-- session B: set up, arm the one stock injection point, then REPACK (blocks)
CREATE TABLE test (id int PRIMARY KEY, big text);
ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL;
INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g;
SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
REPACK (CONCURRENTLY) test;
-- session C: rewrite the toast table in the gap, then let A commit
VACUUM FULL pg_toast.pg_toast_<oid-of-test>;
-- session A:
COMMIT;
-- session C: update the toasted rows
UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3);
SELECT id, left(big,9) AS val, length(big) FROM test ORDER BY id;
id | val | length
----+-----------+--------
1 | NEWNEWNEW | 12000
2 | NEWNEWNEW | 12000
3 | NEWNEWNEW | 12000
-- session C: let REPACK finish, then look again
SELECT injection_points_wakeup('repack-concurrently-before-lock');
SELECT id, left(big,9) AS val, length(big) FROM test ORDER BY id;
id | val | length
----+-----------+--------
1 | oldoldold | 9000
2 | oldoldold | 9000
3 | oldoldold | 9000
SELECT * FROM verify_heapam('t', check_toast => true); -- 0 rows
SELECT bt_index_check('t_pkey', heapallindexed => true); -- passes
So three committed updates have quietly reverted to their old values.
Regards
Thom
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
@ 2026-09-23 00:42 ` Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
1 sibling, 1 reply; 28+ messages in thread
From: Manu @ 2026-09-23 00:42 UTC (permalink / raw)
To: Thom Brown <thom@linux.com>; +Cc: pgsql-hackers@lists.postgresql.org
Hi Thom,
Reproduced here on master (07c73f45063), Linux, following your recipe
exactly: the three rows read NEWNEWNEW/12000 before REPACK finishes and
oldoldold/9000 after it, no error anywhere.
One thing worth adding, because it changes how this reads: the
injection point is not needed.
It makes the race deterministic, but the window your analysis describes
- between repack_setup_logical_decoding() taking the toast relfilenode
and copy_table_data() locking the toast table - is wide enough on its
own, because get_initial_snapshot() sits in it waiting for running
XIDs. An ordinary open transaction is enough to hold it open. Without
any injection point:
session A BEGIN; SELECT pg_current_xact_id(); (sleeps 3s, commits)
session B REPACK (CONCURRENTLY) test; -- waits for A
session C VACUUM FULL pg_toast.pg_toast_<oid>; -- inside the gap
session C UPDATE test SET big = ... ; -- right after A commits
5 of 5 runs lost the update.
Script attached; it builds and drops the table on each iteration so the
result is not an artifact of one particular relfilenode.
So this does not need a debug build or a test-only feature to happen.
It needs a long-running transaction, a REPACK (CONCURRENTLY), and
someone rewriting that table's toast relation in the meantime - which
is an odd thing to do by hand, but it is allowed, the parent is only
held with ShareUpdateExclusiveLock as you say, and "run VACUUM FULL on
the biggest toast tables" is the kind of thing maintenance scripts do.
I have not looked for a fix yet. From your description the obvious
question is whether repack_setup_logical_decoding() should hold the
toast lock until copy_table_data() takes it, or whether the relfilenode
should be re-checked after the snapshot is built; the second sounds
cheaper but I have not read enough of that path to have an opinion
worth posting.
Regards,
Manu
#!/usr/bin/env bash
# ?Hace falta el injection point, o la carrera se gana sola?
#
# Con el punto de inyeccion el bug sale siempre, porque REPACK queda detenido
# justo antes de tomar el lock. Sin el, hay que meter el UPDATE en la ventana
# que va desde que la transaccion vieja commitea hasta que REPACK copia la
# tabla. Aca se intenta N veces, con el UPDATE disparado inmediatamente
# despues del COMMIT, sin esperas.
set -u
B=/home/manu/pgprog/i-serie
D=/home/manu/pgprog/data_carrera
P=55592
N=${N:-5}
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
rm -rf "$D"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<'EOF'
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l /home/manu/pgprog/carrera.log -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
ganadas=0
for i in $(seq 1 $N); do
q "DROP TABLE IF EXISTS test" >/dev/null
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
# A: transaccion abierta, se cierra sola a los 3s
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep(3)" -c "COMMIT" >/dev/null 2>&1 ) &
sleep 0.5
# B: REPACK, que se va a quedar esperando a que A termine
( q "REPACK (CONCURRENTLY) test" >/dev/null 2>&1 ) &
# C: reescribir la toast mientras REPACK espera
sleep 1
q "VACUUM FULL $TOAST" >/dev/null 2>&1
# esperar a que A commitee y disparar el UPDATE lo antes posible
wait %1 2>/dev/null
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null 2>&1
wait 2>/dev/null
res=$(q "SELECT DISTINCT left(big,9) FROM test")
if [ "$res" = "oldoldold" ]; then
ganadas=$((ganadas+1)); echo " intento $i: UPDATE PERDIDO"
else
echo " intento $i: los updates sobrevivieron ($res)"
fi
done
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
echo
echo "sin injection point: $ganadas de $N intentos perdieron el update"
Attachments:
[text/plain] nocfbot-repack-toast-race.sh.txt (2.3K, ../../179012413951.1850281.5077495683381671561@gmail.com/2-nocfbot-repack-toast-race.sh.txt)
download | inline:
#!/usr/bin/env bash
# ?Hace falta el injection point, o la carrera se gana sola?
#
# Con el punto de inyeccion el bug sale siempre, porque REPACK queda detenido
# justo antes de tomar el lock. Sin el, hay que meter el UPDATE en la ventana
# que va desde que la transaccion vieja commitea hasta que REPACK copia la
# tabla. Aca se intenta N veces, con el UPDATE disparado inmediatamente
# despues del COMMIT, sin esperas.
set -u
B=/home/manu/pgprog/i-serie
D=/home/manu/pgprog/data_carrera
P=55592
N=${N:-5}
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
rm -rf "$D"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<'EOF'
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l /home/manu/pgprog/carrera.log -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
ganadas=0
for i in $(seq 1 $N); do
q "DROP TABLE IF EXISTS test" >/dev/null
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
# A: transaccion abierta, se cierra sola a los 3s
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep(3)" -c "COMMIT" >/dev/null 2>&1 ) &
sleep 0.5
# B: REPACK, que se va a quedar esperando a que A termine
( q "REPACK (CONCURRENTLY) test" >/dev/null 2>&1 ) &
# C: reescribir la toast mientras REPACK espera
sleep 1
q "VACUUM FULL $TOAST" >/dev/null 2>&1
# esperar a que A commitee y disparar el UPDATE lo antes posible
wait %1 2>/dev/null
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null 2>&1
wait 2>/dev/null
res=$(q "SELECT DISTINCT left(big,9) FROM test")
if [ "$res" = "oldoldold" ]; then
ganadas=$((ganadas+1)); echo " intento $i: UPDATE PERDIDO"
else
echo " intento $i: los updates sobrevivieron ($res)"
fi
done
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
echo
echo "sin injection point: $ganadas de $N intentos perdieron el update"
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
@ 2026-09-23 05:09 ` shihao zhong <zhong950419@gmail.com>
2026-09-23 08:26 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
0 siblings, 2 replies; 28+ messages in thread
From: shihao zhong @ 2026-09-23 05:09 UTC (permalink / raw)
To: Manu <manuelreyesbravo@gmail.com>; +Cc: Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
> or whether the relfilenode should be re-checked after the snapshot is
built
Holding the toast lock from the start deadlocks. A session that asks for
AccessExclusiveLock gets an XID before it waits, and the decoding worker
waits for all XIDs while it sets up.
So the attached patch re-checks instead. Once the worker is set up it no
longer waits for anyone, so the backend locks the toast table there and
compares its relfilenode with the one the worker uses. If they differ, it
starts a new worker. Nothing has been copied yet, so REPACK just carries on.
0002 adds a test to repack_toast.spec that fails without 0001.
optional.
Thanks,
Shihao
Attachments:
[application/octet-stream] v1-0001-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch (6.5K, ../../CAGRkXqRYLtBRaMzdH+e7PMO-BRaWPPo37gvOx3C=jQ1uP4Cx7w@mail.gmail.com/3-v1-0001-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch)
download | inline diff:
From 7156c0b4376975367d32e4984712dd4043092468 Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Wed, 23 Sep 2026 00:37:52 -0400
Subject: [PATCH v1 1/2] Fix REPACK (CONCURRENTLY) losing updates after a TOAST
rewrite
The decoding worker of REPACK (CONCURRENTLY) remembers the relfilenumber
of the TOAST relation when it starts, and only decodes the TOAST changes
stored under it. The backend did not lock the TOAST relation until it
started to copy the data. In between, the worker waits for running
transactions to finish, so the gap can be long.
If the TOAST relation was rewritten in that gap, for example by VACUUM
FULL run on it directly, the TOAST chunks of concurrent updates were
filtered out. An updated value then reached the apply phase as a plain
on-disk TOAST pointer, which the apply code takes as a sign that the
column did not change. So it kept the old value, and the committed update
was lost with no error.
Fix by locking the TOAST relation as soon as the worker has finished its
setup, and checking that the TOAST relation still has the relfilenumber
the worker uses. If it does not, start over with a new worker. No data
has been copied at that point, so REPACK just goes on.
We cannot take the lock before starting the worker. A transaction waiting
for that lock has an XID, and the worker waits for it to finish, so that
would be a deadlock. Once its setup is done, the worker no longer waits
for other transactions.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com
Backpatch-through: 19
---
src/backend/commands/repack.c | 68 +++++++++++++++++++++++++-
src/backend/commands/repack_worker.c | 1 +
src/include/commands/repack_internal.h | 7 +++
3 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 759be53d6b8..28f60b9933c 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -221,6 +221,7 @@ static void wait_for_repack_decoding_worker(void);
static void stop_repack_decoding_worker(void);
static void stop_repack_decoding_worker_cb(int code, Datum arg);
static Snapshot get_initial_snapshot(DecodingWorker *worker);
+static bool toast_rewritten_since_worker_start(Oid toastrelid);
static void ProcessRepackMessage(StringInfo msg);
static const char *RepackCommandAsString(RepackCommand cmd);
@@ -1141,7 +1142,43 @@ rebuild_relation(Relation OldHeap, Relation index, bool verbose,
* clustering index) and checking again if it's still eligible for
* REPACK CONCURRENTLY.
*/
- start_repack_decoding_worker(tableOid);
+ for (;;)
+ {
+ Oid toastrelid = OldHeap->rd_rel->reltoastrelid;
+
+ start_repack_decoding_worker(tableOid);
+
+ /*
+ * The worker decodes the TOAST chunks of concurrent changes by
+ * the relfilenumber the TOAST relation had when the worker
+ * started, but we don't hold a lock on the TOAST relation yet, so
+ * it could have been rewritten since then (VACUUM FULL can be run
+ * on it directly). If that happened, the TOAST chunks would not
+ * be decoded, and a changed TOASTed value would be taken for an
+ * unchanged one when applying the changes.
+ *
+ * So lock the TOAST relation now and check. If it has been
+ * rewritten, start over with a new worker. We haven't copied any
+ * data yet, so nothing else is lost.
+ *
+ * We can't lock the TOAST relation before starting the worker:
+ * the worker waits for all transactions with XID to finish, and a
+ * transaction waiting for our lock would then cause a deadlock.
+ * Now that the worker has finished its setup, it no longer waits
+ * for other transactions.
+ */
+ if (!OidIsValid(toastrelid))
+ break;
+ LockRelationOid(toastrelid, ShareUpdateExclusiveLock);
+ if (!toast_rewritten_since_worker_start(toastrelid))
+ break;
+
+ ereport(DEBUG1,
+ errmsg_internal("TOAST relation of \"%s\" was rewritten, restarting REPACK decoding worker",
+ RelationGetRelationName(OldHeap)));
+ UnlockRelationOid(toastrelid, ShareUpdateExclusiveLock);
+ stop_repack_decoding_worker();
+ }
/*
* Wait until the worker has the initial snapshot and retrieve it.
@@ -4036,6 +4073,35 @@ get_initial_snapshot(DecodingWorker *worker)
return snapshot;
}
+/*
+ * Has the given TOAST relation been rewritten since the decoding worker
+ * started?
+ *
+ * The worker only decodes the changes of the TOAST relation stored under the
+ * relfilenumber it saw when starting. The caller must hold a lock on the
+ * TOAST relation that prevents it from being rewritten.
+ */
+static bool
+toast_rewritten_since_worker_start(Oid toastrelid)
+{
+ DecodingWorkerShared *shared;
+ RelFileLocator worker_locator;
+ Relation toastrel;
+ bool result;
+
+ shared = (DecodingWorkerShared *) dsm_segment_address(decoding_worker->seg);
+ SpinLockAcquire(&shared->mutex);
+ Assert(shared->initialized);
+ worker_locator = shared->toast_locator;
+ SpinLockRelease(&shared->mutex);
+
+ toastrel = table_open(toastrelid, NoLock);
+ result = !RelFileLocatorEquals(toastrel->rd_locator, worker_locator);
+ table_close(toastrel, NoLock);
+
+ return result;
+}
+
/*
* Generate worker's file name into 'fname', which must be of size MAXPGPATH.
* If relations of the same 'relid' happen to be processed at the same time,
diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index 690863c6411..6df672c2ca7 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -143,6 +143,7 @@ RepackWorkerMain(Datum main_arg)
/* Announce that we're ready. */
SpinLockAcquire(&shared->mutex);
+ shared->toast_locator = repacked_rel_toast_locator;
shared->initialized = true;
SpinLockRelease(&shared->mutex);
ConditionVariableSignal(&shared->cv);
diff --git a/src/include/commands/repack_internal.h b/src/include/commands/repack_internal.h
index ec6e31d77f2..b4a4b9908f3 100644
--- a/src/include/commands/repack_internal.h
+++ b/src/include/commands/repack_internal.h
@@ -102,6 +102,13 @@ typedef struct DecodingWorkerShared
/* Relation from which data changes to decode. */
Oid relid;
+ /*
+ * Locator of the TOAST relation whose changes the worker decodes, set
+ * together with 'initialized'. The relNumber is InvalidRelFileNumber if
+ * the relation has no TOAST relation.
+ */
+ RelFileLocator toast_locator;
+
/* CV the backend waits on */
ConditionVariable cv;
--
2.37.1 (Apple Git-137.1)
[application/octet-stream] v1-0002-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch (6.6K, ../../CAGRkXqRYLtBRaMzdH+e7PMO-BRaWPPo37gvOx3C=jQ1uP4Cx7w@mail.gmail.com/4-v1-0002-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch)
download | inline diff:
From 3a9164457aa15c7fed459533652e35da0c40ba0c Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Wed, 23 Sep 2026 00:37:52 -0400
Subject: [PATCH v1 2/2] Test TOAST rewrite during REPACK (CONCURRENTLY)
startup
Add a permutation to repack_toast.spec that rewrites the TOAST relation
while the decoding worker waits for a running transaction. REPACK has to
start the worker again. Without the fix, the concurrent updates of
TOASTed columns are lost.
Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com
---
.../expected/repack_toast.out | 148 +++++++++++++++++-
.../injection_points/specs/repack_toast.spec | 41 +++++
2 files changed, 188 insertions(+), 1 deletion(-)
diff --git a/src/test/modules/injection_points/expected/repack_toast.out b/src/test/modules/injection_points/expected/repack_toast.out
index 95e7b19893e..ef5c13f8969 100644
--- a/src/test/modules/injection_points/expected/repack_toast.out
+++ b/src/test/modules/injection_points/expected/repack_toast.out
@@ -1,4 +1,4 @@
-Parsed test spec with 2 sessions
+Parsed test spec with 3 sessions
starting permutation: s1_wait_before_lock s2_updates s2_check s2_wakeup_before_lock s1_check
injection_points_attach
@@ -124,3 +124,149 @@ injection_points_detach
(1 row)
+
+starting permutation: s2_begin s1_wait_before_lock s3_rewrite_toast s2_commit s2_updates s2_check s2_wakeup_before_lock s1_check
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s2_begin:
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+
+has_xid
+-------
+t
+(1 row)
+
+step s1_wait_before_lock:
+ REPACK (CONCURRENTLY) repack_toast;
+ <waiting ...>
+step s3_rewrite_toast:
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+
+step s2_commit:
+ COMMIT;
+
+step s2_updates:
+ DELETE FROM repack_toast WHERE i=1;
+ INSERT INTO repack_toast(i, j, k) VALUES (1, gen_external(), gen_compressible(1));
+
+ -- existing toast data unchanged. (This covers the case where we
+ -- adjust the toast pointer.)
+ UPDATE repack_toast SET i=i+300 where i % 10 = 2 RETURNING OLD.i, NEW.i;
+
+ -- "j" is here an external indirect, written to the file separately.
+ UPDATE repack_toast SET j=gen_external() where i % 10 = 3 RETURNING OLD.i, NEW.i;
+
+ -- the updated value of "j" is compressed.
+ UPDATE repack_toast SET j=gen_compressible(1), k=k||'' where i % 10 = 4 RETURNING i;
+
+ -- the updated value of "j" is compressed externally.
+ UPDATE repack_toast SET j=gen_compressible_external(2) where i % 10 = 5 RETURNING i;
+
+ -- the updated value of "j" stays inline.
+ UPDATE repack_toast SET j=gen_inline(), k=repeat(k,5) where i % 10 = 6 RETURNING i;
+
+ -- updated value of "j" is a short varlena; "k" is written separately.
+ UPDATE repack_toast SET j=gen_short(), k=gen_external() where i % 10 = 7 RETURNING i;
+
+ i| i
+--+---
+ 2|302
+12|312
+(2 rows)
+
+ i| i
+--+--
+ 3| 3
+13|13
+(2 rows)
+
+ i
+--
+ 4
+14
+(2 rows)
+
+ i
+--
+ 5
+15
+(2 rows)
+
+ i
+--
+ 6
+16
+(2 rows)
+
+ i
+--
+ 7
+17
+(2 rows)
+
+step s2_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ INSERT INTO data_s2(i, j, j_toast, k, k_toast)
+ SELECT i, j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+step s2_wakeup_before_lock:
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s1_wait_before_lock: <... completed>
+step s1_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ SELECT count(DISTINCT node) FROM relfilenodes;
+
+ INSERT INTO data_s1(i, j, j_toast, k, k_toast)
+ SELECT i,
+ j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+ -- this should be empty
+ SELECT d1.i, substring(d1.j FOR 12) AS d1_j, substring(d1.k FOR 12) AS d1_k,
+ d2.i, substring(d2.j FOR 12) AS d2_j, substring(d2.k FOR 12) AS d2_k,
+ d1.j_toast as d1_j_tst, d2.j_toast as d2_j_tst,
+ d1.k_toast as d1_k_tst, d2.k_toast AS d2_k_tst
+ FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j, k)
+ WHERE d1.i ISNULL OR d2.i ISNULL;
+
+count
+-----
+ 4
+(1 row)
+
+i|d1_j|d1_k|i|d2_j|d2_k|d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst
+-+----+----+-+----+----+--------+--------+--------+--------
+(0 rows)
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/specs/repack_toast.spec b/src/test/modules/injection_points/specs/repack_toast.spec
index cc8f034d016..0288c054f53 100644
--- a/src/test/modules/injection_points/specs/repack_toast.spec
+++ b/src/test/modules/injection_points/specs/repack_toast.spec
@@ -125,6 +125,18 @@ teardown
session s2
+# Keep a transaction with XID open, so that the decoding worker has to wait
+# before it can build the initial snapshot.
+step s2_begin
+{
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+}
+step s2_commit
+{
+ COMMIT;
+}
+
# Test different kinds of toast data changes.
step s2_updates
{
@@ -170,6 +182,23 @@ step s2_wakeup_before_lock
SELECT injection_points_wakeup('repack-concurrently-before-lock');
}
+# Rewrite the TOAST relation. The decoding worker only decodes the changes
+# of the TOAST relation stored under the relfilenumber it saw when starting,
+# so REPACK must notice if the TOAST relation got rewritten before REPACK
+# locked it, and start the worker again. Otherwise the TOAST chunks of the
+# concurrent changes are not decoded, and the changes are lost.
+session s3
+step s3_rewrite_toast
+{
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+}
+
# Test if data changes introduced while one session is performing REPACK
# CONCURRENTLY find their way into the table.
permutation
@@ -178,3 +207,15 @@ permutation
s2_check
s2_wakeup_before_lock
s1_check
+
+# Same, but rewrite the TOAST relation while the decoding worker waits for s2
+# to commit.
+permutation
+ s2_begin
+ s1_wait_before_lock
+ s3_rewrite_toast
+ s2_commit
+ s2_updates
+ s2_check
+ s2_wakeup_before_lock
+ s1_check
--
2.37.1 (Apple Git-137.1)
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
@ 2026-09-23 08:26 ` Thom Brown <thom@linux.com>
2026-09-23 12:08 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
1 sibling, 1 reply; 28+ messages in thread
From: Thom Brown @ 2026-09-23 08:26 UTC (permalink / raw)
To: shihao zhong <zhong950419@gmail.com>; +Cc: Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
On Wed, 23 Sept 2026 at 06:10, shihao zhong <zhong950419@gmail.com> wrote:
>
> > or whether the relfilenode should be re-checked after the snapshot is built
>
> Holding the toast lock from the start deadlocks. A session that asks for
> AccessExclusiveLock gets an XID before it waits, and the decoding worker
> waits for all XIDs while it sets up.
>
> So the attached patch re-checks instead. Once the worker is set up it no
> longer waits for anyone, so the backend locks the toast table there and
> compares its relfilenode with the one the worker uses. If they differ, it
> starts a new worker. Nothing has been copied yet, so REPACK just carries on.
>
> 0002 adds a test to repack_toast.spec that fails without 0001.
Thanks guys.
I've tested your patches, and I can't reproduce the issue with them
applied. The test on its own fails successfully.
I do have a question relating to this:
+ UnlockRelationOid(toastrelid, ShareUpdateExclusiveLock);
+ stop_repack_decoding_worker();
Is there any opportunity for another rewrite to sneak in between these two?
Thom
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 08:26 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
@ 2026-09-23 12:08 ` shihao zhong <zhong950419@gmail.com>
2026-09-23 14:16 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: shihao zhong @ 2026-09-23 12:08 UTC (permalink / raw)
To: Thom Brown <thom@linux.com>; +Cc: Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
> + UnlockRelationOid(toastrelid, ShareUpdateExclusiveLock);
> + stop_repack_decoding_worker();
>
> Is there any opportunity for another rewrite to sneak in between these
two?
Yes, but it doesn't matter. The old worker is thrown away and nothing has
been copied yet. The new worker reads the relfilenode itself when it
starts, so a rewrite before that is simply what it sees. A rewrite after
that is caught by the next check, which is made under the lock again.
The unlock has to come before starting the new worker anyway, or we
are back to the deadlock.
Thanks,
Shihao
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 08:26 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 12:08 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
@ 2026-09-23 14:16 ` Manu <manuelreyesbravo@gmail.com>
0 siblings, 0 replies; 28+ messages in thread
From: Manu @ 2026-09-23 14:16 UTC (permalink / raw)
To: shihao zhong <zhong950419@gmail.com>; +Cc: Thom Brown <thom@linux.com>; Antonin Houska <ah@cybertec.at>; pgsql-hackers@lists.postgresql.org
Hi Shihao,
I tested v1-0001 and v1-0002 on master (cff329240ba), two builds from
the same commit with --enable-cassert --enable-injection-points: one
with only 0002 applied (control) and one with both. 0001 builds with
no new warnings.
1. The race
The script I posted earlier wins the race without an injection point,
so it is a fair before/after check. Same script, same parameters,
against both builds:
master, 0002 only: 5 of 5 runs lost the update
master + 0001: 0 of 5 runs lost the update
2. The test
Thom already reported that the test fails without the fix; this only
adds that it fails for the right reason and nothing else moved:
control: repack_toast FAILS (results differ from expected)
0001: repack_toast passes, 499 ms
The other tests of the injection_points module pass in both builds, so
0002 fails only for the reason it is meant to.
3. The window Thom asked about
> > Is there any opportunity for another rewrite to sneak in between
> > these two?
>
> Yes, but it doesn't matter. The old worker is thrown away and nothing
> has been copied yet. The new worker reads the relfilenode itself when
> it starts, so a rewrite before that is simply what it sees. A rewrite
> after that is caught by the next check, which is made under the lock
> again.
That is the claim I could put under load instead of taking it on
trust. A loop of VACUUM FULL on the toast relation runs for the whole
startup of REPACK, so it lands inside that window many times over,
while an open transaction keeps the worker waiting during setup and an
UPDATE of the TOASTed columns commits right after. With
log_min_messages=debug1 the patch's own DEBUG1 counts the restarts:
hammer 0s: REPACK ended in ~4s, 0 restarts, value correct
hammer 8s: REPACK ended in ~4s, 33 restarts, value correct
hammer 20s: REPACK ended in ~20s, 166 restarts, value correct
hammer 40s: REPACK ended in ~40s, 342 restarts, value correct
master, 20s: REPACK ended in ~5s, n/a, UPDATE LOST
So it holds up under continuous pressure: the value is never wrong, no
deadlock, and the run with no rewrites at all costs nothing (4s, zero
restarts), so the retry loop does not show up on the normal path.
4. One thing worth deciding, not a correctness issue
The numbers above also say that REPACK can be held up for as long as
the rewriting lasts. It is not a hard livelock - with the 8s hammer
it got through in 4s - but with the 20s and the 40s one it finished
only about when the hammer stopped, at roughly 8 restarts per second.
Whether it gets through is a matter of winning the window; the loop
has no cap and no backoff, and each turn starts a worker that waits
for all running transactions again.
I would not call this a bug: correct-but-waiting beats fast-and-wrong,
and a VACUUM FULL loop on a toast relation is not a real workload. But
it is unbounded, and the caller gets no hint of why nothing is
happening. Since 0001 already has the DEBUG1, would it be worth
raising it, or capping the retries and erroring out after N? Your
call - I mention it because the measurement was there.
Script attached (.txt, so the cfbot keeps testing your patches).
Regards,
Manu
#!/usr/bin/env bash
# v1-0001 under a hammer: what happens if the TOAST relation is rewritten
# over and over?
#
# Thom asked (2026-09-23) whether another rewrite can sneak in between
# UnlockRelationOid() and stop_repack_decoding_worker(). Shihao answered that
# it does not matter, because the old worker is discarded and nothing has been
# copied yet. This does not argue with that: it measures it. A loop of
# VACUUM FULL on the TOAST relation runs for the whole startup of REPACK, so
# it lands inside that window many times over.
#
# Three things are measured:
# 1. whether the result is still correct (the UPDATE is not lost),
# 2. how many times the worker was restarted (the DEBUG1 the patch adds),
# 3. whether REPACK finishes, and how fast: the loop in the patch has no
# retry cap, so it matters whether it converges or spins.
#
# hammer.sh [build] [hammer_seconds]
set -u
B=${1:-/home/manu/pgtoast-i-fix}
SECS=${2:-20}
D=/home/manu/pgprog/data_hammer
P=55703
LOG=/home/manu/pgprog/hammer.log
REPACK_OUT=/home/manu/pgprog/hammer-repack.out
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
# The log lives OUTSIDE $D and pg_ctl -l appends: without this, the restart
# count below carries over the previous run (it happened: 446 restarts
# reported on a build that does not even have that message).
rm -rf "$D" "$LOG"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<'EOF'
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
log_min_messages = debug1
log_line_prefix = '%m [%p] '
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l "$LOG" -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
echo "== toast: $TOAST build: $B"
# A: an open transaction, so the worker has someone to wait for during setup
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep(4)" -c "COMMIT" >/dev/null 2>&1 ) &
sleep 0.5
# C: the hammer, rewriting the TOAST relation without pause
( until_t=$((SECONDS + SECS))
while [ $SECONDS -lt $until_t ]; do "$B/bin/psql" -p $P -U postgres -qtAX -c "VACUUM FULL $TOAST" >/dev/null 2>&1; done ) &
HAMMER=$!
# B: the REPACK that has to survive the hammer
t0=$SECONDS
( q "REPACK (CONCURRENTLY) test" > "$REPACK_OUT" 2>&1 ) &
REPACK=$!
wait %1 2>/dev/null # let A commit
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null 2>&1
# Bound the wait on REPACK: if it never returns, that is exactly the data point
waited=0
while kill -0 $REPACK 2>/dev/null && [ $waited -lt $((SECS + 60)) ]; do sleep 1; waited=$((waited+1)); done
if kill -0 $REPACK 2>/dev/null; then
echo "== REPACK DID NOT FINISH after ${waited}s (the hammer ran for ${SECS}s)"; finished=no
else
echo "== REPACK finished in ~$((SECONDS - t0))s"; finished=yes
fi
wait $HAMMER 2>/dev/null
wait 2>/dev/null
value=$(q "SELECT DISTINCT left(big,9) FROM test")
restarts=$(grep -c 'restarting REPACK decoding worker' "$LOG")
echo "== worker restarts: $restarts"
echo "== REPACK output: $(head -2 "$REPACK_OUT" | tr '\n' ' ')"
echo "== final value: $value (expected NEWNEWNEW)"
# Reading the control run (a build WITHOUT the patch): there, 0 restarts is
# normal (the message does not exist) and a correct value proves nothing,
# because this scenario is not a reliable reproducer of the lost update --
# that is what the earlier race script is for, and it wins 5 times out of 5.
# Here the control only tells us how long a REPACK takes when it ignores the
# rewrites.
[ "$finished" = yes ] && [ "$value" = "NEWNEWNEW" ] \
&& echo "== result correct and REPACK finished" \
|| echo "== CHECK: see $LOG"
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
Attachments:
[text/plain] nocfbot-repack-toast-hammer.sh.txt (4.0K, ../../179017301330.2624303.9601990576272593585@gmail.com/2-nocfbot-repack-toast-hammer.sh.txt)
download | inline:
#!/usr/bin/env bash
# v1-0001 under a hammer: what happens if the TOAST relation is rewritten
# over and over?
#
# Thom asked (2026-09-23) whether another rewrite can sneak in between
# UnlockRelationOid() and stop_repack_decoding_worker(). Shihao answered that
# it does not matter, because the old worker is discarded and nothing has been
# copied yet. This does not argue with that: it measures it. A loop of
# VACUUM FULL on the TOAST relation runs for the whole startup of REPACK, so
# it lands inside that window many times over.
#
# Three things are measured:
# 1. whether the result is still correct (the UPDATE is not lost),
# 2. how many times the worker was restarted (the DEBUG1 the patch adds),
# 3. whether REPACK finishes, and how fast: the loop in the patch has no
# retry cap, so it matters whether it converges or spins.
#
# hammer.sh [build] [hammer_seconds]
set -u
B=${1:-/home/manu/pgtoast-i-fix}
SECS=${2:-20}
D=/home/manu/pgprog/data_hammer
P=55703
LOG=/home/manu/pgprog/hammer.log
REPACK_OUT=/home/manu/pgprog/hammer-repack.out
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
# The log lives OUTSIDE $D and pg_ctl -l appends: without this, the restart
# count below carries over the previous run (it happened: 446 restarts
# reported on a build that does not even have that message).
rm -rf "$D" "$LOG"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<'EOF'
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
log_min_messages = debug1
log_line_prefix = '%m [%p] '
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l "$LOG" -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
echo "== toast: $TOAST build: $B"
# A: an open transaction, so the worker has someone to wait for during setup
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep(4)" -c "COMMIT" >/dev/null 2>&1 ) &
sleep 0.5
# C: the hammer, rewriting the TOAST relation without pause
( until_t=$((SECONDS + SECS))
while [ $SECONDS -lt $until_t ]; do "$B/bin/psql" -p $P -U postgres -qtAX -c "VACUUM FULL $TOAST" >/dev/null 2>&1; done ) &
HAMMER=$!
# B: the REPACK that has to survive the hammer
t0=$SECONDS
( q "REPACK (CONCURRENTLY) test" > "$REPACK_OUT" 2>&1 ) &
REPACK=$!
wait %1 2>/dev/null # let A commit
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null 2>&1
# Bound the wait on REPACK: if it never returns, that is exactly the data point
waited=0
while kill -0 $REPACK 2>/dev/null && [ $waited -lt $((SECS + 60)) ]; do sleep 1; waited=$((waited+1)); done
if kill -0 $REPACK 2>/dev/null; then
echo "== REPACK DID NOT FINISH after ${waited}s (the hammer ran for ${SECS}s)"; finished=no
else
echo "== REPACK finished in ~$((SECONDS - t0))s"; finished=yes
fi
wait $HAMMER 2>/dev/null
wait 2>/dev/null
value=$(q "SELECT DISTINCT left(big,9) FROM test")
restarts=$(grep -c 'restarting REPACK decoding worker' "$LOG")
echo "== worker restarts: $restarts"
echo "== REPACK output: $(head -2 "$REPACK_OUT" | tr '\n' ' ')"
echo "== final value: $value (expected NEWNEWNEW)"
# Reading the control run (a build WITHOUT the patch): there, 0 restarts is
# normal (the message does not exist) and a correct value proves nothing,
# because this scenario is not a reliable reproducer of the lost update --
# that is what the earlier race script is for, and it wins 5 times out of 5.
# Here the control only tells us how long a REPACK takes when it ignores the
# rewrites.
[ "$finished" = yes ] && [ "$value" = "NEWNEWNEW" ] \
&& echo "== result correct and REPACK finished" \
|| echo "== CHECK: see $LOG"
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
@ 2026-09-23 16:22 ` Antonin Houska <ah@cybertec.at>
2026-09-23 17:18 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
1 sibling, 2 replies; 28+ messages in thread
From: Antonin Houska @ 2026-09-23 16:22 UTC (permalink / raw)
To: shihao zhong <zhong950419@gmail.com>; +Cc: Manu <manuelreyesbravo@gmail.com>; Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
shihao zhong <zhong950419@gmail.com> wrote:
> > or whether the relfilenode should be re-checked after the snapshot is built
>
> Holding the toast lock from the start deadlocks. A session that asks for
> AccessExclusiveLock gets an XID before it waits, and the decoding worker
> waits for all XIDs while it sets up.
The same (supposedly low) deadlock risk already exists for the main table, see
this comment in rebuild_relation():
/*
* Start the worker that decodes data changes applied while we're
* copying the table contents.
*
* Note that the worker has to wait for all transactions with XID
* already assigned to finish. If some of those transactions is
* waiting for a lock conflicting with ShareUpdateExclusiveLock on our
* table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
* Not sure this risk is worth unlocking/locking the table (and its
* clustering index) and checking again if it's still eligible for
* REPACK CONCURRENTLY.
*/
start_repack_decoding_worker(tableOid);
I'm not sure if locking the TOAST relation earlier would make the situation
worse.
The reason TOAST relation is not locked until copy_table_data() does so is
that CLUSTER / VACUUM FULL in v18 did it this way (not sure what the reason
for such design was). I haven't changed that for REPACK exactly because I
failed to envision this stale relfilenode issue.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
@ 2026-09-23 17:18 ` Thom Brown <thom@linux.com>
2026-09-24 07:57 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
1 sibling, 1 reply; 28+ messages in thread
From: Thom Brown @ 2026-09-23 17:18 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: shihao zhong <zhong950419@gmail.com>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
On Wed, 23 Sept 2026 at 17:22, Antonin Houska <ah@cybertec.at> wrote:
>
> shihao zhong <zhong950419@gmail.com> wrote:
>
> > > or whether the relfilenode should be re-checked after the snapshot is built
> >
> > Holding the toast lock from the start deadlocks. A session that asks for
> > AccessExclusiveLock gets an XID before it waits, and the decoding worker
> > waits for all XIDs while it sets up.
>
> The same (supposedly low) deadlock risk already exists for the main table, see
> this comment in rebuild_relation():
>
> /*
> * Start the worker that decodes data changes applied while we're
> * copying the table contents.
> *
> * Note that the worker has to wait for all transactions with XID
> * already assigned to finish. If some of those transactions is
> * waiting for a lock conflicting with ShareUpdateExclusiveLock on our
> * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
> * Not sure this risk is worth unlocking/locking the table (and its
> * clustering index) and checking again if it's still eligible for
> * REPACK CONCURRENTLY.
> */
> start_repack_decoding_worker(tableOid);
>
> I'm not sure if locking the TOAST relation earlier would make the situation
> worse.
>
> The reason TOAST relation is not locked until copy_table_data() does so is
> that CLUSTER / VACUUM FULL in v18 did it this way (not sure what the reason
> for such design was). I haven't changed that for REPACK exactly because I
> failed to envision this stale relfilenode issue.
I gave that a try, and it does. It just swaps the lost update for a deadlock.
If you lock the toast up front and something rewrites it at the same
time (which is the thing that triggers this in the first place, e.g. a
REPACK of the toast table), REPACK falls over:
Session 1:
BEGIN;
INSERT INTO test VALUES (999999, 'x');
Session 2:
REPACK (CONCURRENTLY) test;
Session 1:
CREATE INDEX ON test (big);
ERROR: deadlock detected
DETAIL: Process 214534 waits for ShareLock on transaction 1774005;
blocked by process 214579.
Process 214579 waits for AccessExclusiveLock on relation 3672470 of
database 5; blocked by process 214534.
CONTEXT: REPACK decoding worker
The rewrite already has an XID by the time it waits, and the worker
waits for that XID whilst it sets up, so the two just sit on each
other. It doesn't matter which lock we take either because anything
that would stop the rewrite conflicts with it.
Thom
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 17:18 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
@ 2026-09-24 07:57 ` Antonin Houska <ah@cybertec.at>
2026-09-24 08:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
0 siblings, 1 reply; 28+ messages in thread
From: Antonin Houska @ 2026-09-24 07:57 UTC (permalink / raw)
To: Thom Brown <thom@linux.com>; +Cc: shihao zhong <zhong950419@gmail.com>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
Thom Brown <thom@linux.com> wrote:
> On Wed, 23 Sept 2026 at 17:22, Antonin Houska <ah@cybertec.at> wrote:
> >
> > shihao zhong <zhong950419@gmail.com> wrote:
> >
> > > > or whether the relfilenode should be re-checked after the snapshot is built
> > >
> > > Holding the toast lock from the start deadlocks. A session that asks for
> > > AccessExclusiveLock gets an XID before it waits, and the decoding worker
> > > waits for all XIDs while it sets up.
> >
> > The same (supposedly low) deadlock risk already exists for the main table, see
> > this comment in rebuild_relation():
> >
> > /*
> > * Start the worker that decodes data changes applied while we're
> > * copying the table contents.
> > *
> > * Note that the worker has to wait for all transactions with XID
> > * already assigned to finish. If some of those transactions is
> > * waiting for a lock conflicting with ShareUpdateExclusiveLock on our
> > * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
> > * Not sure this risk is worth unlocking/locking the table (and its
> > * clustering index) and checking again if it's still eligible for
> > * REPACK CONCURRENTLY.
> > */
> > start_repack_decoding_worker(tableOid);
> >
> > I'm not sure if locking the TOAST relation earlier would make the situation
> > worse.
> >
> > The reason TOAST relation is not locked until copy_table_data() does so is
> > that CLUSTER / VACUUM FULL in v18 did it this way (not sure what the reason
> > for such design was). I haven't changed that for REPACK exactly because I
> > failed to envision this stale relfilenode issue.
>
> I gave that a try, and it does. It just swaps the lost update for a deadlock.
>
> If you lock the toast up front and something rewrites it at the same
> time (which is the thing that triggers this in the first place, e.g. a
> REPACK of the toast table), REPACK falls over:
>
> Session 1:
> BEGIN;
> INSERT INTO test VALUES (999999, 'x');
>
> Session 2:
> REPACK (CONCURRENTLY) test;
>
> Session 1:
> CREATE INDEX ON test (big);
>
> ERROR: deadlock detected
> DETAIL: Process 214534 waits for ShareLock on transaction 1774005;
> blocked by process 214579.
> Process 214579 waits for AccessExclusiveLock on relation 3672470 of
> database 5; blocked by process 214534.
> CONTEXT: REPACK decoding worker
>
> The rewrite already has an XID by the time it waits, and the worker
> waits for that XID whilst it sets up, so the two just sit on each
> other. It doesn't matter which lock we take either because anything
> that would stop the rewrite conflicts with it.
IMO this example does not exactly demonstrate the problem described in the
comment above: if REPACK (CONCURRENTLY) waits for AccessExclusiveLock, it's
going to perform the relation swap, so the worker should already be gone.
On the other hand, the message
"Process ... waits for ShareLock on transaction ..."
is what the deadlock detector would report for the decoding worker. However,
where would the request for AccessExclusiveLock come from in that case? CREATE
INDEX only uses it to lock the new index relation, however that cannot be
locked by other backends until the transaction has committed (because it's not
visible before commit).
What exactly have you changed in the code?
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 17:18 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-24 07:57 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
@ 2026-09-24 08:32 ` Thom Brown <thom@linux.com>
2026-09-25 10:40 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
0 siblings, 1 reply; 28+ messages in thread
From: Thom Brown @ 2026-09-24 08:32 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: shihao zhong <zhong950419@gmail.com>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
On Thu, 24 Sept 2026 at 08:57, Antonin Houska <ah@cybertec.at> wrote:
>
> Thom Brown <thom@linux.com> wrote:
>
> > On Wed, 23 Sept 2026 at 17:22, Antonin Houska <ah@cybertec.at> wrote:
> > >
> > > shihao zhong <zhong950419@gmail.com> wrote:
> > >
> > > > > or whether the relfilenode should be re-checked after the snapshot is built
> > > >
> > > > Holding the toast lock from the start deadlocks. A session that asks for
> > > > AccessExclusiveLock gets an XID before it waits, and the decoding worker
> > > > waits for all XIDs while it sets up.
> > >
> > > The same (supposedly low) deadlock risk already exists for the main table, see
> > > this comment in rebuild_relation():
> > >
> > > /*
> > > * Start the worker that decodes data changes applied while we're
> > > * copying the table contents.
> > > *
> > > * Note that the worker has to wait for all transactions with XID
> > > * already assigned to finish. If some of those transactions is
> > > * waiting for a lock conflicting with ShareUpdateExclusiveLock on our
> > > * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
> > > * Not sure this risk is worth unlocking/locking the table (and its
> > > * clustering index) and checking again if it's still eligible for
> > > * REPACK CONCURRENTLY.
> > > */
> > > start_repack_decoding_worker(tableOid);
> > >
> > > I'm not sure if locking the TOAST relation earlier would make the situation
> > > worse.
> > >
> > > The reason TOAST relation is not locked until copy_table_data() does so is
> > > that CLUSTER / VACUUM FULL in v18 did it this way (not sure what the reason
> > > for such design was). I haven't changed that for REPACK exactly because I
> > > failed to envision this stale relfilenode issue.
> >
> > I gave that a try, and it does. It just swaps the lost update for a deadlock.
> >
> > If you lock the toast up front and something rewrites it at the same
> > time (which is the thing that triggers this in the first place, e.g. a
> > REPACK of the toast table), REPACK falls over:
> >
> > Session 1:
> > BEGIN;
> > INSERT INTO test VALUES (999999, 'x');
> >
> > Session 2:
> > REPACK (CONCURRENTLY) test;
> >
> > Session 1:
> > CREATE INDEX ON test (big);
> >
> > ERROR: deadlock detected
> > DETAIL: Process 214534 waits for ShareLock on transaction 1774005;
> > blocked by process 214579.
> > Process 214579 waits for AccessExclusiveLock on relation 3672470 of
> > database 5; blocked by process 214534.
> > CONTEXT: REPACK decoding worker
> >
> > The rewrite already has an XID by the time it waits, and the worker
> > waits for that XID whilst it sets up, so the two just sit on each
> > other. It doesn't matter which lock we take either because anything
> > that would stop the rewrite conflicts with it.
>
> IMO this example does not exactly demonstrate the problem described in the
> comment above: if REPACK (CONCURRENTLY) waits for AccessExclusiveLock, it's
> going to perform the relation swap, so the worker should already be gone.
>
> On the other hand, the message
>
> "Process ... waits for ShareLock on transaction ..."
>
> is what the deadlock detector would report for the decoding worker. However,
> where would the request for AccessExclusiveLock come from in that case? CREATE
> INDEX only uses it to lock the new index relation, however that cannot be
> locked by other backends until the transaction has committed (because it's not
> visible before commit).
>
> What exactly have you changed in the code?
Apologies, I seem to have incorrectly paired tests with different
results during my copy and pasting.
I'll see if I can untangle it later today.
Thom
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 17:18 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-24 07:57 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-24 08:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
@ 2026-09-25 10:40 ` Thom Brown <thom@linux.com>
0 siblings, 0 replies; 28+ messages in thread
From: Thom Brown @ 2026-09-25 10:40 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: shihao zhong <zhong950419@gmail.com>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
On Thu, 24 Sept 2026 at 09:32, Thom Brown <thom@linux.com> wrote:
>
> On Thu, 24 Sept 2026 at 08:57, Antonin Houska <ah@cybertec.at> wrote:
> >
> > Thom Brown <thom@linux.com> wrote:
> >
> > > On Wed, 23 Sept 2026 at 17:22, Antonin Houska <ah@cybertec.at> wrote:
> > > >
> > > > shihao zhong <zhong950419@gmail.com> wrote:
> > > >
> > > > > > or whether the relfilenode should be re-checked after the snapshot is built
> > > > >
> > > > > Holding the toast lock from the start deadlocks. A session that asks for
> > > > > AccessExclusiveLock gets an XID before it waits, and the decoding worker
> > > > > waits for all XIDs while it sets up.
> > > >
> > > > The same (supposedly low) deadlock risk already exists for the main table, see
> > > > this comment in rebuild_relation():
> > > >
> > > > /*
> > > > * Start the worker that decodes data changes applied while we're
> > > > * copying the table contents.
> > > > *
> > > > * Note that the worker has to wait for all transactions with XID
> > > > * already assigned to finish. If some of those transactions is
> > > > * waiting for a lock conflicting with ShareUpdateExclusiveLock on our
> > > > * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
> > > > * Not sure this risk is worth unlocking/locking the table (and its
> > > > * clustering index) and checking again if it's still eligible for
> > > > * REPACK CONCURRENTLY.
> > > > */
> > > > start_repack_decoding_worker(tableOid);
> > > >
> > > > I'm not sure if locking the TOAST relation earlier would make the situation
> > > > worse.
> > > >
> > > > The reason TOAST relation is not locked until copy_table_data() does so is
> > > > that CLUSTER / VACUUM FULL in v18 did it this way (not sure what the reason
> > > > for such design was). I haven't changed that for REPACK exactly because I
> > > > failed to envision this stale relfilenode issue.
> > >
> > > I gave that a try, and it does. It just swaps the lost update for a deadlock.
> > >
> > > If you lock the toast up front and something rewrites it at the same
> > > time (which is the thing that triggers this in the first place, e.g. a
> > > REPACK of the toast table), REPACK falls over:
> > >
> > > Session 1:
> > > BEGIN;
> > > INSERT INTO test VALUES (999999, 'x');
> > >
> > > Session 2:
> > > REPACK (CONCURRENTLY) test;
> > >
> > > Session 1:
> > > CREATE INDEX ON test (big);
> > >
> > > ERROR: deadlock detected
> > > DETAIL: Process 214534 waits for ShareLock on transaction 1774005;
> > > blocked by process 214579.
> > > Process 214579 waits for AccessExclusiveLock on relation 3672470 of
> > > database 5; blocked by process 214534.
> > > CONTEXT: REPACK decoding worker
> > >
> > > The rewrite already has an XID by the time it waits, and the worker
> > > waits for that XID whilst it sets up, so the two just sit on each
> > > other. It doesn't matter which lock we take either because anything
> > > that would stop the rewrite conflicts with it.
> >
> > IMO this example does not exactly demonstrate the problem described in the
> > comment above: if REPACK (CONCURRENTLY) waits for AccessExclusiveLock, it's
> > going to perform the relation swap, so the worker should already be gone.
> >
> > On the other hand, the message
> >
> > "Process ... waits for ShareLock on transaction ..."
> >
> > is what the deadlock detector would report for the decoding worker. However,
> > where would the request for AccessExclusiveLock come from in that case? CREATE
> > INDEX only uses it to lock the new index relation, however that cannot be
> > locked by other backends until the transaction has committed (because it's not
> > visible before commit).
> >
> > What exactly have you changed in the code?
>
> Apologies, I seem to have incorrectly paired tests with different
> results during my copy and pasting.
>
> I'll see if I can untangle it later today.
I'd run two separate tests and pasted the error from one under the
steps of the other.
Okay, if I start from scratch:
Session 1:
CREATE TABLE test (id int PRIMARY KEY, big text);
BEGIN;
INSERT INTO test VALUES (999999, 'x');
Session 2:
REPACK (CONCURRENTLY) test;
Session 1:
CREATE INDEX ON test (big);
ERROR: deadlock detected
DETAIL: Process 1172013 waits for ShareLock on relation 3672610 of
database 5; blocked by process 1172091.
Process 1172091 waits for ShareLock on transaction 1774055; blocked by
process 1172013.
Regards
Thom
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
@ 2026-09-23 18:27 ` Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 01:45 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
1 sibling, 2 replies; 28+ messages in thread
From: Masahiko Sawada @ 2026-09-23 18:27 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: shihao zhong <zhong950419@gmail.com>; Manu <manuelreyesbravo@gmail.com>; Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
On Wed, Sep 23, 2026 at 9:23 AM Antonin Houska <ah@cybertec.at> wrote:
>
> shihao zhong <zhong950419@gmail.com> wrote:
>
> > > or whether the relfilenode should be re-checked after the snapshot is built
> >
> > Holding the toast lock from the start deadlocks. A session that asks for
> > AccessExclusiveLock gets an XID before it waits, and the decoding worker
> > waits for all XIDs while it sets up.
>
> The same (supposedly low) deadlock risk already exists for the main table, see
> this comment in rebuild_relation():
>
> /*
> * Start the worker that decodes data changes applied while we're
> * copying the table contents.
> *
> * Note that the worker has to wait for all transactions with XID
> * already assigned to finish. If some of those transactions is
> * waiting for a lock conflicting with ShareUpdateExclusiveLock on our
> * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
> * Not sure this risk is worth unlocking/locking the table (and its
> * clustering index) and checking again if it's still eligible for
> * REPACK CONCURRENTLY.
> */
> start_repack_decoding_worker(tableOid);
>
> I'm not sure if locking the TOAST relation earlier would make the situation
> worse.
Agreed.
So I think the simplest fix would be to acquire a lock on the TOAST
table before starting the repack worker. It would make the case in
question fail with a deadlock, instead of silently losing updates.
The proposed patch also fixes the problem, but I'm concerned that it
repeatedly starts and stops the repack worker without any limit. I
think we could error out if we detect a concurrent rewrite, so that
users can re-run REPACK CONCURRENTLY. This check could also be done on
the repack worker side: after getting the relfilelocator of the TOAST
table and initializing the logical decoding, the repack worker
rechecks the relfilelocator. If they don't match, it raises an error.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
@ 2026-09-24 01:45 ` shihao zhong <zhong950419@gmail.com>
2026-09-24 03:08 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
1 sibling, 1 reply; 28+ messages in thread
From: shihao zhong @ 2026-09-24 01:45 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Antonin Houska <ah@cybertec.at>; Manu <manuelreyesbravo@gmail.com>; Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
Hi Masahiko,
Thanks for reviewing it.
> I think we could error out if we detect a concurrent rewrite, so that
> users can re-run REPACK CONCURRENTLY. This check could also be done on
> the repack worker side
Agreed. v2 attached. REPACK now fails if the TOAST table was rewritten,
and the user can run it again.
The check stays in the backend, under the lock, though. If the worker
checks, a rewrite can still come after that check and before
copy_table_data() locks the TOAST table, and the update is lost the same
way. The backend takes the lock right after the worker is set up and
keeps it. Locking first would also work, but then the same race ends in
a deadlock instead of a clear error.
With the loop gone, the window Thom asked about is gone too. 0002 is the
test and is optional.
Shihao
On Wed, Sep 23, 2026 at 2:27 PM Masahiko Sawada <sawada.mshk@gmail.com>
wrote:
> On Wed, Sep 23, 2026 at 9:23 AM Antonin Houska <ah@cybertec.at> wrote:
> >
> > shihao zhong <zhong950419@gmail.com> wrote:
> >
> > > > or whether the relfilenode should be re-checked after the snapshot
> is built
> > >
> > > Holding the toast lock from the start deadlocks. A session that asks
> for
> > > AccessExclusiveLock gets an XID before it waits, and the decoding
> worker
> > > waits for all XIDs while it sets up.
> >
> > The same (supposedly low) deadlock risk already exists for the main
> table, see
> > this comment in rebuild_relation():
> >
> > /*
> > * Start the worker that decodes data changes applied while we're
> > * copying the table contents.
> > *
> > * Note that the worker has to wait for all transactions with XID
> > * already assigned to finish. If some of those transactions is
> > * waiting for a lock conflicting with ShareUpdateExclusiveLock on
> our
> > * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
> > * Not sure this risk is worth unlocking/locking the table (and its
> > * clustering index) and checking again if it's still eligible for
> > * REPACK CONCURRENTLY.
> > */
> > start_repack_decoding_worker(tableOid);
> >
> > I'm not sure if locking the TOAST relation earlier would make the
> situation
> > worse.
>
> Agreed.
>
> So I think the simplest fix would be to acquire a lock on the TOAST
> table before starting the repack worker. It would make the case in
> question fail with a deadlock, instead of silently losing updates.
>
> The proposed patch also fixes the problem, but I'm concerned that it
> repeatedly starts and stops the repack worker without any limit. I
> think we could error out if we detect a concurrent rewrite, so that
> users can re-run REPACK CONCURRENTLY. This check could also be done on
> the repack worker side: after getting the relfilelocator of the TOAST
> table and initializing the logical decoding, the repack worker
> rechecks the relfilelocator. If they don't match, it raises an error.
>
> Regards,
>
> --
> Masahiko Sawada
> Amazon Web Services: https://aws.amazon.com
>
Attachments:
[application/octet-stream] v2-0001-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch (6.4K, ../../CAGRkXqR19T5MMoL=0-tuiYSO82uGfNUdGceSqoDZ9_Fvd6NtUw@mail.gmail.com/3-v2-0001-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch)
download | inline diff:
From c8c4feb560f46591a1e942362fd5ab34276d692a Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Wed, 23 Sep 2026 21:03:13 -0400
Subject: [PATCH v2 1/2] Fix REPACK (CONCURRENTLY) losing updates after a TOAST
rewrite
The decoding worker of REPACK (CONCURRENTLY) remembers the relfilenumber
of the TOAST relation when it starts, and only decodes the TOAST changes
stored under it. The backend did not lock the TOAST relation until it
started to copy the data. In between, the worker waits for running
transactions to finish, so the gap can be long.
If the TOAST relation was rewritten in that gap, for example by VACUUM
FULL run on it directly, the TOAST chunks of concurrent updates were
filtered out. An updated value then reached the apply phase as a plain
on-disk TOAST pointer, which the apply code takes as a sign that the
column did not change. So it kept the old value, and the committed update
was lost with no error.
Fix by locking the TOAST relation as soon as the worker has finished its
setup, and checking that the TOAST relation still has the relfilenumber
the worker uses. If it does not, REPACK fails, and the user can run it
again. The lock is held till the end of the transaction, so no rewrite
can come after the check.
We could take the lock before starting the worker instead, but a
transaction waiting for that lock has an XID, and the worker waits for it
to finish. That turns the lost update into a deadlock. Once its setup is
done, the worker no longer waits for other transactions.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com
Backpatch-through: 19
---
src/backend/commands/repack.c | 59 ++++++++++++++++++++++++++
src/backend/commands/repack_worker.c | 1 +
src/include/commands/repack_internal.h | 7 +++
3 files changed, 67 insertions(+)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 759be53d6b8..815655343a0 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -221,6 +221,7 @@ static void wait_for_repack_decoding_worker(void);
static void stop_repack_decoding_worker(void);
static void stop_repack_decoding_worker_cb(int code, Datum arg);
static Snapshot get_initial_snapshot(DecodingWorker *worker);
+static void check_toast_not_rewritten(Relation OldHeap);
static void ProcessRepackMessage(StringInfo msg);
static const char *RepackCommandAsString(RepackCommand cmd);
@@ -1143,6 +1144,32 @@ rebuild_relation(Relation OldHeap, Relation index, bool verbose,
*/
start_repack_decoding_worker(tableOid);
+ /*
+ * The worker decodes the TOAST chunks of concurrent changes by the
+ * relfilenumber the TOAST relation had when the worker started, but
+ * we don't hold a lock on the TOAST relation yet, so it could have
+ * been rewritten since then (VACUUM FULL can be run on it directly).
+ * If that happened, the TOAST chunks would not be decoded, and a
+ * changed TOASTed value would be taken for an unchanged one when
+ * applying the changes.
+ *
+ * So lock the TOAST relation now and check. The lock is held till
+ * the end of the transaction, so the TOAST relation cannot be
+ * rewritten after the check.
+ *
+ * We can't lock the TOAST relation before starting the worker: the
+ * worker waits for all transactions with XID to finish, and a
+ * transaction waiting for our lock would then cause a deadlock. Now
+ * that the worker has finished its setup, it no longer waits for
+ * other transactions.
+ */
+ if (OidIsValid(OldHeap->rd_rel->reltoastrelid))
+ {
+ LockRelationOid(OldHeap->rd_rel->reltoastrelid,
+ ShareUpdateExclusiveLock);
+ check_toast_not_rewritten(OldHeap);
+ }
+
/*
* Wait until the worker has the initial snapshot and retrieve it.
*/
@@ -4036,6 +4063,38 @@ get_initial_snapshot(DecodingWorker *worker)
return snapshot;
}
+/*
+ * Check that the TOAST relation of OldHeap still has the relfilenumber that
+ * the decoding worker saw when it started, and fail if it does not.
+ *
+ * The worker only decodes the changes of the TOAST relation stored under that
+ * relfilenumber. The caller must hold a lock on the TOAST relation that
+ * prevents it from being rewritten.
+ */
+static void
+check_toast_not_rewritten(Relation OldHeap)
+{
+ DecodingWorkerShared *shared;
+ RelFileLocator worker_locator;
+ Relation toastrel;
+
+ shared = (DecodingWorkerShared *) dsm_segment_address(decoding_worker->seg);
+ SpinLockAcquire(&shared->mutex);
+ Assert(shared->initialized);
+ worker_locator = shared->toast_locator;
+ SpinLockRelease(&shared->mutex);
+
+ toastrel = table_open(OldHeap->rd_rel->reltoastrelid, NoLock);
+ if (!RelFileLocatorEquals(toastrel->rd_locator, worker_locator))
+ ereport(ERROR,
+ errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+ errmsg("could not execute %s on relation \"%s\"",
+ "REPACK (CONCURRENTLY)", RelationGetRelationName(OldHeap)),
+ errdetail("The TOAST relation was rewritten concurrently."),
+ errhint("The transaction might succeed if retried."));
+ table_close(toastrel, NoLock);
+}
+
/*
* Generate worker's file name into 'fname', which must be of size MAXPGPATH.
* If relations of the same 'relid' happen to be processed at the same time,
diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index 690863c6411..6df672c2ca7 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -143,6 +143,7 @@ RepackWorkerMain(Datum main_arg)
/* Announce that we're ready. */
SpinLockAcquire(&shared->mutex);
+ shared->toast_locator = repacked_rel_toast_locator;
shared->initialized = true;
SpinLockRelease(&shared->mutex);
ConditionVariableSignal(&shared->cv);
diff --git a/src/include/commands/repack_internal.h b/src/include/commands/repack_internal.h
index ec6e31d77f2..b4a4b9908f3 100644
--- a/src/include/commands/repack_internal.h
+++ b/src/include/commands/repack_internal.h
@@ -102,6 +102,13 @@ typedef struct DecodingWorkerShared
/* Relation from which data changes to decode. */
Oid relid;
+ /*
+ * Locator of the TOAST relation whose changes the worker decodes, set
+ * together with 'initialized'. The relNumber is InvalidRelFileNumber if
+ * the relation has no TOAST relation.
+ */
+ RelFileLocator toast_locator;
+
/* CV the backend waits on */
ConditionVariable cv;
--
2.37.1 (Apple Git-137.1)
[application/octet-stream] v2-0002-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch (7.1K, ../../CAGRkXqR19T5MMoL=0-tuiYSO82uGfNUdGceSqoDZ9_Fvd6NtUw@mail.gmail.com/4-v2-0002-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch)
download | inline diff:
From cd941178a647d9026b8c5739364305675733398e Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Wed, 23 Sep 2026 21:03:13 -0400
Subject: [PATCH v2 2/2] Test TOAST rewrite during REPACK (CONCURRENTLY)
startup
Add a permutation to repack_toast.spec that rewrites the TOAST relation
while the decoding worker waits for a running transaction. REPACK has to
fail and leave the table alone. Without the fix, it succeeds and the
concurrent updates of TOASTed columns are lost.
Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com
---
.../expected/repack_toast.out | 151 +++++++++++++++++-
.../injection_points/specs/repack_toast.spec | 49 ++++++
2 files changed, 199 insertions(+), 1 deletion(-)
diff --git a/src/test/modules/injection_points/expected/repack_toast.out b/src/test/modules/injection_points/expected/repack_toast.out
index 95e7b19893e..81634339745 100644
--- a/src/test/modules/injection_points/expected/repack_toast.out
+++ b/src/test/modules/injection_points/expected/repack_toast.out
@@ -1,4 +1,4 @@
-Parsed test spec with 2 sessions
+Parsed test spec with 3 sessions
starting permutation: s1_wait_before_lock s2_updates s2_check s2_wakeup_before_lock s1_check
injection_points_attach
@@ -124,3 +124,152 @@ injection_points_detach
(1 row)
+
+starting permutation: s2_begin s1_wait_before_lock s3_rewrite_toast s2_commit s2_updates s2_check s2_wakeup_before_lock_if_waiting s1_check
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s2_begin:
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+
+has_xid
+-------
+t
+(1 row)
+
+step s1_wait_before_lock:
+ REPACK (CONCURRENTLY) repack_toast;
+ <waiting ...>
+step s3_rewrite_toast:
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+
+step s2_commit:
+ COMMIT;
+
+step s1_wait_before_lock: <... completed>
+ERROR: could not execute REPACK (CONCURRENTLY) on relation "repack_toast"
+step s2_updates:
+ DELETE FROM repack_toast WHERE i=1;
+ INSERT INTO repack_toast(i, j, k) VALUES (1, gen_external(), gen_compressible(1));
+
+ -- existing toast data unchanged. (This covers the case where we
+ -- adjust the toast pointer.)
+ UPDATE repack_toast SET i=i+300 where i % 10 = 2 RETURNING OLD.i, NEW.i;
+
+ -- "j" is here an external indirect, written to the file separately.
+ UPDATE repack_toast SET j=gen_external() where i % 10 = 3 RETURNING OLD.i, NEW.i;
+
+ -- the updated value of "j" is compressed.
+ UPDATE repack_toast SET j=gen_compressible(1), k=k||'' where i % 10 = 4 RETURNING i;
+
+ -- the updated value of "j" is compressed externally.
+ UPDATE repack_toast SET j=gen_compressible_external(2) where i % 10 = 5 RETURNING i;
+
+ -- the updated value of "j" stays inline.
+ UPDATE repack_toast SET j=gen_inline(), k=repeat(k,5) where i % 10 = 6 RETURNING i;
+
+ -- updated value of "j" is a short varlena; "k" is written separately.
+ UPDATE repack_toast SET j=gen_short(), k=gen_external() where i % 10 = 7 RETURNING i;
+
+ i| i
+--+---
+ 2|302
+12|312
+(2 rows)
+
+ i| i
+--+--
+ 3| 3
+13|13
+(2 rows)
+
+ i
+--
+ 4
+14
+(2 rows)
+
+ i
+--
+ 5
+15
+(2 rows)
+
+ i
+--
+ 6
+16
+(2 rows)
+
+ i
+--
+ 7
+17
+(2 rows)
+
+step s2_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ INSERT INTO data_s2(i, j, j_toast, k, k_toast)
+ SELECT i, j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+step s2_wakeup_before_lock_if_waiting:
+ SELECT injection_points_wakeup('repack-concurrently-before-lock')
+ FROM pg_stat_activity
+ WHERE wait_event_type = 'InjectionPoint' AND
+ wait_event = 'repack-concurrently-before-lock';
+
+injection_points_wakeup
+-----------------------
+(0 rows)
+
+step s1_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ SELECT count(DISTINCT node) FROM relfilenodes;
+
+ INSERT INTO data_s1(i, j, j_toast, k, k_toast)
+ SELECT i,
+ j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+ -- this should be empty
+ SELECT d1.i, substring(d1.j FOR 12) AS d1_j, substring(d1.k FOR 12) AS d1_k,
+ d2.i, substring(d2.j FOR 12) AS d2_j, substring(d2.k FOR 12) AS d2_k,
+ d1.j_toast as d1_j_tst, d2.j_toast as d2_j_tst,
+ d1.k_toast as d1_k_tst, d2.k_toast AS d2_k_tst
+ FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j, k)
+ WHERE d1.i ISNULL OR d2.i ISNULL;
+
+count
+-----
+ 2
+(1 row)
+
+i|d1_j|d1_k|i|d2_j|d2_k|d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst
+-+----+----+-+----+----+--------+--------+--------+--------
+(0 rows)
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/specs/repack_toast.spec b/src/test/modules/injection_points/specs/repack_toast.spec
index cc8f034d016..a5cf148d97d 100644
--- a/src/test/modules/injection_points/specs/repack_toast.spec
+++ b/src/test/modules/injection_points/specs/repack_toast.spec
@@ -125,6 +125,18 @@ teardown
session s2
+# Keep a transaction with XID open, so that the decoding worker has to wait
+# before it can build the initial snapshot.
+step s2_begin
+{
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+}
+step s2_commit
+{
+ COMMIT;
+}
+
# Test different kinds of toast data changes.
step s2_updates
{
@@ -169,6 +181,31 @@ step s2_wakeup_before_lock
{
SELECT injection_points_wakeup('repack-concurrently-before-lock');
}
+# Like above, but only if REPACK got that far.
+step s2_wakeup_before_lock_if_waiting
+{
+ SELECT injection_points_wakeup('repack-concurrently-before-lock')
+ FROM pg_stat_activity
+ WHERE wait_event_type = 'InjectionPoint' AND
+ wait_event = 'repack-concurrently-before-lock';
+}
+
+# Rewrite the TOAST relation. The decoding worker only decodes the changes
+# of the TOAST relation stored under the relfilenumber it saw when starting,
+# so REPACK must fail if the TOAST relation got rewritten before REPACK locked
+# it. Otherwise the TOAST chunks of the concurrent changes are not decoded,
+# and the changes are lost.
+session s3
+step s3_rewrite_toast
+{
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+}
# Test if data changes introduced while one session is performing REPACK
# CONCURRENTLY find their way into the table.
@@ -178,3 +215,15 @@ permutation
s2_check
s2_wakeup_before_lock
s1_check
+
+# Same, but rewrite the TOAST relation while the decoding worker waits for s2
+# to commit. REPACK must fail and leave the table alone.
+permutation
+ s2_begin
+ s1_wait_before_lock
+ s3_rewrite_toast
+ s2_commit
+ s2_updates
+ s2_check
+ s2_wakeup_before_lock_if_waiting
+ s1_check
--
2.37.1 (Apple Git-137.1)
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 01:45 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
@ 2026-09-24 03:08 ` Manu <manuelreyesbravo@gmail.com>
0 siblings, 0 replies; 28+ messages in thread
From: Manu @ 2026-09-24 03:08 UTC (permalink / raw)
To: shihao zhong <zhong950419@gmail.com>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; Antonin Houska <ah@cybertec.at>; Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
Hi Shihao,
> Agreed. v2 attached. REPACK now fails if the TOAST table was
> rewritten, and the user can run it again.
I ran the same checks as for v1 against v2, on cff329240ba with
--enable-cassert --enable-injection-points, next to a control build
with only v2-0002. Both patches apply cleanly and build with no
warnings.
1. The race without an injection point (VACUUM FULL of the TOAST
relation while the worker waits, UPDATE right after), 5 runs each:
control: REPACK succeeds, update lost in 5 of 5
v2: REPACK fails in 5 of 5, update kept in 5 of 5
ERROR: could not execute REPACK (CONCURRENTLY) on relation "test"
DETAIL: The TOAST relation was rewritten concurrently.
HINT: The transaction might succeed if retried.
2. VACUUM FULL of the TOAST relation in a loop for 20 s, over the
whole startup: v2 fails after 3.6 s with the same error, and the
update is kept. With v1 the same run took about 20 s and 166 worker
restarts, so the unbounded wait I mentioned for v1 is gone.
3. No rewrite at all: REPACK succeeds in 3 of 3, 2.5-2.6 s, the same
as the control.
4. Thom's deadlock case, where a transaction that already has an XID
locks the TOAST relation while the worker waits for it:
REINDEX TABLE of the TOAST relation (lock, no rewrite)
v2: REPACK succeeds, no deadlock, update kept
CLUSTER of the TOAST relation (lock and rewrite)
control: REPACK succeeds, update lost
v2: REPACK fails with the error above, update kept
So taking the lock after the worker's setup does what the commit
message says: no deadlock, and a clear error when the rewrite does
happen.
5. Tests: repack_toast fails on the control and passes with v2. With
v2 all injection_points tests pass (4 regress, 14 isolation), and so
do make check (239) and src/test/isolation (133).
The script is attached (.txt, so the cfbot keeps testing your
patches).
Regards,
Manu
#!/usr/bin/env bash
# v2 of the fix: REPACK (CONCURRENTLY) now errors out if the TOAST relation
# was rewritten while the decoding worker was starting, instead of retrying.
# Every case runs against a build and reports REPACK's own outcome (ok or
# its error), how long it took, and the final value of the updated rows.
#
# race Thom's case without an injection point: an open transaction
# keeps the worker waiting, VACUUM FULL rewrites the TOAST
# relation meanwhile, an UPDATE of the TOASTed column commits
# right after the transaction ends. N attempts.
# hammer VACUUM FULL of the TOAST relation in a loop for SECS seconds,
# over the whole startup of REPACK (the v1 retry loop spun here).
# none the same, with no rewrite at all: the normal path.
# xidlock a transaction that already has an XID locks the TOAST
# relation (REINDEX of it) while the worker waits for it: taking
# the TOAST lock before starting the worker deadlocks here.
# xidrewrite the same, but the transaction rewrites the TOAST relation
# (CLUSTER of it) before committing.
#
# v2_check.sh <install dir> [case ...]
set -u
B=$1; shift
CASES=${*:-race hammer none xidlock xidrewrite}
N=${N:-5}
SECS=${SECS:-20}
D=${D:-$HOME/pgprog/data_v2check}
P=${P:-55711}
LOG=$HOME/pgprog/v2check.log
OUT=$HOME/pgprog/v2check-repack.out
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
rm -rf "$D" "$LOG"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<'EOF'
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
deadlock_timeout = 1s
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l "$LOG" -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
fresh() { # a new table with three TOASTed rows; sets TOAST
q "DROP TABLE IF EXISTS test" >/dev/null
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
}
start_repack() { # in the background; sets REPACK and T0
T0=$(date +%s.%N)
( q "REPACK (CONCURRENTLY) test" > "$OUT" 2>&1; date +%s.%N > "$OUT.end" ) &
REPACK=$!
}
report() { # label
local limit=$((SECS + 60)) waited=0
while kill -0 $REPACK 2>/dev/null && [ $waited -lt $limit ]; do sleep 1; waited=$((waited+1)); done
wait 2>/dev/null
local secs=$(echo "$(cat "$OUT.end" 2>/dev/null || date +%s.%N) - $T0" | bc)
local repack=$(grep -m1 -E 'ERROR|FATAL' "$OUT" | sed 's/^.*\(ERROR\|FATAL\): *//')
local value=$(q "SELECT string_agg(DISTINCT left(big, 9), ',') FROM test")
printf ' %-12s REPACK %-50s %5.1fs value %s\n' "$1" "${repack:-ok}" "$secs" "$value"
[ -z "$repack" ] || grep -E '^(DETAIL|HINT):' "$OUT" | sed 's/^/ /'
}
open_xact() { # seconds: a transaction with an XID, closed after the sleep
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep($1)" -c "COMMIT" >/dev/null 2>&1 ) &
XACT=$!
}
update_new() {
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null
}
echo "== build: $B ($("$B/bin/postgres" --version))"
for c in $CASES; do
case $c in
race)
for i in $(seq 1 $N); do
fresh; open_xact 3; sleep 0.5
start_repack; sleep 1
q "VACUUM FULL $TOAST" >/dev/null
wait $XACT; update_new
report "race $i"
done ;;
hammer)
fresh; open_xact 4; sleep 0.5
( until_t=$((SECONDS + SECS))
while [ $SECONDS -lt $until_t ]; do q "VACUUM FULL $TOAST" >/dev/null; done ) &
HAMMER=$!
start_repack
wait $XACT; update_new
report "hammer ${SECS}s"
wait $HAMMER 2>/dev/null ;;
none)
for i in $(seq 1 3); do
fresh; open_xact 3; sleep 0.5
start_repack
wait $XACT; update_new
report "none $i"
done ;;
xidlock|xidrewrite)
fresh
# LOCK TABLE is refused on a TOAST relation, so use commands that
# lock it for real: REINDEX takes ShareLock on it without a rewrite,
# CLUSTER rewrites it (new relfilenumber).
if [ $c = xidlock ]; then
stmt="REINDEX TABLE $TOAST"
else
idx=$(q "SELECT c.relname FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid WHERE i.indrelid = '$TOAST'::regclass")
stmt="CLUSTER $TOAST USING $idx"
fi
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "INSERT INTO test VALUES (100, 'x')" -c "SELECT pg_sleep(1.5)" \
-c "$stmt" -c "SELECT pg_sleep(1)" -c "COMMIT" > "$OUT.s1" 2>&1 ) &
S1=$!
sleep 0.5; start_repack
wait $S1
s1=$(grep -m1 -E 'ERROR' "$OUT.s1" | sed 's/^.*ERROR: *//')
update_new
report "$c"
echo " session 1 ($stmt): ${s1:-ok}" ;;
esac
done
grep -E 'deadlock detected' "$LOG" | head -3 | sed 's/^/ log: /'
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
Attachments:
[text/plain] nocfbot-repack-toast-v2-check.sh.txt (4.9K, ../../179021928483.3690793.10906772966940835615@gmail.com/2-nocfbot-repack-toast-v2-check.sh.txt)
download | inline:
#!/usr/bin/env bash
# v2 of the fix: REPACK (CONCURRENTLY) now errors out if the TOAST relation
# was rewritten while the decoding worker was starting, instead of retrying.
# Every case runs against a build and reports REPACK's own outcome (ok or
# its error), how long it took, and the final value of the updated rows.
#
# race Thom's case without an injection point: an open transaction
# keeps the worker waiting, VACUUM FULL rewrites the TOAST
# relation meanwhile, an UPDATE of the TOASTed column commits
# right after the transaction ends. N attempts.
# hammer VACUUM FULL of the TOAST relation in a loop for SECS seconds,
# over the whole startup of REPACK (the v1 retry loop spun here).
# none the same, with no rewrite at all: the normal path.
# xidlock a transaction that already has an XID locks the TOAST
# relation (REINDEX of it) while the worker waits for it: taking
# the TOAST lock before starting the worker deadlocks here.
# xidrewrite the same, but the transaction rewrites the TOAST relation
# (CLUSTER of it) before committing.
#
# v2_check.sh <install dir> [case ...]
set -u
B=$1; shift
CASES=${*:-race hammer none xidlock xidrewrite}
N=${N:-5}
SECS=${SECS:-20}
D=${D:-$HOME/pgprog/data_v2check}
P=${P:-55711}
LOG=$HOME/pgprog/v2check.log
OUT=$HOME/pgprog/v2check-repack.out
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
rm -rf "$D" "$LOG"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<'EOF'
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
deadlock_timeout = 1s
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l "$LOG" -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
fresh() { # a new table with three TOASTed rows; sets TOAST
q "DROP TABLE IF EXISTS test" >/dev/null
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
}
start_repack() { # in the background; sets REPACK and T0
T0=$(date +%s.%N)
( q "REPACK (CONCURRENTLY) test" > "$OUT" 2>&1; date +%s.%N > "$OUT.end" ) &
REPACK=$!
}
report() { # label
local limit=$((SECS + 60)) waited=0
while kill -0 $REPACK 2>/dev/null && [ $waited -lt $limit ]; do sleep 1; waited=$((waited+1)); done
wait 2>/dev/null
local secs=$(echo "$(cat "$OUT.end" 2>/dev/null || date +%s.%N) - $T0" | bc)
local repack=$(grep -m1 -E 'ERROR|FATAL' "$OUT" | sed 's/^.*\(ERROR\|FATAL\): *//')
local value=$(q "SELECT string_agg(DISTINCT left(big, 9), ',') FROM test")
printf ' %-12s REPACK %-50s %5.1fs value %s\n' "$1" "${repack:-ok}" "$secs" "$value"
[ -z "$repack" ] || grep -E '^(DETAIL|HINT):' "$OUT" | sed 's/^/ /'
}
open_xact() { # seconds: a transaction with an XID, closed after the sleep
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep($1)" -c "COMMIT" >/dev/null 2>&1 ) &
XACT=$!
}
update_new() {
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null
}
echo "== build: $B ($("$B/bin/postgres" --version))"
for c in $CASES; do
case $c in
race)
for i in $(seq 1 $N); do
fresh; open_xact 3; sleep 0.5
start_repack; sleep 1
q "VACUUM FULL $TOAST" >/dev/null
wait $XACT; update_new
report "race $i"
done ;;
hammer)
fresh; open_xact 4; sleep 0.5
( until_t=$((SECONDS + SECS))
while [ $SECONDS -lt $until_t ]; do q "VACUUM FULL $TOAST" >/dev/null; done ) &
HAMMER=$!
start_repack
wait $XACT; update_new
report "hammer ${SECS}s"
wait $HAMMER 2>/dev/null ;;
none)
for i in $(seq 1 3); do
fresh; open_xact 3; sleep 0.5
start_repack
wait $XACT; update_new
report "none $i"
done ;;
xidlock|xidrewrite)
fresh
# LOCK TABLE is refused on a TOAST relation, so use commands that
# lock it for real: REINDEX takes ShareLock on it without a rewrite,
# CLUSTER rewrites it (new relfilenumber).
if [ $c = xidlock ]; then
stmt="REINDEX TABLE $TOAST"
else
idx=$(q "SELECT c.relname FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid WHERE i.indrelid = '$TOAST'::regclass")
stmt="CLUSTER $TOAST USING $idx"
fi
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "INSERT INTO test VALUES (100, 'x')" -c "SELECT pg_sleep(1.5)" \
-c "$stmt" -c "SELECT pg_sleep(1)" -c "COMMIT" > "$OUT.s1" 2>&1 ) &
S1=$!
sleep 0.5; start_repack
wait $S1
s1=$(grep -m1 -E 'ERROR' "$OUT.s1" | sed 's/^.*ERROR: *//')
update_new
report "$c"
echo " session 1 ($stmt): ${s1:-ok}" ;;
esac
done
grep -E 'deadlock detected' "$LOG" | head -3 | sed 's/^/ log: /'
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
@ 2026-09-24 03:51 ` Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
1 sibling, 1 reply; 28+ messages in thread
From: Robert Treat @ 2026-09-24 03:51 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Antonin Houska <ah@cybertec.at>; shihao zhong <zhong950419@gmail.com>; Manu <manuelreyesbravo@gmail.com>; Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
On Wed, Sep 23, 2026 at 2:28 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Wed, Sep 23, 2026 at 9:23 AM Antonin Houska <ah@cybertec.at> wrote:
> >
> > shihao zhong <zhong950419@gmail.com> wrote:
> >
> > > > or whether the relfilenode should be re-checked after the snapshot is built
> > >
> > > Holding the toast lock from the start deadlocks. A session that asks for
> > > AccessExclusiveLock gets an XID before it waits, and the decoding worker
> > > waits for all XIDs while it sets up.
> >
> > The same (supposedly low) deadlock risk already exists for the main table, see
> > this comment in rebuild_relation():
> >
> > /*
> > * Start the worker that decodes data changes applied while we're
> > * copying the table contents.
> > *
> > * Note that the worker has to wait for all transactions with XID
> > * already assigned to finish. If some of those transactions is
> > * waiting for a lock conflicting with ShareUpdateExclusiveLock on our
> > * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
> > * Not sure this risk is worth unlocking/locking the table (and its
> > * clustering index) and checking again if it's still eligible for
> > * REPACK CONCURRENTLY.
> > */
> > start_repack_decoding_worker(tableOid);
> >
> > I'm not sure if locking the TOAST relation earlier would make the situation
> > worse.
>
> Agreed.
>
> So I think the simplest fix would be to acquire a lock on the TOAST
> table before starting the repack worker. It would make the case in
> question fail with a deadlock, instead of silently losing updates.
>
> The proposed patch also fixes the problem, but I'm concerned that it
> repeatedly starts and stops the repack worker without any limit. I
> think we could error out if we detect a concurrent rewrite, so that
> users can re-run REPACK CONCURRENTLY. This check could also be done on
> the repack worker side: after getting the relfilelocator of the TOAST
> table and initializing the logical decoding, the repack worker
> rechecks the relfilelocator. If they don't match, it raises an error.
>
It feels a little off to me that if I am trying to REPACKCC, and
someone (maybe even myself, but certainly not Postgres) comes along
and runs a command the conflicts with my existing REPACKCC, that my
REPACKCC is canceled rather than having the other command either wait
or error out. That's a little more complicated a fix, with likely
heavier and/or longer held locks, but feels like it would be less
surprising for users.
Robert Treat
https://xzilla.net
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
@ 2026-09-24 08:41 ` Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: Antonin Houska @ 2026-09-24 08:41 UTC (permalink / raw)
To: Robert Treat <rob@xzilla.net>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; shihao zhong <zhong950419@gmail.com>; Manu <manuelreyesbravo@gmail.com>; Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
Robert Treat <rob@xzilla.net> wrote:
> On Wed, Sep 23, 2026 at 2:28 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Wed, Sep 23, 2026 at 9:23 AM Antonin Houska <ah@cybertec.at> wrote:
> > >
> > > shihao zhong <zhong950419@gmail.com> wrote:
> > >
> > > > > or whether the relfilenode should be re-checked after the snapshot is built
> > > >
> > > > Holding the toast lock from the start deadlocks. A session that asks for
> > > > AccessExclusiveLock gets an XID before it waits, and the decoding worker
> > > > waits for all XIDs while it sets up.
> > >
> > > The same (supposedly low) deadlock risk already exists for the main table, see
> > > this comment in rebuild_relation():
> > >
> > > /*
> > > * Start the worker that decodes data changes applied while we're
> > > * copying the table contents.
> > > *
> > > * Note that the worker has to wait for all transactions with XID
> > > * already assigned to finish. If some of those transactions is
> > > * waiting for a lock conflicting with ShareUpdateExclusiveLock on our
> > > * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
> > > * Not sure this risk is worth unlocking/locking the table (and its
> > > * clustering index) and checking again if it's still eligible for
> > > * REPACK CONCURRENTLY.
> > > */
> > > start_repack_decoding_worker(tableOid);
> > >
> > > I'm not sure if locking the TOAST relation earlier would make the situation
> > > worse.
> >
> > Agreed.
> >
> > So I think the simplest fix would be to acquire a lock on the TOAST
> > table before starting the repack worker. It would make the case in
> > question fail with a deadlock, instead of silently losing updates.
> >
> > The proposed patch also fixes the problem, but I'm concerned that it
> > repeatedly starts and stops the repack worker without any limit. I
> > think we could error out if we detect a concurrent rewrite, so that
> > users can re-run REPACK CONCURRENTLY. This check could also be done on
> > the repack worker side: after getting the relfilelocator of the TOAST
> > table and initializing the logical decoding, the repack worker
> > rechecks the relfilelocator. If they don't match, it raises an error.
> >
>
> It feels a little off to me that if I am trying to REPACKCC, and
> someone (maybe even myself, but certainly not Postgres) comes along
> and runs a command the conflicts with my existing REPACKCC, that my
> REPACKCC is canceled rather than having the other command either wait
> or error out.
pg_squeeze gives up as soon as it notices a "disrupting" catalog
change. Although I haven't heard complaints about this behavior (it's probably
not common to run conflicting DDL commands during maintenance window), I admit
it's not the ideal approach.
For REPACK (CONCURRENTLY), we decided to not give up voluntarily. Even if
REPACK ends up in a deadlock, it still has some chance to win. The direction
we took here is to adjust the deadlock detector (in future versions) so that
REPACK always wins. Raising ERROR on REPACK's side in case of specific
conflict would be against that strategy.
(What I said does not mean that I'm in favor of restarting the decoding worker
either. I still prefer locking the TOAST relation early, as I noted elsewhere
in the thread.)
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
@ 2026-09-25 04:32 ` shihao zhong <zhong950419@gmail.com>
2026-09-25 06:13 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-25 06:24 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
0 siblings, 2 replies; 28+ messages in thread
From: shihao zhong @ 2026-09-25 04:32 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: Robert Treat <rob@xzilla.net>; Masahiko Sawada <sawada.mshk@gmail.com>; Manu <manuelreyesbravo@gmail.com>; Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
> (What I said does not mean that I'm in favor of restarting the decoding
worker
> either. I still prefer locking the TOAST relation early, as I noted
elsewhere
> in the thread.)
OK. v3 locks the TOAST relation before the worker starts, as Sawada-san
first suggested. A rewrite of the TOAST relation now waits for REPACK,
which I think is also what Robert asked for.
The deadlock I mentioned only happens if the rewrite starts while the
worker still waits for older transactions. The rewrite gets the error if
those finish within deadlock_timeout, otherwise REPACK does. Nothing is
lost either way. ALTER TABLE on the table itself hits the same deadlock
on master today.
0002 is the test. The rewrite there uses lock_timeout,
so the result does not depend on which side the deadlock detector picks.
Thanks,
Shihao
Attachments:
[application/octet-stream] v3-0002-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch (7.2K, ../../CAGRkXqR+jco=86v10goPL5jQbKJJcK_dwWB_4dFh4CEDnN7jpg@mail.gmail.com/3-v3-0002-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch)
download | inline diff:
From 9908ffddca55aa81e0846e66149a70f8720522ce Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Thu, 24 Sep 2026 23:46:58 -0400
Subject: [PATCH v3 2/2] Test TOAST rewrite during REPACK (CONCURRENTLY)
startup
Add a permutation to repack_toast.spec that tries to rewrite the TOAST
relation while the decoding worker waits for a running transaction. The
rewrite has to fail on lock_timeout, and REPACK has to keep all the
concurrent changes. Without the fix, the rewrite succeeds and the
concurrent updates of TOASTed columns are lost.
Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com
---
.../expected/repack_toast.out | 151 +++++++++++++++++-
.../injection_points/specs/repack_toast.spec | 51 ++++++
2 files changed, 201 insertions(+), 1 deletion(-)
diff --git a/src/test/modules/injection_points/expected/repack_toast.out b/src/test/modules/injection_points/expected/repack_toast.out
index 95e7b19893e..756e7e8187f 100644
--- a/src/test/modules/injection_points/expected/repack_toast.out
+++ b/src/test/modules/injection_points/expected/repack_toast.out
@@ -1,4 +1,4 @@
-Parsed test spec with 2 sessions
+Parsed test spec with 3 sessions
starting permutation: s1_wait_before_lock s2_updates s2_check s2_wakeup_before_lock s1_check
injection_points_attach
@@ -124,3 +124,152 @@ injection_points_detach
(1 row)
+
+starting permutation: s2_begin s1_wait_before_lock s3_rewrite_toast s3_noop s2_commit s2_updates s2_check s2_wakeup_before_lock s1_check
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s2_begin:
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+
+has_xid
+-------
+t
+(1 row)
+
+step s1_wait_before_lock:
+ REPACK (CONCURRENTLY) repack_toast;
+ <waiting ...>
+step s3_rewrite_toast:
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+ <waiting ...>
+step s3_rewrite_toast: <... completed>
+ERROR: canceling statement due to lock timeout
+step s3_noop:
+step s2_commit:
+ COMMIT;
+
+step s2_updates:
+ DELETE FROM repack_toast WHERE i=1;
+ INSERT INTO repack_toast(i, j, k) VALUES (1, gen_external(), gen_compressible(1));
+
+ -- existing toast data unchanged. (This covers the case where we
+ -- adjust the toast pointer.)
+ UPDATE repack_toast SET i=i+300 where i % 10 = 2 RETURNING OLD.i, NEW.i;
+
+ -- "j" is here an external indirect, written to the file separately.
+ UPDATE repack_toast SET j=gen_external() where i % 10 = 3 RETURNING OLD.i, NEW.i;
+
+ -- the updated value of "j" is compressed.
+ UPDATE repack_toast SET j=gen_compressible(1), k=k||'' where i % 10 = 4 RETURNING i;
+
+ -- the updated value of "j" is compressed externally.
+ UPDATE repack_toast SET j=gen_compressible_external(2) where i % 10 = 5 RETURNING i;
+
+ -- the updated value of "j" stays inline.
+ UPDATE repack_toast SET j=gen_inline(), k=repeat(k,5) where i % 10 = 6 RETURNING i;
+
+ -- updated value of "j" is a short varlena; "k" is written separately.
+ UPDATE repack_toast SET j=gen_short(), k=gen_external() where i % 10 = 7 RETURNING i;
+
+ i| i
+--+---
+ 2|302
+12|312
+(2 rows)
+
+ i| i
+--+--
+ 3| 3
+13|13
+(2 rows)
+
+ i
+--
+ 4
+14
+(2 rows)
+
+ i
+--
+ 5
+15
+(2 rows)
+
+ i
+--
+ 6
+16
+(2 rows)
+
+ i
+--
+ 7
+17
+(2 rows)
+
+step s2_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ INSERT INTO data_s2(i, j, j_toast, k, k_toast)
+ SELECT i, j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+step s2_wakeup_before_lock:
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s1_wait_before_lock: <... completed>
+step s1_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ SELECT count(DISTINCT node) FROM relfilenodes;
+
+ INSERT INTO data_s1(i, j, j_toast, k, k_toast)
+ SELECT i,
+ j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+ -- this should be empty
+ SELECT d1.i, substring(d1.j FOR 12) AS d1_j, substring(d1.k FOR 12) AS d1_k,
+ d2.i, substring(d2.j FOR 12) AS d2_j, substring(d2.k FOR 12) AS d2_k,
+ d1.j_toast as d1_j_tst, d2.j_toast as d2_j_tst,
+ d1.k_toast as d1_k_tst, d2.k_toast AS d2_k_tst
+ FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j, k)
+ WHERE d1.i ISNULL OR d2.i ISNULL;
+
+count
+-----
+ 4
+(1 row)
+
+i|d1_j|d1_k|i|d2_j|d2_k|d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst
+-+----+----+-+----+----+--------+--------+--------+--------
+(0 rows)
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/specs/repack_toast.spec b/src/test/modules/injection_points/specs/repack_toast.spec
index cc8f034d016..a105a44848e 100644
--- a/src/test/modules/injection_points/specs/repack_toast.spec
+++ b/src/test/modules/injection_points/specs/repack_toast.spec
@@ -125,6 +125,18 @@ teardown
session s2
+# Keep a transaction with XID open, so that the decoding worker has to wait
+# before it can build the initial snapshot.
+step s2_begin
+{
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+}
+step s2_commit
+{
+ COMMIT;
+}
+
# Test different kinds of toast data changes.
step s2_updates
{
@@ -170,6 +182,32 @@ step s2_wakeup_before_lock
SELECT injection_points_wakeup('repack-concurrently-before-lock');
}
+# Try to rewrite the TOAST relation. The decoding worker only decodes the
+# changes of the TOAST relation stored under the relfilenumber it saw when
+# starting, so REPACK must not let the TOAST relation be rewritten after that.
+# Otherwise the TOAST chunks of the concurrent changes are not decoded, and the
+# changes are lost.
+#
+# The name of the TOAST relation is only known at run time, hence the DO
+# block, and REPACK rather than VACUUM FULL, which cannot run in one.
+#
+# Don't wait for the lock. The rewrite gets an XID before it waits, and once
+# s2 commits, the decoding worker would wait for that XID, which is a deadlock.
+session s3
+setup { SET lock_timeout = 10; }
+step s3_rewrite_toast
+{
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+}
+# Empty step, so that s2 cannot go on until s3_rewrite_toast is done.
+step s3_noop { }
+
# Test if data changes introduced while one session is performing REPACK
# CONCURRENTLY find their way into the table.
permutation
@@ -178,3 +216,16 @@ permutation
s2_check
s2_wakeup_before_lock
s1_check
+
+# Same, but try to rewrite the TOAST relation while the decoding worker waits
+# for s2 to commit.
+permutation
+ s2_begin
+ s1_wait_before_lock
+ s3_rewrite_toast(*)
+ s3_noop
+ s2_commit
+ s2_updates
+ s2_check
+ s2_wakeup_before_lock
+ s1_check
--
2.37.1 (Apple Git-137.1)
[application/octet-stream] v3-0001-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch (3.5K, ../../CAGRkXqR+jco=86v10goPL5jQbKJJcK_dwWB_4dFh4CEDnN7jpg@mail.gmail.com/4-v3-0001-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch)
download | inline diff:
From c9324e22409e064202872dfa1c559f539094e531 Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Thu, 24 Sep 2026 23:46:58 -0400
Subject: [PATCH v3 1/2] Fix REPACK (CONCURRENTLY) losing updates after a TOAST
rewrite
The decoding worker of REPACK (CONCURRENTLY) remembers the relfilenumber
of the TOAST relation when it starts, and only decodes the TOAST changes
stored under it. The backend did not lock the TOAST relation until it
started to copy the data. In between, the worker waits for running
transactions to finish, so the gap can be long.
If the TOAST relation was rewritten in that gap, for example by VACUUM
FULL run on it directly, the TOAST chunks of concurrent updates were
filtered out. An updated value then reached the apply phase as a plain
on-disk TOAST pointer, which the apply code takes as a sign that the
column did not change. So it kept the old value, and the committed update
was lost with no error.
Fix by locking the TOAST relation before the worker starts, like the
table itself. A rewrite that comes during the startup now waits for
REPACK. The rewrite has an XID by then, so if the worker still waits for
running transactions, the two can deadlock, and the deadlock detector
cancels one of them. DDL on the table itself has the same risk already.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com
---
src/backend/commands/repack.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 759be53d6b8..03fb4d5f0b9 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -1129,6 +1129,20 @@ rebuild_relation(Relation OldHeap, Relation index, bool verbose,
*/
BecomeLockGroupLeader();
+ /*
+ * Lock the TOAST relation before the worker starts. The worker only
+ * decodes the changes of the TOAST relation stored under the
+ * relfilenumber it sees when it starts. If the TOAST relation got
+ * rewritten after that (VACUUM FULL can be run on it directly), the
+ * TOAST chunks of concurrent changes would not be decoded, and a
+ * changed TOASTed value would be taken for an unchanged one when
+ * applying the changes. copy_table_data() locks the TOAST relation
+ * too, but that's too late for this purpose.
+ */
+ if (OidIsValid(OldHeap->rd_rel->reltoastrelid))
+ LockRelationOid(OldHeap->rd_rel->reltoastrelid,
+ ShareUpdateExclusiveLock);
+
/*
* Start the worker that decodes data changes applied while we're
* copying the table contents.
@@ -1136,10 +1150,10 @@ rebuild_relation(Relation OldHeap, Relation index, bool verbose,
* Note that the worker has to wait for all transactions with XID
* already assigned to finish. If some of those transactions is
* waiting for a lock conflicting with ShareUpdateExclusiveLock on our
- * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
- * Not sure this risk is worth unlocking/locking the table (and its
- * clustering index) and checking again if it's still eligible for
- * REPACK CONCURRENTLY.
+ * table or its TOAST relation (e.g. it runs CREATE INDEX), we can
+ * end up in a deadlock. Not sure this risk is worth unlocking/locking
+ * the table (and its clustering index) and checking again if it's
+ * still eligible for REPACK CONCURRENTLY.
*/
start_repack_decoding_worker(tableOid);
--
2.37.1 (Apple Git-137.1)
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
@ 2026-09-25 06:13 ` Manu <manuelreyesbravo@gmail.com>
1 sibling, 0 replies; 28+ messages in thread
From: Manu @ 2026-09-25 06:13 UTC (permalink / raw)
To: shihao zhong <zhong950419@gmail.com>; +Cc: Antonin Houska <ah@cybertec.at>; Robert Treat <rob@xzilla.net>; Masahiko Sawada <sawada.mshk@gmail.com>; Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
Hi Shihao,
shihao zhong <zhong950419@gmail.com> wrote:
> The rewrite gets the error if those finish within deadlock_timeout,
> otherwise REPACK does.
That matches what I see with v3 on master (e27f3b2cad7), using the
same checks as for v2. With Thom's sequence (a transaction open for
3 s, VACUUM FULL of the TOAST relation meanwhile) and the default
deadlock_timeout of 1 s, REPACK got "deadlock detected" in all 6 runs.
With deadlock_timeout = 5s, REPACK finished and the VACUUM FULL got
the error in all 3. No update was lost in any run; master loses it
every time.
One case differs from v2: a transaction that already has an XID runs
REINDEX on the TOAST relation, with no rewrite, during the startup.
On master and with v2, REPACK completes; with v3 it gets "deadlock
detected", at both timeouts. ALTER TABLE on the table itself
deadlocks the same way on master, as you said.
repack_toast fails without 0001 and passed 20 of 20 runs with it;
check and the isolation suite pass. The script and output are
attached.
Regards,
Manu
v3 of the REPACK (CONCURRENTLY) TOAST fix, checked on master e27f3b2cad7
(--enable-cassert --enable-injection-points)
ctl3 = v3-0002 only (test without the fix), v3 = v3-0001 + v3-0002
===== v3_check.sh =====
#!/usr/bin/env bash
# The same checks as v2_check.sh, for v3 of the fix: REPACK (CONCURRENTLY)
# now locks the TOAST relation before the decoding worker starts, so a
# rewrite of it waits for REPACK. Every case runs against a build and
# reports REPACK's own outcome (ok or its error), how long it took, and the
# final value of the updated rows.
#
# New in v3_check.sh:
# xidalter a transaction that already has an XID runs ALTER TABLE on the
# table itself while the worker waits for it (v3's commit
# message says master already deadlocks here).
#
# race Thom's case without an injection point: an open transaction
# keeps the worker waiting, VACUUM FULL rewrites the TOAST
# relation meanwhile, an UPDATE of the TOASTed column commits
# right after the transaction ends. N attempts.
# hammer VACUUM FULL of the TOAST relation in a loop for SECS seconds,
# over the whole startup of REPACK (the v1 retry loop spun here).
# none the same, with no rewrite at all: the normal path.
# xidlock a transaction that already has an XID locks the TOAST
# relation (REINDEX of it) while the worker waits for it: taking
# the TOAST lock before starting the worker deadlocks here.
# xidrewrite the same, but the transaction rewrites the TOAST relation
# (CLUSTER of it) before committing.
#
# v2_check.sh <install dir> [case ...]
set -u
B=$1; shift
CASES=${*:-race hammer none xidlock xidrewrite xidalter}
N=${N:-5}
SECS=${SECS:-20}
D=${D:-$HOME/pgprog/data_v3check}
P=${P:-55711}
LOG=$HOME/pgprog/v3check.log
OUT=$HOME/pgprog/v3check-repack.out
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
rm -rf "$D" "$LOG"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<EOF
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
deadlock_timeout = ${DT:-1s}
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l "$LOG" -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
fresh() { # a new table with three TOASTed rows; sets TOAST
q "DROP TABLE IF EXISTS test" >/dev/null
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
}
start_repack() { # in the background; sets REPACK and T0
T0=$(date +%s.%N)
( q "REPACK (CONCURRENTLY) test" > "$OUT" 2>&1; date +%s.%N > "$OUT.end" ) &
REPACK=$!
}
report() { # label
local limit=$((SECS + 60)) waited=0
while kill -0 $REPACK 2>/dev/null && [ $waited -lt $limit ]; do sleep 1; waited=$((waited+1)); done
wait 2>/dev/null
local secs=$(echo "$(cat "$OUT.end" 2>/dev/null || date +%s.%N) - $T0" | bc)
local repack=$(grep -m1 -E 'ERROR|FATAL' "$OUT" | sed 's/^.*\(ERROR\|FATAL\): *//')
local value=$(q "SELECT string_agg(DISTINCT left(big, 9), ',') FROM test")
printf ' %-12s REPACK %-50s %5.1fs value %s\n' "$1" "${repack:-ok}" "$secs" "$value"
[ -z "$repack" ] || grep -E '^(DETAIL|HINT):' "$OUT" | sed 's/^/ /'
}
open_xact() { # seconds: a transaction with an XID, closed after the sleep
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep($1)" -c "COMMIT" >/dev/null 2>&1 ) &
XACT=$!
}
update_new() {
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null
}
echo "== build: $B ($("$B/bin/postgres" --version)), deadlock_timeout ${DT:-1s}"
for c in $CASES; do
case $c in
race)
for i in $(seq 1 $N); do
fresh; open_xact 3; sleep 0.5
start_repack; sleep 1
q "VACUUM FULL $TOAST" >/dev/null
wait $XACT; update_new
report "race $i"
done ;;
hammer)
fresh; open_xact 4; sleep 0.5
( until_t=$((SECONDS + SECS))
while [ $SECONDS -lt $until_t ]; do q "VACUUM FULL $TOAST" >/dev/null; done ) &
HAMMER=$!
start_repack
wait $XACT; update_new
report "hammer ${SECS}s"
wait $HAMMER 2>/dev/null ;;
none)
for i in $(seq 1 3); do
fresh; open_xact 3; sleep 0.5
start_repack
wait $XACT; update_new
report "none $i"
done ;;
xidlock|xidrewrite|xidalter)
fresh
# LOCK TABLE is refused on a TOAST relation, so use commands that
# lock it for real: REINDEX takes ShareLock on it without a rewrite,
# CLUSTER rewrites it (new relfilenumber). xidalter locks the table
# itself instead.
if [ $c = xidlock ]; then
stmt="REINDEX TABLE $TOAST"
elif [ $c = xidalter ]; then
stmt="ALTER TABLE test ADD COLUMN extra int"
else
idx=$(q "SELECT c.relname FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid WHERE i.indrelid = '$TOAST'::regclass")
stmt="CLUSTER $TOAST USING $idx"
fi
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "INSERT INTO test VALUES (100, 'x')" -c "SELECT pg_sleep(1.5)" \
-c "$stmt" -c "SELECT pg_sleep(1)" -c "COMMIT" > "$OUT.s1" 2>&1 ) &
S1=$!
sleep 0.5; start_repack
wait $S1
s1=$(grep -m1 -E 'ERROR' "$OUT.s1" | sed 's/^.*ERROR: *//')
update_new
report "$c"
echo " session 1 ($stmt): ${s1:-ok}" ;;
esac
done
grep -E 'deadlock detected' "$LOG" | head -3 | sed 's/^/ log: /'
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
===== v3_all.sh =====
#!/bin/bash
# Everything for the v3 review, in one run:
# 1. behaviour checks (v3_check.sh) on the control build (test only) and on v3
# 2. the injection_points tests of both: repack_toast should fail without the fix
# 3. repack_toast alone N times on v3 (it relies on locks and lock_timeout)
# 4. core regression + isolation on v3
set -u
A=$(cd "$(dirname "$0")" && pwd)
RUNS=${RUNS:-20}
for b in ctl3 v3; do
bash $A/v3_check.sh $HOME/pgtoast-i-$b
echo
done
for b in ctl3 v3; do
echo "== injection_points check, $b"
make -C $HOME/pgtoast-$b/src/test/modules/injection_points check 2>&1 \
| grep -E '^(ok|not ok|# All|# [0-9]+ of)|tests (passed|failed)' | tail -30
done
echo "== repack_toast alone, $RUNS runs, v3"
pass=0; fail=0
for i in $(seq $RUNS); do
if make -C $HOME/pgtoast-v3/src/test/modules/injection_points check \
REGRESS= ISOLATION=repack_toast > /tmp/claude-1000/rt_$i.log 2>&1; then
pass=$((pass + 1))
else
fail=$((fail + 1)); cp /tmp/claude-1000/rt_$i.log $A/v3_repack_toast_fail_$i.log
fi
done
echo "repack_toast: $pass passed, $fail failed of $RUNS"
echo "== core regression + isolation, v3"
make -C $HOME/pgtoast-v3 -s check 2>&1 | grep -E '# All|# [0-9]+ of|not ok' | tail -5
make -C $HOME/pgtoast-v3/src/test/isolation -s check 2>&1 | grep -E '# All|# [0-9]+ of|not ok' | tail -5
echo V3-ALL-DONE
===== ./v3_all.sh (deadlock_timeout 1s) =====
== build: /home/manu/pgtoast-i-ctl3 (postgres (PostgreSQL) 20devel)
race 1 REPACK ok 2.5s value oldoldold
race 2 REPACK ok 2.5s value oldoldold
race 3 REPACK ok 2.7s value oldoldold
race 4 REPACK ok 2.5s value oldoldold
race 5 REPACK ok 2.5s value oldoldold
hammer 20s REPACK ok 3.5s value oldoldold
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK ok 2.0s value NEWNEWNEW,x
session 1 (REINDEX TABLE pg_toast.pg_toast_23013): ok
xidrewrite REPACK ok 2.0s value oldoldold,x
session 1 (CLUSTER pg_toast.pg_toast_23041 USING pg_toast_23041_index): ok
xidalter REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 433771 waits for ShareLock on transaction 2346; blocked by process 433611.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 03:04:28.970 -03 [433771] ERROR: deadlock detected
log: 2026-09-25 03:04:28.973 -03 [433754] ERROR: deadlock detected
== build: /home/manu/pgtoast-i-v3 (postgres (PostgreSQL) 20devel)
race 1 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 434893 waits for ShareLock on transaction 668; blocked by process 435170.
HINT: See server log for query details.
race 2 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 436301 waits for ShareLock on transaction 675; blocked by process 436572.
HINT: See server log for query details.
race 3 REPACK deadlock detected 3.6s value NEWNEWNEW
DETAIL: Process 437506 waits for ShareLock on transaction 682; blocked by process 437794.
HINT: See server log for query details.
race 4 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 438838 waits for ShareLock on transaction 689; blocked by process 439118.
HINT: See server log for query details.
race 5 REPACK deadlock detected 3.6s value NEWNEWNEW
DETAIL: Process 440083 waits for ShareLock on transaction 696; blocked by process 440416.
HINT: See server log for query details.
hammer 20s REPACK deadlock detected 4.5s value NEWNEWNEW
DETAIL: Process 441414 waits for ShareLock on transaction 704; blocked by process 441416.
HINT: See server log for query details.
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 454261 waits for ShareLock on transaction 2307; blocked by process 454120.
HINT: See server log for query details.
session 1 (REINDEX TABLE pg_toast.pg_toast_22881): ok
xidrewrite REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 455050 waits for ShareLock on transaction 2313; blocked by process 454885.
HINT: See server log for query details.
session 1 (CLUSTER pg_toast.pg_toast_22896 USING pg_toast_22896_index): ok
xidalter REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 455852 waits for ShareLock on transaction 2319; blocked by process 455689.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 03:04:34.927 -03 [434893] ERROR: deadlock detected
log: 2026-09-25 03:04:35.076 -03 [434892] ERROR: deadlock detected
log: 2026-09-25 03:04:39.335 -03 [436301] ERROR: deadlock detected
== injection_points check, ctl3
ok 1 - injection_points 72 ms
ok 2 - hashagg 17 ms
ok 3 - reindex_conc 16 ms
ok 4 - vacuum 15 ms
# All 4 tests passed.
ok 1 - basic 81 ms
ok 2 - inplace 837 ms
ok 3 - reindex_concurrently_deferred 111 ms
ok 4 - repack 181 ms
ok 5 - repack_commit_race 511 ms
ok 6 - repack_decode 267 ms
ok 7 - repack_temporal 151 ms
ok 8 - repack_temporal_multirange 156 ms
not ok 9 - repack_toast 537 ms
ok 10 - ri_fastpath_reindex 341 ms
ok 11 - ri_fastpath_snapshot 67 ms
ok 12 - syscache-update-pruned 658 ms
ok 13 - wait_cleanup 79 ms
ok 14 - heap_lock_update 134 ms
ok 15 - on_conflict_probe_window 283 ms
# 1 of 15 tests failed.
== injection_points check, v3
ok 1 - injection_points 73 ms
ok 2 - hashagg 15 ms
ok 3 - reindex_conc 18 ms
ok 4 - vacuum 20 ms
# All 4 tests passed.
ok 1 - basic 91 ms
ok 2 - inplace 817 ms
ok 3 - reindex_concurrently_deferred 112 ms
ok 4 - repack 180 ms
ok 5 - repack_commit_race 497 ms
ok 6 - repack_decode 249 ms
ok 7 - repack_temporal 141 ms
ok 8 - repack_temporal_multirange 142 ms
ok 9 - repack_toast 521 ms
ok 10 - ri_fastpath_reindex 343 ms
ok 11 - ri_fastpath_snapshot 62 ms
ok 12 - syscache-update-pruned 563 ms
ok 13 - wait_cleanup 69 ms
ok 14 - heap_lock_update 126 ms
ok 15 - on_conflict_probe_window 288 ms
# All 15 tests passed.
== repack_toast alone, 20 runs, v3
repack_toast: 20 passed, 0 failed of 20
== core regression + isolation, v3
# All 239 tests passed.
# All 133 tests passed.
V3-ALL-DONE
===== DT=5s N=3 ./v3_check.sh <v3> race xidlock xidalter =====
== build: /home/manu/pgtoast-i-v3 (postgres (PostgreSQL) 20devel), deadlock_timeout 5s
race 1 REPACK ok 6.0s value NEWNEWNEW
race 2 REPACK ok 6.0s value NEWNEWNEW
race 3 REPACK ok 6.0s value NEWNEWNEW
xidlock REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 531395 waits for ShareLock on transaction 691; blocked by process 531224.
HINT: See server log for query details.
session 1 (REINDEX TABLE pg_toast.pg_toast_16465): ok
xidalter REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 533699 waits for ShareLock on transaction 697; blocked by process 533548.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 03:08:05.832 -03 [523132] ERROR: deadlock detected
log: 2026-09-25 03:08:13.400 -03 [525785] ERROR: deadlock detected
log: 2026-09-25 03:08:20.968 -03 [528631] ERROR: deadlock detected
log excerpt, first deadlock with deadlock_timeout 5s:
2026-09-25 03:08:05.832 -03 [523132] ERROR: deadlock detected
2026-09-25 03:08:05.832 -03 [523132] DETAIL: Process 523132 waits for AccessExclusiveLock on relation 16388 of database 5; blocked by process 522701.
Process 522701 waits for ShareLock on transaction 668; blocked by process 523132.
Process 523132: VACUUM FULL pg_toast.pg_toast_16384
Process 522701: <command string not enabled>
2026-09-25 03:08:05.832 -03 [523132] HINT: See server log for query details.
2026-09-25 03:08:05.832 -03 [523132] STATEMENT: VACUUM FULL pg_toast.pg_toast_16384
Attachments:
[text/plain] nocfbot-repack-toast-v3-check.txt (16.3K, ../../179031682043.622163.16451343164955142463@gmail.com/2-nocfbot-repack-toast-v3-check.txt)
download | inline:
v3 of the REPACK (CONCURRENTLY) TOAST fix, checked on master e27f3b2cad7
(--enable-cassert --enable-injection-points)
ctl3 = v3-0002 only (test without the fix), v3 = v3-0001 + v3-0002
===== v3_check.sh =====
#!/usr/bin/env bash
# The same checks as v2_check.sh, for v3 of the fix: REPACK (CONCURRENTLY)
# now locks the TOAST relation before the decoding worker starts, so a
# rewrite of it waits for REPACK. Every case runs against a build and
# reports REPACK's own outcome (ok or its error), how long it took, and the
# final value of the updated rows.
#
# New in v3_check.sh:
# xidalter a transaction that already has an XID runs ALTER TABLE on the
# table itself while the worker waits for it (v3's commit
# message says master already deadlocks here).
#
# race Thom's case without an injection point: an open transaction
# keeps the worker waiting, VACUUM FULL rewrites the TOAST
# relation meanwhile, an UPDATE of the TOASTed column commits
# right after the transaction ends. N attempts.
# hammer VACUUM FULL of the TOAST relation in a loop for SECS seconds,
# over the whole startup of REPACK (the v1 retry loop spun here).
# none the same, with no rewrite at all: the normal path.
# xidlock a transaction that already has an XID locks the TOAST
# relation (REINDEX of it) while the worker waits for it: taking
# the TOAST lock before starting the worker deadlocks here.
# xidrewrite the same, but the transaction rewrites the TOAST relation
# (CLUSTER of it) before committing.
#
# v2_check.sh <install dir> [case ...]
set -u
B=$1; shift
CASES=${*:-race hammer none xidlock xidrewrite xidalter}
N=${N:-5}
SECS=${SECS:-20}
D=${D:-$HOME/pgprog/data_v3check}
P=${P:-55711}
LOG=$HOME/pgprog/v3check.log
OUT=$HOME/pgprog/v3check-repack.out
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
rm -rf "$D" "$LOG"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<EOF
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
deadlock_timeout = ${DT:-1s}
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l "$LOG" -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
fresh() { # a new table with three TOASTed rows; sets TOAST
q "DROP TABLE IF EXISTS test" >/dev/null
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
}
start_repack() { # in the background; sets REPACK and T0
T0=$(date +%s.%N)
( q "REPACK (CONCURRENTLY) test" > "$OUT" 2>&1; date +%s.%N > "$OUT.end" ) &
REPACK=$!
}
report() { # label
local limit=$((SECS + 60)) waited=0
while kill -0 $REPACK 2>/dev/null && [ $waited -lt $limit ]; do sleep 1; waited=$((waited+1)); done
wait 2>/dev/null
local secs=$(echo "$(cat "$OUT.end" 2>/dev/null || date +%s.%N) - $T0" | bc)
local repack=$(grep -m1 -E 'ERROR|FATAL' "$OUT" | sed 's/^.*\(ERROR\|FATAL\): *//')
local value=$(q "SELECT string_agg(DISTINCT left(big, 9), ',') FROM test")
printf ' %-12s REPACK %-50s %5.1fs value %s\n' "$1" "${repack:-ok}" "$secs" "$value"
[ -z "$repack" ] || grep -E '^(DETAIL|HINT):' "$OUT" | sed 's/^/ /'
}
open_xact() { # seconds: a transaction with an XID, closed after the sleep
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep($1)" -c "COMMIT" >/dev/null 2>&1 ) &
XACT=$!
}
update_new() {
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null
}
echo "== build: $B ($("$B/bin/postgres" --version)), deadlock_timeout ${DT:-1s}"
for c in $CASES; do
case $c in
race)
for i in $(seq 1 $N); do
fresh; open_xact 3; sleep 0.5
start_repack; sleep 1
q "VACUUM FULL $TOAST" >/dev/null
wait $XACT; update_new
report "race $i"
done ;;
hammer)
fresh; open_xact 4; sleep 0.5
( until_t=$((SECONDS + SECS))
while [ $SECONDS -lt $until_t ]; do q "VACUUM FULL $TOAST" >/dev/null; done ) &
HAMMER=$!
start_repack
wait $XACT; update_new
report "hammer ${SECS}s"
wait $HAMMER 2>/dev/null ;;
none)
for i in $(seq 1 3); do
fresh; open_xact 3; sleep 0.5
start_repack
wait $XACT; update_new
report "none $i"
done ;;
xidlock|xidrewrite|xidalter)
fresh
# LOCK TABLE is refused on a TOAST relation, so use commands that
# lock it for real: REINDEX takes ShareLock on it without a rewrite,
# CLUSTER rewrites it (new relfilenumber). xidalter locks the table
# itself instead.
if [ $c = xidlock ]; then
stmt="REINDEX TABLE $TOAST"
elif [ $c = xidalter ]; then
stmt="ALTER TABLE test ADD COLUMN extra int"
else
idx=$(q "SELECT c.relname FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid WHERE i.indrelid = '$TOAST'::regclass")
stmt="CLUSTER $TOAST USING $idx"
fi
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "INSERT INTO test VALUES (100, 'x')" -c "SELECT pg_sleep(1.5)" \
-c "$stmt" -c "SELECT pg_sleep(1)" -c "COMMIT" > "$OUT.s1" 2>&1 ) &
S1=$!
sleep 0.5; start_repack
wait $S1
s1=$(grep -m1 -E 'ERROR' "$OUT.s1" | sed 's/^.*ERROR: *//')
update_new
report "$c"
echo " session 1 ($stmt): ${s1:-ok}" ;;
esac
done
grep -E 'deadlock detected' "$LOG" | head -3 | sed 's/^/ log: /'
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
===== v3_all.sh =====
#!/bin/bash
# Everything for the v3 review, in one run:
# 1. behaviour checks (v3_check.sh) on the control build (test only) and on v3
# 2. the injection_points tests of both: repack_toast should fail without the fix
# 3. repack_toast alone N times on v3 (it relies on locks and lock_timeout)
# 4. core regression + isolation on v3
set -u
A=$(cd "$(dirname "$0")" && pwd)
RUNS=${RUNS:-20}
for b in ctl3 v3; do
bash $A/v3_check.sh $HOME/pgtoast-i-$b
echo
done
for b in ctl3 v3; do
echo "== injection_points check, $b"
make -C $HOME/pgtoast-$b/src/test/modules/injection_points check 2>&1 \
| grep -E '^(ok|not ok|# All|# [0-9]+ of)|tests (passed|failed)' | tail -30
done
echo "== repack_toast alone, $RUNS runs, v3"
pass=0; fail=0
for i in $(seq $RUNS); do
if make -C $HOME/pgtoast-v3/src/test/modules/injection_points check \
REGRESS= ISOLATION=repack_toast > /tmp/claude-1000/rt_$i.log 2>&1; then
pass=$((pass + 1))
else
fail=$((fail + 1)); cp /tmp/claude-1000/rt_$i.log $A/v3_repack_toast_fail_$i.log
fi
done
echo "repack_toast: $pass passed, $fail failed of $RUNS"
echo "== core regression + isolation, v3"
make -C $HOME/pgtoast-v3 -s check 2>&1 | grep -E '# All|# [0-9]+ of|not ok' | tail -5
make -C $HOME/pgtoast-v3/src/test/isolation -s check 2>&1 | grep -E '# All|# [0-9]+ of|not ok' | tail -5
echo V3-ALL-DONE
===== ./v3_all.sh (deadlock_timeout 1s) =====
== build: /home/manu/pgtoast-i-ctl3 (postgres (PostgreSQL) 20devel)
race 1 REPACK ok 2.5s value oldoldold
race 2 REPACK ok 2.5s value oldoldold
race 3 REPACK ok 2.7s value oldoldold
race 4 REPACK ok 2.5s value oldoldold
race 5 REPACK ok 2.5s value oldoldold
hammer 20s REPACK ok 3.5s value oldoldold
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK ok 2.0s value NEWNEWNEW,x
session 1 (REINDEX TABLE pg_toast.pg_toast_23013): ok
xidrewrite REPACK ok 2.0s value oldoldold,x
session 1 (CLUSTER pg_toast.pg_toast_23041 USING pg_toast_23041_index): ok
xidalter REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 433771 waits for ShareLock on transaction 2346; blocked by process 433611.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 03:04:28.970 -03 [433771] ERROR: deadlock detected
log: 2026-09-25 03:04:28.973 -03 [433754] ERROR: deadlock detected
== build: /home/manu/pgtoast-i-v3 (postgres (PostgreSQL) 20devel)
race 1 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 434893 waits for ShareLock on transaction 668; blocked by process 435170.
HINT: See server log for query details.
race 2 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 436301 waits for ShareLock on transaction 675; blocked by process 436572.
HINT: See server log for query details.
race 3 REPACK deadlock detected 3.6s value NEWNEWNEW
DETAIL: Process 437506 waits for ShareLock on transaction 682; blocked by process 437794.
HINT: See server log for query details.
race 4 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 438838 waits for ShareLock on transaction 689; blocked by process 439118.
HINT: See server log for query details.
race 5 REPACK deadlock detected 3.6s value NEWNEWNEW
DETAIL: Process 440083 waits for ShareLock on transaction 696; blocked by process 440416.
HINT: See server log for query details.
hammer 20s REPACK deadlock detected 4.5s value NEWNEWNEW
DETAIL: Process 441414 waits for ShareLock on transaction 704; blocked by process 441416.
HINT: See server log for query details.
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 454261 waits for ShareLock on transaction 2307; blocked by process 454120.
HINT: See server log for query details.
session 1 (REINDEX TABLE pg_toast.pg_toast_22881): ok
xidrewrite REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 455050 waits for ShareLock on transaction 2313; blocked by process 454885.
HINT: See server log for query details.
session 1 (CLUSTER pg_toast.pg_toast_22896 USING pg_toast_22896_index): ok
xidalter REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 455852 waits for ShareLock on transaction 2319; blocked by process 455689.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 03:04:34.927 -03 [434893] ERROR: deadlock detected
log: 2026-09-25 03:04:35.076 -03 [434892] ERROR: deadlock detected
log: 2026-09-25 03:04:39.335 -03 [436301] ERROR: deadlock detected
== injection_points check, ctl3
ok 1 - injection_points 72 ms
ok 2 - hashagg 17 ms
ok 3 - reindex_conc 16 ms
ok 4 - vacuum 15 ms
# All 4 tests passed.
ok 1 - basic 81 ms
ok 2 - inplace 837 ms
ok 3 - reindex_concurrently_deferred 111 ms
ok 4 - repack 181 ms
ok 5 - repack_commit_race 511 ms
ok 6 - repack_decode 267 ms
ok 7 - repack_temporal 151 ms
ok 8 - repack_temporal_multirange 156 ms
not ok 9 - repack_toast 537 ms
ok 10 - ri_fastpath_reindex 341 ms
ok 11 - ri_fastpath_snapshot 67 ms
ok 12 - syscache-update-pruned 658 ms
ok 13 - wait_cleanup 79 ms
ok 14 - heap_lock_update 134 ms
ok 15 - on_conflict_probe_window 283 ms
# 1 of 15 tests failed.
== injection_points check, v3
ok 1 - injection_points 73 ms
ok 2 - hashagg 15 ms
ok 3 - reindex_conc 18 ms
ok 4 - vacuum 20 ms
# All 4 tests passed.
ok 1 - basic 91 ms
ok 2 - inplace 817 ms
ok 3 - reindex_concurrently_deferred 112 ms
ok 4 - repack 180 ms
ok 5 - repack_commit_race 497 ms
ok 6 - repack_decode 249 ms
ok 7 - repack_temporal 141 ms
ok 8 - repack_temporal_multirange 142 ms
ok 9 - repack_toast 521 ms
ok 10 - ri_fastpath_reindex 343 ms
ok 11 - ri_fastpath_snapshot 62 ms
ok 12 - syscache-update-pruned 563 ms
ok 13 - wait_cleanup 69 ms
ok 14 - heap_lock_update 126 ms
ok 15 - on_conflict_probe_window 288 ms
# All 15 tests passed.
== repack_toast alone, 20 runs, v3
repack_toast: 20 passed, 0 failed of 20
== core regression + isolation, v3
# All 239 tests passed.
# All 133 tests passed.
V3-ALL-DONE
===== DT=5s N=3 ./v3_check.sh <v3> race xidlock xidalter =====
== build: /home/manu/pgtoast-i-v3 (postgres (PostgreSQL) 20devel), deadlock_timeout 5s
race 1 REPACK ok 6.0s value NEWNEWNEW
race 2 REPACK ok 6.0s value NEWNEWNEW
race 3 REPACK ok 6.0s value NEWNEWNEW
xidlock REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 531395 waits for ShareLock on transaction 691; blocked by process 531224.
HINT: See server log for query details.
session 1 (REINDEX TABLE pg_toast.pg_toast_16465): ok
xidalter REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 533699 waits for ShareLock on transaction 697; blocked by process 533548.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 03:08:05.832 -03 [523132] ERROR: deadlock detected
log: 2026-09-25 03:08:13.400 -03 [525785] ERROR: deadlock detected
log: 2026-09-25 03:08:20.968 -03 [528631] ERROR: deadlock detected
log excerpt, first deadlock with deadlock_timeout 5s:
2026-09-25 03:08:05.832 -03 [523132] ERROR: deadlock detected
2026-09-25 03:08:05.832 -03 [523132] DETAIL: Process 523132 waits for AccessExclusiveLock on relation 16388 of database 5; blocked by process 522701.
Process 522701 waits for ShareLock on transaction 668; blocked by process 523132.
Process 523132: VACUUM FULL pg_toast.pg_toast_16384
Process 522701: <command string not enabled>
2026-09-25 03:08:05.832 -03 [523132] HINT: See server log for query details.
2026-09-25 03:08:05.832 -03 [523132] STATEMENT: VACUUM FULL pg_toast.pg_toast_16384
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
@ 2026-09-25 06:24 ` Antonin Houska <ah@cybertec.at>
2026-09-25 12:12 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
1 sibling, 1 reply; 28+ messages in thread
From: Antonin Houska @ 2026-09-25 06:24 UTC (permalink / raw)
To: shihao zhong <zhong950419@gmail.com>; +Cc: Robert Treat <rob@xzilla.net>; Masahiko Sawada <sawada.mshk@gmail.com>; Manu <manuelreyesbravo@gmail.com>; Thom Brown <thom@linux.com>; pgsql-hackers@lists.postgresql.org
shihao zhong <zhong950419@gmail.com> wrote:
> > (What I said does not mean that I'm in favor of restarting the decoding worker
> > either. I still prefer locking the TOAST relation early, as I noted elsewhere
> > in the thread.)
>
> OK. v3 locks the TOAST relation before the worker starts, as Sawada-san
> first suggested. A rewrite of the TOAST relation now waits for REPACK,
> which I think is also what Robert asked for.
Thanks for the patch. I'm just not sure this is the best place to lock the
TOAST table: note that copy_table_data() locks it again.
I'd prefer locking it close to the place we lock the main table (perhaps in
cluster_rel(), after all the checks have been done?) and replace the locking
statements (both in the copy_table_data() and in your patch) with
Assert(CheckRelationLockedByMe(...)).
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-25 06:24 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
@ 2026-09-25 12:12 ` Thom Brown <thom@linux.com>
2026-09-25 14:06 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-25 14:56 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Álvaro Herrera <alvherre@kurilemu.de>
0 siblings, 2 replies; 28+ messages in thread
From: Thom Brown @ 2026-09-25 12:12 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: shihao zhong <zhong950419@gmail.com>; Robert Treat <rob@xzilla.net>; Masahiko Sawada <sawada.mshk@gmail.com>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
On Fri, 25 Sept 2026 at 07:24, Antonin Houska <ah@cybertec.at> wrote:
>
> shihao zhong <zhong950419@gmail.com> wrote:
>
> > > (What I said does not mean that I'm in favor of restarting the decoding worker
> > > either. I still prefer locking the TOAST relation early, as I noted elsewhere
> > > in the thread.)
> >
> > OK. v3 locks the TOAST relation before the worker starts, as Sawada-san
> > first suggested. A rewrite of the TOAST relation now waits for REPACK,
> > which I think is also what Robert asked for.
>
> Thanks for the patch. I'm just not sure this is the best place to lock the
> TOAST table: note that copy_table_data() locks it again.
>
> I'd prefer locking it close to the place we lock the main table (perhaps in
> cluster_rel(), after all the checks have been done?) and replace the locking
> statements (both in the copy_table_data() and in your patch) with
> Assert(CheckRelationLockedByMe(...)).
Moving it into cluster_rel() makes sense, but does it need to be the
full ShareUpdateExclusiveLock there?
If I've understood it right, all that matters before the worker starts
is that nobody can rewrite the TOAST relation, and a rewrite needs
AccessExclusiveLock. So would an AccessShareLock be enough at that
point, at least in the concurrent case? If so, I guess it would also
avoid the case Manu found, where a REINDEX of the TOAST relation from
a transaction that already has an XID deadlocks during startup, as
REINDEX doesn't need a lock that strong.
The stronger lock could presumably still be taken later, where it is
now, once the worker has finished waiting for other transactions, so
waiting for it there couldn't turn into a deadlock. But I may well be
missing a reason it has to be the stronger one from the start.
Thom
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-25 06:24 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 12:12 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
@ 2026-09-25 14:06 ` shihao zhong <zhong950419@gmail.com>
1 sibling, 0 replies; 28+ messages in thread
From: shihao zhong @ 2026-09-25 14:06 UTC (permalink / raw)
To: Thom Brown <thom@linux.com>; +Cc: Antonin Houska <ah@cybertec.at>; Robert Treat <rob@xzilla.net>; Masahiko Sawada <sawada.mshk@gmail.com>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
Hi Thom,
> If I've understood it right, all that matters before the worker starts
> is that nobody can rewrite the TOAST relation, and a rewrite needs
> AccessExclusiveLock. So would an AccessShareLock be enough at that
> point, at least in the concurrent case?
Yes. v4 does it that way.
cluster_rel() now locks the TOAST relation after the checks, as Antonin
suggested. It takes AccessShareLock in the concurrent case and
AccessExclusiveLock otherwise. rebuild_relation() upgrades to
ShareUpdateExclusiveLock once the worker has returned the initial
snapshot, when it no longer waits for other transactions.
copy_table_data() only has the Assert now.
The upgrade does not deadlock with a VACUUM FULL of the TOAST relation
that queued in between. The lock manager puts us ahead of a waiter that
conflicts with a lock we already hold. I checked this with REPACK
stopped in a debugger right before the upgrade.
Manu's REINDEX case passes now, at 1s and 5s deadlock_timeout. 0002 also
runs that REINDEX now, from the transaction the worker waits for. With
v3 it gets "deadlock detected".
Thanks,
Shihao
Attachments:
[application/octet-stream] v4-0002-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch (7.4K, ../../CAGRkXqQJwemwFHyeLCR4uMAmJE95_Ci5-82dEcyvZx3Fw=8xfA@mail.gmail.com/3-v4-0002-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch)
download | inline diff:
From 3fc946abb30255e4ee29d971a91de6da97b26a11 Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Fri, 25 Sep 2026 10:05:22 -0400
Subject: [PATCH v4 2/2] Test TOAST rewrite during REPACK (CONCURRENTLY)
startup
Add a permutation to repack_toast.spec that tries to rewrite the TOAST
relation while the decoding worker waits for a running transaction. The
rewrite has to fail on lock_timeout, and REPACK has to keep all the
concurrent changes. Without the fix, the rewrite succeeds and the
concurrent updates of TOASTed columns are lost.
Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com
---
.../expected/repack_toast.out | 160 +++++++++++++++++-
.../injection_points/specs/repack_toast.spec | 58 +++++++
2 files changed, 217 insertions(+), 1 deletion(-)
diff --git a/src/test/modules/injection_points/expected/repack_toast.out b/src/test/modules/injection_points/expected/repack_toast.out
index 95e7b19893e..7a8edf68f71 100644
--- a/src/test/modules/injection_points/expected/repack_toast.out
+++ b/src/test/modules/injection_points/expected/repack_toast.out
@@ -1,4 +1,4 @@
-Parsed test spec with 2 sessions
+Parsed test spec with 3 sessions
starting permutation: s1_wait_before_lock s2_updates s2_check s2_wakeup_before_lock s1_check
injection_points_attach
@@ -124,3 +124,161 @@ injection_points_detach
(1 row)
+
+starting permutation: s2_begin s1_wait_before_lock s3_rewrite_toast s3_noop s2_reindex_toast s2_commit s2_updates s2_check s2_wakeup_before_lock s1_check
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s2_begin:
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+
+has_xid
+-------
+t
+(1 row)
+
+step s1_wait_before_lock:
+ REPACK (CONCURRENTLY) repack_toast;
+ <waiting ...>
+step s3_rewrite_toast:
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+ <waiting ...>
+step s3_rewrite_toast: <... completed>
+ERROR: canceling statement due to lock timeout
+step s3_noop:
+step s2_reindex_toast:
+ DO $$
+ BEGIN
+ EXECUTE format('REINDEX TABLE %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+
+step s2_commit:
+ COMMIT;
+
+step s2_updates:
+ DELETE FROM repack_toast WHERE i=1;
+ INSERT INTO repack_toast(i, j, k) VALUES (1, gen_external(), gen_compressible(1));
+
+ -- existing toast data unchanged. (This covers the case where we
+ -- adjust the toast pointer.)
+ UPDATE repack_toast SET i=i+300 where i % 10 = 2 RETURNING OLD.i, NEW.i;
+
+ -- "j" is here an external indirect, written to the file separately.
+ UPDATE repack_toast SET j=gen_external() where i % 10 = 3 RETURNING OLD.i, NEW.i;
+
+ -- the updated value of "j" is compressed.
+ UPDATE repack_toast SET j=gen_compressible(1), k=k||'' where i % 10 = 4 RETURNING i;
+
+ -- the updated value of "j" is compressed externally.
+ UPDATE repack_toast SET j=gen_compressible_external(2) where i % 10 = 5 RETURNING i;
+
+ -- the updated value of "j" stays inline.
+ UPDATE repack_toast SET j=gen_inline(), k=repeat(k,5) where i % 10 = 6 RETURNING i;
+
+ -- updated value of "j" is a short varlena; "k" is written separately.
+ UPDATE repack_toast SET j=gen_short(), k=gen_external() where i % 10 = 7 RETURNING i;
+
+ i| i
+--+---
+ 2|302
+12|312
+(2 rows)
+
+ i| i
+--+--
+ 3| 3
+13|13
+(2 rows)
+
+ i
+--
+ 4
+14
+(2 rows)
+
+ i
+--
+ 5
+15
+(2 rows)
+
+ i
+--
+ 6
+16
+(2 rows)
+
+ i
+--
+ 7
+17
+(2 rows)
+
+step s2_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ INSERT INTO data_s2(i, j, j_toast, k, k_toast)
+ SELECT i, j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+step s2_wakeup_before_lock:
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s1_wait_before_lock: <... completed>
+step s1_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ SELECT count(DISTINCT node) FROM relfilenodes;
+
+ INSERT INTO data_s1(i, j, j_toast, k, k_toast)
+ SELECT i,
+ j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+ -- this should be empty
+ SELECT d1.i, substring(d1.j FOR 12) AS d1_j, substring(d1.k FOR 12) AS d1_k,
+ d2.i, substring(d2.j FOR 12) AS d2_j, substring(d2.k FOR 12) AS d2_k,
+ d1.j_toast as d1_j_tst, d2.j_toast as d2_j_tst,
+ d1.k_toast as d1_k_tst, d2.k_toast AS d2_k_tst
+ FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j, k)
+ WHERE d1.i ISNULL OR d2.i ISNULL;
+
+count
+-----
+ 4
+(1 row)
+
+i|d1_j|d1_k|i|d2_j|d2_k|d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst
+-+----+----+-+----+----+--------+--------+--------+--------
+(0 rows)
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/specs/repack_toast.spec b/src/test/modules/injection_points/specs/repack_toast.spec
index cc8f034d016..65c4de8a721 100644
--- a/src/test/modules/injection_points/specs/repack_toast.spec
+++ b/src/test/modules/injection_points/specs/repack_toast.spec
@@ -125,6 +125,31 @@ teardown
session s2
+# Keep a transaction with XID open, so that the decoding worker has to wait
+# before it can build the initial snapshot.
+step s2_begin
+{
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+}
+step s2_commit
+{
+ COMMIT;
+}
+
+# REINDEX does not rewrite the TOAST relation, so REPACK must not block it.
+# The worker waits for s2, so waiting here would be a deadlock.
+step s2_reindex_toast
+{
+ DO $$
+ BEGIN
+ EXECUTE format('REINDEX TABLE %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+}
+
# Test different kinds of toast data changes.
step s2_updates
{
@@ -170,6 +195,25 @@ step s2_wakeup_before_lock
SELECT injection_points_wakeup('repack-concurrently-before-lock');
}
+# Try to rewrite the TOAST relation while the decoding worker is starting.
+# REPACK must block this, or the TOAST changes after the rewrite are lost.
+# REPACK instead of VACUUM FULL, as the name is only known inside a DO block.
+# lock_timeout, because the rewrite gets an XID that the worker would wait for.
+session s3
+setup { SET lock_timeout = 10; }
+step s3_rewrite_toast
+{
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+}
+# Empty step, so that s2 cannot go on until s3_rewrite_toast is done.
+step s3_noop { }
+
# Test if data changes introduced while one session is performing REPACK
# CONCURRENTLY find their way into the table.
permutation
@@ -178,3 +222,17 @@ permutation
s2_check
s2_wakeup_before_lock
s1_check
+
+# Same, but try to rewrite the TOAST relation, and then REINDEX it, while the
+# decoding worker waits for s2 to commit.
+permutation
+ s2_begin
+ s1_wait_before_lock
+ s3_rewrite_toast(*)
+ s3_noop
+ s2_reindex_toast
+ s2_commit
+ s2_updates
+ s2_check
+ s2_wakeup_before_lock
+ s1_check
--
2.37.1 (Apple Git-137.1)
[application/octet-stream] v4-0001-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch (4.6K, ../../CAGRkXqQJwemwFHyeLCR4uMAmJE95_Ci5-82dEcyvZx3Fw=8xfA@mail.gmail.com/4-v4-0001-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch)
download | inline diff:
From 3356a88cb6c1138ef03463c4ff0c2a22546eb958 Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Fri, 25 Sep 2026 10:05:22 -0400
Subject: [PATCH v4 1/2] Fix REPACK (CONCURRENTLY) losing updates after a TOAST
rewrite
The decoding worker of REPACK (CONCURRENTLY) remembers the relfilenumber
of the TOAST relation when it starts, and only decodes the TOAST changes
stored under it. The backend did not lock the TOAST relation until it
started to copy the data. In between, the worker waits for running
transactions to finish, so the gap can be long.
If the TOAST relation was rewritten in that gap, for example by VACUUM
FULL run on it directly, the TOAST chunks of concurrent updates were
filtered out. An updated value then reached the apply phase as a plain
on-disk TOAST pointer, which the apply code takes as a sign that the
column did not change. So it kept the old value, and the committed update
was lost with no error.
Fix by locking the TOAST relation before the worker starts, like the
table itself. A rewrite that comes during the startup now waits for
REPACK. The rewrite has an XID by then, so if the worker still waits for
running transactions, the two can deadlock, and the deadlock detector
cancels one of them. DDL on the table itself has the same risk already.
Backpatch to v19, where REPACK (CONCURRENTLY) was introduced.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com
---
src/backend/commands/repack.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index b748426930a..7f22d4cbf3e 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -659,6 +659,18 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
OldHeap->rd_rel->relkind == RELKIND_MATVIEW ||
OldHeap->rd_rel->relkind == RELKIND_TOASTVALUE);
+ /*
+ * Lock the TOAST relation too. In the concurrent case, the decoding
+ * worker only decodes TOAST changes stored under the relfilenumber it
+ * sees at startup, so the relation must not be rewritten from then on.
+ * AccessShareLock is enough for that. A stronger lock could deadlock
+ * with the transactions the worker waits for, so rebuild_relation()
+ * upgrades it after the worker has started.
+ */
+ if (OidIsValid(OldHeap->rd_rel->reltoastrelid))
+ LockRelationOid(OldHeap->rd_rel->reltoastrelid,
+ concurrent ? AccessShareLock : lmode);
+
/*
* All predicate locks on the tuples or pages are about to be made
* invalid, because we move tuples around. Promote them to relation
@@ -1156,6 +1168,11 @@ rebuild_relation(Relation OldHeap, Relation index, bool verbose,
snapshot = get_initial_snapshot(decoding_worker);
PushActiveSnapshot(snapshot);
+
+ /* Now that the worker is done waiting, upgrade the TOAST lock. */
+ if (OidIsValid(OldHeap->rd_rel->reltoastrelid))
+ LockRelationOid(OldHeap->rd_rel->reltoastrelid,
+ ShareUpdateExclusiveLock);
}
/* for CLUSTER or REPACK USING INDEX, mark the index as the one to use */
@@ -1402,9 +1419,6 @@ copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex,
PGRUsage ru0;
char *nspname;
bool concurrent = snapshot != NULL;
- LOCKMODE lmode;
-
- lmode = RepackLockLevel(concurrent);
pg_rusage_init(&ru0);
@@ -1420,7 +1434,7 @@ copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex,
Assert(newTupDesc->natts == oldTupDesc->natts);
/*
- * If the OldHeap has a toast table, get lock on the toast table to keep
+ * If the OldHeap has a toast table, callers must have locked it to keep
* it from being vacuumed. This is needed because autovacuum processes
* toast tables independently of their main tables, with no lock on the
* latter. If an autovacuum were to start on the toast table after we
@@ -1428,12 +1442,10 @@ copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex,
* possibly remove as DEAD toast tuples belonging to main tuples we think
* are only RECENTLY_DEAD. Then we'd fail while trying to copy those
* tuples.
- *
- * We don't need to open the toast relation here, just lock it. The lock
- * will be held till end of transaction.
*/
- if (OldHeap->rd_rel->reltoastrelid)
- LockRelationOid(OldHeap->rd_rel->reltoastrelid, lmode);
+ Assert(!OidIsValid(OldHeap->rd_rel->reltoastrelid) ||
+ CheckRelationOidLockedByMe(OldHeap->rd_rel->reltoastrelid,
+ RepackLockLevel(concurrent), false));
/*
* If both tables have TOAST tables, perform toast swap by content. It is
--
2.37.1 (Apple Git-137.1)
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-25 06:24 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 12:12 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
@ 2026-09-25 14:56 ` Álvaro Herrera <alvherre@kurilemu.de>
2026-09-25 15:29 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-25 17:20 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
1 sibling, 2 replies; 28+ messages in thread
From: Álvaro Herrera @ 2026-09-25 14:56 UTC (permalink / raw)
To: Thom Brown <thom@linux.com>; +Cc: Antonin Houska <ah@cybertec.at>; shihao zhong <zhong950419@gmail.com>; Robert Treat <rob@xzilla.net>; Masahiko Sawada <sawada.mshk@gmail.com>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
On 2026-Sep-25, Thom Brown wrote:
> Moving it into cluster_rel() makes sense, but does it need to be the
> full ShareUpdateExclusiveLock there?
Yeah, it's better to acquire the lock you want upfront, because
otherwise you introduce more risk of deadlock caused by lock upgrades
(admittedly the user would have to be doing something really stupid in
order for this to be a real problem, but still.) It's only AEL that we
don't want to hold for long.
Maybe the patch could be somewhat like this, then? I didn't review the
test carefully other than running without the code fix to verify that it
fails, and then passes with the fix; and I didn't read the commit
messages either, which I think are LLM-written and not really correct.
(Also, I would push both things as a single commit.)
I think changing the lock as obtained by copy_table_data is not very
nice, because that one is unconditional, and here we only want it in
concurrent mode. BTW I noticed that the comment for copy_table_data
mentions decoding_ctx as an argument, which doesn't exist.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)
Attachments:
[text/x-diff] v4-0001-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch (0B, ../../araJcIXmBySfiXzc@alvherre.pgsql/2-v4-0001-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch)
download
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-25 06:24 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 12:12 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-25 14:56 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Álvaro Herrera <alvherre@kurilemu.de>
@ 2026-09-25 15:29 ` shihao zhong <zhong950419@gmail.com>
1 sibling, 0 replies; 28+ messages in thread
From: shihao zhong @ 2026-09-25 15:29 UTC (permalink / raw)
To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Thom Brown <thom@linux.com>; Antonin Houska <ah@cybertec.at>; Robert Treat <rob@xzilla.net>; Masahiko Sawada <sawada.mshk@gmail.com>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
> (Also, I would push both things as a single commit.)
Fine with me. I kept the test separate only because committers sometimes
push the fix without the test.
Taking the full lock upfront brings back the deadlock Manu found, where
a transaction the worker waits for runs REINDEX on the TOAST relation.
That is the same kind of case as DDL on the table itself, so I'm fine
with it.
Thanks,
Shihao
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-25 06:24 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 12:12 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-25 14:56 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Álvaro Herrera <alvherre@kurilemu.de>
@ 2026-09-25 17:20 ` Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-25 18:10 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
1 sibling, 1 reply; 28+ messages in thread
From: Masahiko Sawada @ 2026-09-25 17:20 UTC (permalink / raw)
To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Thom Brown <thom@linux.com>; Antonin Houska <ah@cybertec.at>; shihao zhong <zhong950419@gmail.com>; Robert Treat <rob@xzilla.net>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
On Fri, Sep 25, 2026 at 7:57 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2026-Sep-25, Thom Brown wrote:
>
> > Moving it into cluster_rel() makes sense, but does it need to be the
> > full ShareUpdateExclusiveLock there?
>
> Yeah, it's better to acquire the lock you want upfront, because
> otherwise you introduce more risk of deadlock caused by lock upgrades
> (admittedly the user would have to be doing something really stupid in
> order for this to be a real problem, but still.) It's only AEL that we
> don't want to hold for long.
+1
> Maybe the patch could be somewhat like this, then? I didn't review the
> test carefully other than running without the code fix to verify that it
> fails, and then passes with the fix; and I didn't read the commit
> messages either, which I think are LLM-written and not really correct.
> (Also, I would push both things as a single commit.)
The patch looks good to me. How about updating the comment in
repack_setup_logical_decoding() to explain that the backend holds the
lock on both relations so the worker doesn't need to keep the locks on
them? For example:
@@ -253,7 +253,9 @@ repack_setup_logical_decoding(Oid relid)
/*
* Set up repacked_rel_locator and repacked_rel_toast_locator, which we
- * use to skip decoding of unrelated relations.
+ * use to skip decoding of unrelated relations. We need not keep the
+ * locks as the backend holds a lock on both the table and its TOAST
+ * relation that prevents them from being rewritten until REPACK finishes.
*/
rel = table_open(relid, AccessShareLock);
repacked_rel_locator = rel->rd_locator;
> I think changing the lock as obtained by copy_table_data is not very
> nice, because that one is unconditional, and here we only want it in
> concurrent mode. BTW I noticed that the comment for copy_table_data
> mentions decoding_ctx as an argument, which doesn't exist.
True.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-25 06:24 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 12:12 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-25 14:56 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Álvaro Herrera <alvherre@kurilemu.de>
2026-09-25 17:20 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
@ 2026-09-25 18:10 ` shihao zhong <zhong950419@gmail.com>
2026-09-25 19:47 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: shihao zhong @ 2026-09-25 18:10 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; Thom Brown <thom@linux.com>; Antonin Houska <ah@cybertec.at>; Robert Treat <rob@xzilla.net>; Manu <manuelreyesbravo@gmail.com>; pgsql-hackers@lists.postgresql.org
Done in v5. 0001 is Álvaro's version as one commit, with that comment
added and a shorter commit message. 0002 fixes the decoding_ctx comment
in copy_table_data().
Thanks,
Shihao
Attachments:
[application/octet-stream] v5-0002-Fix-stale-comment-of-copy_table_data.patch (1.1K, ../../CAGRkXqRkybKtWgCqioPthbqtVDU+yVazOHUBDZ-G0ZEy6k6b1A@mail.gmail.com/3-v5-0002-Fix-stale-comment-of-copy_table_data.patch)
download | inline diff:
From 2a6411d30f6a5e7d39606a025f3d50a6677fb920 Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Fri, 25 Sep 2026 13:53:21 -0400
Subject: [PATCH v5 2/2] Fix stale comment of copy_table_data()
It mentioned a decoding_ctx argument, which the function does not have.
Discussion: https://postgr.es/m/araJcIXmBySfiXzc@alvherre.pgsql
---
src/backend/commands/repack.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 386797915be..5e2dc1ff6ee 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -1387,8 +1387,8 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
/*
* Do the physical copying of table data.
*
- * 'snapshot' and 'decoding_ctx': see table_relation_copy_for_cluster(). Pass
- * iff concurrent processing is required.
+ * 'snapshot': see table_relation_copy_for_cluster(). Pass iff concurrent
+ * processing is required.
*
* There are three output parameters:
* *pSwapToastByContent is set true if toast tables must be swapped by content.
--
2.37.1 (Apple Git-137.1)
[application/octet-stream] v5-0001-Lock-the-TOAST-table-early-in-REPACK-CONCURRENTLY.patch (10.3K, ../../CAGRkXqRkybKtWgCqioPthbqtVDU+yVazOHUBDZ-G0ZEy6k6b1A@mail.gmail.com/4-v5-0001-Lock-the-TOAST-table-early-in-REPACK-CONCURRENTLY.patch)
download | inline diff:
From b2d3fe4fa72ac56ac0016da49b99eee3ef36e7d6 Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Fri, 25 Sep 2026 13:53:21 -0400
Subject: [PATCH v5 1/2] Lock the TOAST table early in REPACK (CONCURRENTLY)
The decoding worker records the relfilenumber of the TOAST table when it
starts, and skips changes stored under any other one. The TOAST table was
not locked until the data copy began. If VACUUM FULL rewrote it while the
worker was starting, concurrent updates of TOASTed columns were lost with
no error.
Lock the TOAST table in cluster_rel() in the concurrent case, before the
worker starts.
Reported-by: Thom Brown <thom@linux.com>
Discussion: https://postgr.es/m/CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com
---
src/backend/commands/repack.c | 22 ++-
src/backend/commands/repack_worker.c | 4 +-
.../expected/repack_toast.out | 151 +++++++++++++++++-
.../injection_points/specs/repack_toast.spec | 51 ++++++
4 files changed, 222 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index b748426930a..386797915be 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -536,6 +536,15 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
if (concurrent)
check_concurrent_repack_requirements(OldHeap, &ident_idx);
+ /*
+ * In concurrent mode, also lock the toast table. Otherwise it would be
+ * possible for the toast relfilenode to change (e.g. because VACUUM FULL
+ * is run on it), and then logical decoding would fail to detect any
+ * concurrent changes there.
+ */
+ if (concurrent && OidIsValid(OldHeap->rd_rel->reltoastrelid))
+ LockRelationOid(OldHeap->rd_rel->reltoastrelid, lmode);
+
/*
* Also check the state of indexes; this can abort the command for REPACK.
* Historically this hasn't affected CLUSTER or VACUUM FULL, so don't do
@@ -1136,6 +1145,11 @@ rebuild_relation(Relation OldHeap, Relation index, bool verbose,
*/
BecomeLockGroupLeader();
+ /* If there is a toast table, it must have been locked already */
+ Assert(!OidIsValid(OldHeap->rd_rel->reltoastrelid) ||
+ CheckRelationOidLockedByMe(OldHeap->rd_rel->reltoastrelid,
+ lmode, false));
+
/*
* Start the worker that decodes data changes applied while we're
* copying the table contents.
@@ -1143,10 +1157,10 @@ rebuild_relation(Relation OldHeap, Relation index, bool verbose,
* Note that the worker has to wait for all transactions with XID
* already assigned to finish. If some of those transactions is
* waiting for a lock conflicting with ShareUpdateExclusiveLock on our
- * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
- * Not sure this risk is worth unlocking/locking the table (and its
- * clustering index) and checking again if it's still eligible for
- * REPACK CONCURRENTLY.
+ * table or its TOAST relation (e.g. it runs CREATE INDEX), we can
+ * end up in a deadlock. Not sure this risk is worth unlocking/locking
+ * the table (and its clustering index) and checking again if it's
+ * still eligible for REPACK CONCURRENTLY.
*/
start_repack_decoding_worker(tableOid);
diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index 690863c6411..4f379a41d6d 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -253,7 +253,9 @@ repack_setup_logical_decoding(Oid relid)
/*
* Set up repacked_rel_locator and repacked_rel_toast_locator, which we
- * use to skip decoding of unrelated relations.
+ * use to skip decoding of unrelated relations. We need not keep the locks
+ * as the backend holds a lock on both the table and its TOAST relation
+ * that prevents them from being rewritten until REPACK finishes.
*/
rel = table_open(relid, AccessShareLock);
repacked_rel_locator = rel->rd_locator;
diff --git a/src/test/modules/injection_points/expected/repack_toast.out b/src/test/modules/injection_points/expected/repack_toast.out
index 95e7b19893e..756e7e8187f 100644
--- a/src/test/modules/injection_points/expected/repack_toast.out
+++ b/src/test/modules/injection_points/expected/repack_toast.out
@@ -1,4 +1,4 @@
-Parsed test spec with 2 sessions
+Parsed test spec with 3 sessions
starting permutation: s1_wait_before_lock s2_updates s2_check s2_wakeup_before_lock s1_check
injection_points_attach
@@ -124,3 +124,152 @@ injection_points_detach
(1 row)
+
+starting permutation: s2_begin s1_wait_before_lock s3_rewrite_toast s3_noop s2_commit s2_updates s2_check s2_wakeup_before_lock s1_check
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s2_begin:
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+
+has_xid
+-------
+t
+(1 row)
+
+step s1_wait_before_lock:
+ REPACK (CONCURRENTLY) repack_toast;
+ <waiting ...>
+step s3_rewrite_toast:
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+ <waiting ...>
+step s3_rewrite_toast: <... completed>
+ERROR: canceling statement due to lock timeout
+step s3_noop:
+step s2_commit:
+ COMMIT;
+
+step s2_updates:
+ DELETE FROM repack_toast WHERE i=1;
+ INSERT INTO repack_toast(i, j, k) VALUES (1, gen_external(), gen_compressible(1));
+
+ -- existing toast data unchanged. (This covers the case where we
+ -- adjust the toast pointer.)
+ UPDATE repack_toast SET i=i+300 where i % 10 = 2 RETURNING OLD.i, NEW.i;
+
+ -- "j" is here an external indirect, written to the file separately.
+ UPDATE repack_toast SET j=gen_external() where i % 10 = 3 RETURNING OLD.i, NEW.i;
+
+ -- the updated value of "j" is compressed.
+ UPDATE repack_toast SET j=gen_compressible(1), k=k||'' where i % 10 = 4 RETURNING i;
+
+ -- the updated value of "j" is compressed externally.
+ UPDATE repack_toast SET j=gen_compressible_external(2) where i % 10 = 5 RETURNING i;
+
+ -- the updated value of "j" stays inline.
+ UPDATE repack_toast SET j=gen_inline(), k=repeat(k,5) where i % 10 = 6 RETURNING i;
+
+ -- updated value of "j" is a short varlena; "k" is written separately.
+ UPDATE repack_toast SET j=gen_short(), k=gen_external() where i % 10 = 7 RETURNING i;
+
+ i| i
+--+---
+ 2|302
+12|312
+(2 rows)
+
+ i| i
+--+--
+ 3| 3
+13|13
+(2 rows)
+
+ i
+--
+ 4
+14
+(2 rows)
+
+ i
+--
+ 5
+15
+(2 rows)
+
+ i
+--
+ 6
+16
+(2 rows)
+
+ i
+--
+ 7
+17
+(2 rows)
+
+step s2_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ INSERT INTO data_s2(i, j, j_toast, k, k_toast)
+ SELECT i, j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+step s2_wakeup_before_lock:
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s1_wait_before_lock: <... completed>
+step s1_check:
+ INSERT INTO relfilenodes(node)
+ SELECT c2.relfilenode
+ FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.oid OR c2.oid = c1.reltoastrelid
+ WHERE c1.relname='repack_toast';
+
+ SELECT count(DISTINCT node) FROM relfilenodes;
+
+ INSERT INTO data_s1(i, j, j_toast, k, k_toast)
+ SELECT i,
+ j, COALESCE(pg_column_toast_chunk_id(j), 0) AS j_toast,
+ k, COALESCE(pg_column_toast_chunk_id(k), 0) AS k_toast
+ FROM repack_toast;
+
+ -- this should be empty
+ SELECT d1.i, substring(d1.j FOR 12) AS d1_j, substring(d1.k FOR 12) AS d1_k,
+ d2.i, substring(d2.j FOR 12) AS d2_j, substring(d2.k FOR 12) AS d2_k,
+ d1.j_toast as d1_j_tst, d2.j_toast as d2_j_tst,
+ d1.k_toast as d1_k_tst, d2.k_toast AS d2_k_tst
+ FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j, k)
+ WHERE d1.i ISNULL OR d2.i ISNULL;
+
+count
+-----
+ 4
+(1 row)
+
+i|d1_j|d1_k|i|d2_j|d2_k|d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst
+-+----+----+-+----+----+--------+--------+--------+--------
+(0 rows)
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/specs/repack_toast.spec b/src/test/modules/injection_points/specs/repack_toast.spec
index cc8f034d016..a105a44848e 100644
--- a/src/test/modules/injection_points/specs/repack_toast.spec
+++ b/src/test/modules/injection_points/specs/repack_toast.spec
@@ -125,6 +125,18 @@ teardown
session s2
+# Keep a transaction with XID open, so that the decoding worker has to wait
+# before it can build the initial snapshot.
+step s2_begin
+{
+ BEGIN;
+ SELECT pg_current_xact_id() IS NOT NULL AS has_xid;
+}
+step s2_commit
+{
+ COMMIT;
+}
+
# Test different kinds of toast data changes.
step s2_updates
{
@@ -170,6 +182,32 @@ step s2_wakeup_before_lock
SELECT injection_points_wakeup('repack-concurrently-before-lock');
}
+# Try to rewrite the TOAST relation. The decoding worker only decodes the
+# changes of the TOAST relation stored under the relfilenumber it saw when
+# starting, so REPACK must not let the TOAST relation be rewritten after that.
+# Otherwise the TOAST chunks of the concurrent changes are not decoded, and the
+# changes are lost.
+#
+# The name of the TOAST relation is only known at run time, hence the DO
+# block, and REPACK rather than VACUUM FULL, which cannot run in one.
+#
+# Don't wait for the lock. The rewrite gets an XID before it waits, and once
+# s2 commits, the decoding worker would wait for that XID, which is a deadlock.
+session s3
+setup { SET lock_timeout = 10; }
+step s3_rewrite_toast
+{
+ DO $$
+ BEGIN
+ EXECUTE format('REPACK %s',
+ (SELECT reltoastrelid::regclass FROM pg_class
+ WHERE relname = 'repack_toast'));
+ END;
+ $$;
+}
+# Empty step, so that s2 cannot go on until s3_rewrite_toast is done.
+step s3_noop { }
+
# Test if data changes introduced while one session is performing REPACK
# CONCURRENTLY find their way into the table.
permutation
@@ -178,3 +216,16 @@ permutation
s2_check
s2_wakeup_before_lock
s1_check
+
+# Same, but try to rewrite the TOAST relation while the decoding worker waits
+# for s2 to commit.
+permutation
+ s2_begin
+ s1_wait_before_lock
+ s3_rewrite_toast(*)
+ s3_noop
+ s2_commit
+ s2_updates
+ s2_check
+ s2_wakeup_before_lock
+ s1_check
--
2.37.1 (Apple Git-137.1)
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-23 16:22 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-23 18:27 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 03:51 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
2026-09-25 06:24 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Antonin Houska <ah@cybertec.at>
2026-09-25 12:12 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-25 14:56 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Álvaro Herrera <alvherre@kurilemu.de>
2026-09-25 17:20 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-25 18:10 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten shihao zhong <zhong950419@gmail.com>
@ 2026-09-25 19:47 ` Manu <manuelreyesbravo@gmail.com>
0 siblings, 0 replies; 28+ messages in thread
From: Manu @ 2026-09-25 19:47 UTC (permalink / raw)
To: shihao zhong <zhong950419@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; Masahiko Sawada <sawada.mshk@gmail.com>; Thom Brown <thom@linux.com>; Antonin Houska <ah@cybertec.at>; Robert Treat <rob@xzilla.net>; pgsql-hackers@lists.postgresql.org
Hi,
shihao zhong <zhong950419@gmail.com> wrote:
> Done in v5. 0001 is Álvaro's version as one commit, with that comment
> added and a shorter commit message. 0002 fixes the decoding_ctx comment
> in copy_table_data().
I ran v5 through the same checks as v3, on master and on
REL_19_STABLE, where it applies cleanly.
With the test but without the code change, repack_toast fails in
s1_check: 10 rows of s2's updates are missing, not just the lock
timeout line. With v5 it passed 30 of 30 runs, and 30 of 30 with the
run pinned to one CPU, since the new permutation depends on
lock_timeout and the (*) marker.
The core regression and isolation suites pass on both branches. In
the behaviour checks no update was lost. A rewrite or REINDEX of the
TOAST relation from a transaction the worker waits for now ends in a
deadlock error, as discussed: REPACK is the one cancelled at a 1s
deadlock_timeout, the other session at 5s.
The output and scripts are attached.
Regards,
Manu
Checks of v5 (0001 + 0002)
v5: master 3e9bc39c5d7 + v5
ctl5: the same with src/backend reverted (v5's test, no fix)
v5r19: REL_19_STABLE a4052efe2aa + v5
All --enable-cassert --enable-injection-points.
===== output of v5_all.sh =====
== build: /home/manu/pgtoast-i-ctl5 (postgres (PostgreSQL) 20devel), deadlock_timeout 1s
race 1 REPACK ok 2.5s value oldoldold
race 2 REPACK ok 2.5s value oldoldold
race 3 REPACK ok 2.5s value oldoldold
race 4 REPACK ok 2.5s value oldoldold
race 5 REPACK ok 2.5s value oldoldold
hammer 20s REPACK ok 3.5s value NEWNEWNEW
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK ok 2.0s value NEWNEWNEW,x
session 1 (REINDEX TABLE pg_toast.pg_toast_31349): ok
xidrewrite REPACK ok 2.0s value oldoldold,x
session 1 (CLUSTER pg_toast.pg_toast_31377 USING pg_toast_31377_index): ok
xidalter REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 2026188 waits for ShareLock on transaction 4430; blocked by process 2026146.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 16:29:54.813 -03 [2026188] ERROR: deadlock detected
log: 2026-09-25 16:29:54.816 -03 [2026187] ERROR: deadlock detected
== build: /home/manu/pgtoast-i-v5 (postgres (PostgreSQL) 20devel), deadlock_timeout 1s
race 1 REPACK deadlock detected 3.6s value NEWNEWNEW
DETAIL: Process 2026286 waits for ShareLock on transaction 668; blocked by process 2026340.
HINT: See server log for query details.
race 2 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 2026398 waits for ShareLock on transaction 675; blocked by process 2026401.
HINT: See server log for query details.
race 3 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 2026492 waits for ShareLock on transaction 682; blocked by process 2026495.
HINT: See server log for query details.
race 4 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 2026575 waits for ShareLock on transaction 689; blocked by process 2026625.
HINT: See server log for query details.
race 5 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 2026684 waits for ShareLock on transaction 696; blocked by process 2026717.
HINT: See server log for query details.
hammer 20s REPACK deadlock detected 4.5s value NEWNEWNEW
DETAIL: Process 2026819 waits for ShareLock on transaction 704; blocked by process 2026823.
HINT: See server log for query details.
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 2033502 waits for ShareLock on transaction 3783; blocked by process 2033490.
HINT: See server log for query details.
session 1 (REINDEX TABLE pg_toast.pg_toast_28785): ok
xidrewrite REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 2033578 waits for ShareLock on transaction 3789; blocked by process 2033554.
HINT: See server log for query details.
session 1 (CLUSTER pg_toast.pg_toast_28800 USING pg_toast_28800_index): ok
xidalter REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 2033692 waits for ShareLock on transaction 3795; blocked by process 2033686.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 16:30:00.653 -03 [2026286] ERROR: deadlock detected
log: 2026-09-25 16:30:00.655 -03 [2026285] ERROR: deadlock detected
log: 2026-09-25 16:30:04.858 -03 [2026398] ERROR: deadlock detected
== build: /home/manu/pgtoast-i-ctl5 (postgres (PostgreSQL) 20devel), deadlock_timeout 5s
race 1 REPACK ok 2.5s value oldoldold
race 2 REPACK ok 2.5s value oldoldold
race 3 REPACK ok 2.5s value oldoldold
race 4 REPACK ok 2.5s value oldoldold
race 5 REPACK ok 2.5s value oldoldold
hammer 20s REPACK ok 3.5s value NEWNEWNEW
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK ok 2.0s value NEWNEWNEW,x
session 1 (REINDEX TABLE pg_toast.pg_toast_32397): ok
xidrewrite REPACK ok 2.0s value oldoldold,x
session 1 (CLUSTER pg_toast.pg_toast_32422 USING pg_toast_32422_index): ok
xidalter REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 2042720 waits for ShareLock on transaction 4692; blocked by process 2042715.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 16:32:02.791 -03 [2042720] ERROR: deadlock detected
log: 2026-09-25 16:32:02.794 -03 [2042719] ERROR: deadlock detected
== build: /home/manu/pgtoast-i-v5 (postgres (PostgreSQL) 20devel), deadlock_timeout 5s
race 1 REPACK ok 6.0s value NEWNEWNEW
race 2 REPACK ok 6.0s value NEWNEWNEW
race 3 REPACK ok 6.0s value NEWNEWNEW
race 4 REPACK ok 6.0s value NEWNEWNEW
race 5 REPACK ok 6.0s value NEWNEWNEW
hammer 20s REPACK ok 5.1s value NEWNEWNEW
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 2049799 waits for ShareLock on transaction 3479; blocked by process 2049792.
HINT: See server log for query details.
session 1 (REINDEX TABLE pg_toast.pg_toast_27577): ok
xidrewrite REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 2049978 waits for ShareLock on transaction 3485; blocked by process 2049971.
HINT: See server log for query details.
session 1 (CLUSTER pg_toast.pg_toast_27592 USING pg_toast_27592_index): ok
xidalter REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 2050106 waits for ShareLock on transaction 3491; blocked by process 2050100.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 16:32:10.940 -03 [2042937] ERROR: deadlock detected
log: 2026-09-25 16:32:18.487 -03 [2043020] ERROR: deadlock detected
log: 2026-09-25 16:32:26.034 -03 [2043162] ERROR: deadlock detected
== injection_points check, ctl5
ok 1 - injection_points 69 ms
ok 2 - hashagg 10 ms
ok 3 - reindex_conc 10 ms
ok 4 - vacuum 8 ms
# All 4 tests passed.
ok 1 - basic 43 ms
ok 2 - inplace 588 ms
ok 3 - reindex_concurrently_deferred 95 ms
ok 4 - repack 167 ms
ok 5 - repack_commit_race 492 ms
ok 6 - repack_decode 248 ms
ok 7 - repack_missingval 128 ms
ok 8 - repack_temporal 140 ms
ok 9 - repack_temporal_multirange 139 ms
not ok 10 - repack_toast 497 ms
ok 11 - ri_fastpath_reindex 317 ms
ok 12 - ri_fastpath_snapshot 33 ms
ok 13 - syscache-update-pruned 516 ms
ok 14 - wait_cleanup 61 ms
ok 15 - heap_lock_update 119 ms
ok 16 - on_conflict_probe_window 146 ms
# 1 of 16 tests failed.
== injection_points check, v5
ok 1 - injection_points 70 ms
ok 2 - hashagg 12 ms
ok 3 - reindex_conc 10 ms
ok 4 - vacuum 10 ms
# All 4 tests passed.
ok 1 - basic 57 ms
ok 2 - inplace 605 ms
ok 3 - reindex_concurrently_deferred 101 ms
ok 4 - repack 172 ms
ok 5 - repack_commit_race 497 ms
ok 6 - repack_decode 249 ms
ok 7 - repack_missingval 130 ms
ok 8 - repack_temporal 140 ms
ok 9 - repack_temporal_multirange 140 ms
ok 10 - repack_toast 506 ms
ok 11 - ri_fastpath_reindex 326 ms
ok 12 - ri_fastpath_snapshot 59 ms
ok 13 - syscache-update-pruned 441 ms
ok 14 - wait_cleanup 65 ms
ok 15 - heap_lock_update 122 ms
ok 16 - on_conflict_probe_window 185 ms
# All 16 tests passed.
== injection_points check, v5r19
ok 1 - injection_points 70 ms
ok 2 - hashagg 12 ms
ok 3 - reindex_conc 11 ms
ok 4 - vacuum 9 ms
# All 4 tests passed.
ok 1 - basic 49 ms
ok 2 - inplace 493 ms
ok 3 - reindex_concurrently_deferred 92 ms
ok 4 - repack 174 ms
ok 5 - repack_commit_race 457 ms
ok 6 - repack_decode 251 ms
ok 7 - repack_missingval 131 ms
ok 8 - repack_temporal 142 ms
ok 9 - repack_temporal_multirange 140 ms
ok 10 - repack_toast 511 ms
ok 11 - ri_fastpath_reindex 241 ms
ok 12 - ri_fastpath_snapshot 59 ms
ok 13 - syscache-update-pruned 427 ms
ok 14 - wait_cleanup 63 ms
ok 15 - heap_lock_update 104 ms
ok 16 - on_conflict_probe_window 149 ms
# All 16 tests passed.
== repack_toast alone on v5
repack_toast, normal: 30 passed, 0 failed of 30
repack_toast, one-cpu: 30 passed, 0 failed of 30
== core regression + isolation, v5
# All 239 tests passed.
# All 133 tests passed.
== core regression + isolation, v5r19
# All 239 tests passed.
# All 133 tests passed.
V5-ALL-DONE
===== repack_toast.diffs on ctl5 =====
diff -U3 /home/manu/pgtoast-ctl5/src/test/modules/injection_points/expected/repack_toast.out /home/manu/pgtoast-ctl5/src/test/modules/injection_points/output_iso/results/repack_toast.out
--- /home/manu/pgtoast-ctl5/src/test/modules/injection_points/expected/repack_toast.out 2026-09-25 16:27:31.345804460 -0300
+++ /home/manu/pgtoast-ctl5/src/test/modules/injection_points/output_iso/results/repack_toast.out 2026-09-25 16:33:39.158553380 -0300
@@ -153,7 +153,6 @@
$$;
<waiting ...>
step s3_rewrite_toast: <... completed>
-ERROR: canceling statement due to lock timeout
step s3_noop:
step s2_commit:
COMMIT;
@@ -264,9 +263,29 @@
4
(1 row)
-i|d1_j|d1_k|i|d2_j|d2_k|d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst
--+----+----+-+----+----+--------+--------+--------+--------
-(0 rows)
+ i|d1_j |d1_k | i|d2_j |d2_k |d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst
+--+------------+------------+--+------------+------------+--------+--------+--------+--------
+ | | | 3|MTLOQXVAQMNK|e93028bdc1aa| | 21233| | 0
+ 3|NAEPUVPXSSYJ|e93028bdc1aa| | | | 21212| | 0|
+ | | | 4|a9b7ba70783b|1bd69c7df311| | 21235| | 0
+ 4|ENOUIRMPNQBG|1bd69c7df311| | | | 21213| | 0|
+ | | | 5|08f90c1a4171|a35fe7f7fe82| | 21237| | 0
+ 5|OXXFSAYTLVSM|a35fe7f7fe82| | | | 21214| | 0|
+ | | | 6|CTSKKFLJLRKC|a8c6dd982010| | 21240| | 21239
+ 6|UNTFMJLQFINS|a8c6dd982010| | | | 21215| | 0|
+ | | | 7|DSWDHEUAMEJS|WOTJBBSSWUXQ| | 21244| | 21243
+ 7|UCVTCQCOMSSV|708be71b9ab6| | | | 21216| | 0|
+ | | |13|KOMFHVXDHWMF|276756f75e1e| | 21234| | 0
+13|XQRQLOJAIWIK|276756f75e1e| | | | 21222| | 0|
+ | | |14|a9b7ba70783b|276756f75e1e| | 21236| | 0
+14|CWSRBLODIBBP|276756f75e1e| | | | 21223| | 0|
+ | | |15|08f90c1a4171|276756f75e1e| | 21238| | 0
+15|MOEWNGRTFMJW|276756f75e1e| | | | 21224| | 0|
+ | | |16|ELUKGVABNCQN|276756f75e1e| | 21242| | 21241
+16|HBTFGNMCAHJF|276756f75e1e| | | | 21225| | 0|
+ | | |17|OKQUBIQHPEII|APAYNYNUEWFD| | 21246| | 21245
+17|SGFDJRVYLYUF|276756f75e1e| | | | 21226| | 0|
+(20 rows)
injection_points_detach
-----------------------
===== build_v5.sh =====
#!/bin/bash
# Three builds for the v5 review, --enable-cassert --enable-injection-points:
# v5 current master + v5-0001 + v5-0002
# ctl5 the same, with src/backend reverted: v5's test without the fix
# v5r19 current REL_19_STABLE + v5-0001 + v5-0002 (does it apply there?)
set -eu
SRC=$HOME/Proyectos/postgresql
git -C $SRC fetch -q origin
P=$(cd "$(dirname "$0")" && pwd)/shihao-v5
build() { # name base [revert]
local name=$1 base=$2 revert=${3:-}
local tree=$HOME/pgtoast-$name
if [ ! -d $tree ]; then
git -C $SRC worktree add -q --detach $tree $base
git -C $tree am -q $P/v5-0001-*.patch $P/v5-0002-*.patch
if [ -n "$revert" ]; then
git -C $tree checkout -q HEAD~2 -- src/backend
fi
fi
cd $tree
./configure --prefix=$HOME/pgtoast-i-$name --enable-cassert --enable-injection-points \
--enable-tap-tests > $HOME/pgtoast-configure-$name.log 2>&1
make -j"$(nproc)" -s > $HOME/pgtoast-make-$name.log 2>&1
make -s install > $HOME/pgtoast-install-$name.log 2>&1
make -C src/test/modules/injection_points -s install >> $HOME/pgtoast-install-$name.log 2>&1
echo "$name: base $(git -C $tree rev-parse --short HEAD~2), warnings=$(grep -c 'warning:' $HOME/pgtoast-make-$name.log), backend diff vs base: $(git -C $tree diff --stat HEAD~2 -- src/backend | tail -1)"
}
build v5 origin/master &
build ctl5 origin/master revert &
build v5r19 origin/REL_19_STABLE &
wait
echo BUILD-DONE
===== v5_all.sh =====
#!/bin/bash
# Everything for the v5 review, in one run:
# 1. behaviour checks (v3_check.sh) on ctl5 (v5's test, no fix) and on v5,
# at deadlock_timeout 1s and 5s
# 2. the injection_points tests on ctl5, v5 and v5r19 (REL_19_STABLE):
# repack_toast should fail on ctl5 only
# 3. repack_toast alone RUNS times on v5, then RUNS times with the whole
# run pinned to one CPU, to look for timing-dependent output (the new
# permutation relies on lock_timeout = 10 and the (*) marker)
# 4. core regression + isolation on v5 and v5r19
set -u
A=$(cd "$(dirname "$0")" && pwd)
RUNS=${RUNS:-30}
for dt in 1s 5s; do
for b in ctl5 v5; do
DT=$dt bash $A/v3_check.sh $HOME/pgtoast-i-$b
echo
done
done
for b in ctl5 v5 v5r19; do
echo "== injection_points check, $b"
make -C $HOME/pgtoast-$b/src/test/modules/injection_points check 2>&1 \
| grep -E '^(ok|not ok|# All|# [0-9]+ of)|tests (passed|failed)' | tail -30
[ $b = ctl5 ] && cp $HOME/pgtoast-ctl5/src/test/modules/injection_points/output_iso/regression.diffs \
$A/v5_ctl5_repack_toast.diffs 2>/dev/null
done
rt() { # label prefix...
local label=$1; shift
local pass=0 fail=0
for i in $(seq $RUNS); do
if "$@" make -C $HOME/pgtoast-v5/src/test/modules/injection_points check \
REGRESS= ISOLATION=repack_toast > /tmp/claude-1000/rt5_$i.log 2>&1; then
pass=$((pass + 1))
else
fail=$((fail + 1)); cp /tmp/claude-1000/rt5_$i.log $A/v5_repack_toast_fail_${label}_$i.log
cp $HOME/pgtoast-v5/src/test/modules/injection_points/output_iso/regression.diffs \
$A/v5_repack_toast_fail_${label}_$i.diffs 2>/dev/null
fi
done
echo "repack_toast, $label: $pass passed, $fail failed of $RUNS"
}
echo "== repack_toast alone on v5"
rt normal env
rt one-cpu taskset -c 0
for b in v5 v5r19; do
echo "== core regression + isolation, $b"
make -C $HOME/pgtoast-$b -s check 2>&1 | grep -E '# All|# [0-9]+ of|not ok' | tail -5
make -C $HOME/pgtoast-$b/src/test/isolation -s check 2>&1 | grep -E '# All|# [0-9]+ of|not ok' | tail -5
done
echo V5-ALL-DONE
===== v3_check.sh (behaviour checks, unchanged since the v3 review) =====
#!/usr/bin/env bash
# The same checks as v2_check.sh, for v3 of the fix: REPACK (CONCURRENTLY)
# now locks the TOAST relation before the decoding worker starts, so a
# rewrite of it waits for REPACK. Every case runs against a build and
# reports REPACK's own outcome (ok or its error), how long it took, and the
# final value of the updated rows.
#
# New in v3_check.sh:
# xidalter a transaction that already has an XID runs ALTER TABLE on the
# table itself while the worker waits for it (v3's commit
# message says master already deadlocks here).
#
# race Thom's case without an injection point: an open transaction
# keeps the worker waiting, VACUUM FULL rewrites the TOAST
# relation meanwhile, an UPDATE of the TOASTed column commits
# right after the transaction ends. N attempts.
# hammer VACUUM FULL of the TOAST relation in a loop for SECS seconds,
# over the whole startup of REPACK (the v1 retry loop spun here).
# none the same, with no rewrite at all: the normal path.
# xidlock a transaction that already has an XID locks the TOAST
# relation (REINDEX of it) while the worker waits for it: taking
# the TOAST lock before starting the worker deadlocks here.
# xidrewrite the same, but the transaction rewrites the TOAST relation
# (CLUSTER of it) before committing.
#
# v2_check.sh <install dir> [case ...]
set -u
B=$1; shift
CASES=${*:-race hammer none xidlock xidrewrite xidalter}
N=${N:-5}
SECS=${SECS:-20}
D=${D:-$HOME/pgprog/data_v3check}
P=${P:-55711}
LOG=$HOME/pgprog/v3check.log
OUT=$HOME/pgprog/v3check-repack.out
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
rm -rf "$D" "$LOG"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<EOF
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
deadlock_timeout = ${DT:-1s}
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l "$LOG" -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
fresh() { # a new table with three TOASTed rows; sets TOAST
q "DROP TABLE IF EXISTS test" >/dev/null
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
}
start_repack() { # in the background; sets REPACK and T0
T0=$(date +%s.%N)
( q "REPACK (CONCURRENTLY) test" > "$OUT" 2>&1; date +%s.%N > "$OUT.end" ) &
REPACK=$!
}
report() { # label
local limit=$((SECS + 60)) waited=0
while kill -0 $REPACK 2>/dev/null && [ $waited -lt $limit ]; do sleep 1; waited=$((waited+1)); done
wait 2>/dev/null
local secs=$(echo "$(cat "$OUT.end" 2>/dev/null || date +%s.%N) - $T0" | bc)
local repack=$(grep -m1 -E 'ERROR|FATAL' "$OUT" | sed 's/^.*\(ERROR\|FATAL\): *//')
local value=$(q "SELECT string_agg(DISTINCT left(big, 9), ',') FROM test")
printf ' %-12s REPACK %-50s %5.1fs value %s\n' "$1" "${repack:-ok}" "$secs" "$value"
[ -z "$repack" ] || grep -E '^(DETAIL|HINT):' "$OUT" | sed 's/^/ /'
}
open_xact() { # seconds: a transaction with an XID, closed after the sleep
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep($1)" -c "COMMIT" >/dev/null 2>&1 ) &
XACT=$!
}
update_new() {
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null
}
echo "== build: $B ($("$B/bin/postgres" --version)), deadlock_timeout ${DT:-1s}"
for c in $CASES; do
case $c in
race)
for i in $(seq 1 $N); do
fresh; open_xact 3; sleep 0.5
start_repack; sleep 1
q "VACUUM FULL $TOAST" >/dev/null
wait $XACT; update_new
report "race $i"
done ;;
hammer)
fresh; open_xact 4; sleep 0.5
( until_t=$((SECONDS + SECS))
while [ $SECONDS -lt $until_t ]; do q "VACUUM FULL $TOAST" >/dev/null; done ) &
HAMMER=$!
start_repack
wait $XACT; update_new
report "hammer ${SECS}s"
wait $HAMMER 2>/dev/null ;;
none)
for i in $(seq 1 3); do
fresh; open_xact 3; sleep 0.5
start_repack
wait $XACT; update_new
report "none $i"
done ;;
xidlock|xidrewrite|xidalter)
fresh
# LOCK TABLE is refused on a TOAST relation, so use commands that
# lock it for real: REINDEX takes ShareLock on it without a rewrite,
# CLUSTER rewrites it (new relfilenumber). xidalter locks the table
# itself instead.
if [ $c = xidlock ]; then
stmt="REINDEX TABLE $TOAST"
elif [ $c = xidalter ]; then
stmt="ALTER TABLE test ADD COLUMN extra int"
else
idx=$(q "SELECT c.relname FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid WHERE i.indrelid = '$TOAST'::regclass")
stmt="CLUSTER $TOAST USING $idx"
fi
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "INSERT INTO test VALUES (100, 'x')" -c "SELECT pg_sleep(1.5)" \
-c "$stmt" -c "SELECT pg_sleep(1)" -c "COMMIT" > "$OUT.s1" 2>&1 ) &
S1=$!
sleep 0.5; start_repack
wait $S1
s1=$(grep -m1 -E 'ERROR' "$OUT.s1" | sed 's/^.*ERROR: *//')
update_new
report "$c"
echo " session 1 ($stmt): ${s1:-ok}" ;;
esac
done
grep -E 'deadlock detected' "$LOG" | head -3 | sed 's/^/ log: /'
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
Attachments:
[text/plain] nocfbot-repack-toast-v5-check.txt (25.1K, ../../179036565943.2118102.16573869613641283994@gmail.com/2-nocfbot-repack-toast-v5-check.txt)
download | inline:
Checks of v5 (0001 + 0002)
v5: master 3e9bc39c5d7 + v5
ctl5: the same with src/backend reverted (v5's test, no fix)
v5r19: REL_19_STABLE a4052efe2aa + v5
All --enable-cassert --enable-injection-points.
===== output of v5_all.sh =====
== build: /home/manu/pgtoast-i-ctl5 (postgres (PostgreSQL) 20devel), deadlock_timeout 1s
race 1 REPACK ok 2.5s value oldoldold
race 2 REPACK ok 2.5s value oldoldold
race 3 REPACK ok 2.5s value oldoldold
race 4 REPACK ok 2.5s value oldoldold
race 5 REPACK ok 2.5s value oldoldold
hammer 20s REPACK ok 3.5s value NEWNEWNEW
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK ok 2.0s value NEWNEWNEW,x
session 1 (REINDEX TABLE pg_toast.pg_toast_31349): ok
xidrewrite REPACK ok 2.0s value oldoldold,x
session 1 (CLUSTER pg_toast.pg_toast_31377 USING pg_toast_31377_index): ok
xidalter REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 2026188 waits for ShareLock on transaction 4430; blocked by process 2026146.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 16:29:54.813 -03 [2026188] ERROR: deadlock detected
log: 2026-09-25 16:29:54.816 -03 [2026187] ERROR: deadlock detected
== build: /home/manu/pgtoast-i-v5 (postgres (PostgreSQL) 20devel), deadlock_timeout 1s
race 1 REPACK deadlock detected 3.6s value NEWNEWNEW
DETAIL: Process 2026286 waits for ShareLock on transaction 668; blocked by process 2026340.
HINT: See server log for query details.
race 2 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 2026398 waits for ShareLock on transaction 675; blocked by process 2026401.
HINT: See server log for query details.
race 3 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 2026492 waits for ShareLock on transaction 682; blocked by process 2026495.
HINT: See server log for query details.
race 4 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 2026575 waits for ShareLock on transaction 689; blocked by process 2026625.
HINT: See server log for query details.
race 5 REPACK deadlock detected 3.7s value NEWNEWNEW
DETAIL: Process 2026684 waits for ShareLock on transaction 696; blocked by process 2026717.
HINT: See server log for query details.
hammer 20s REPACK deadlock detected 4.5s value NEWNEWNEW
DETAIL: Process 2026819 waits for ShareLock on transaction 704; blocked by process 2026823.
HINT: See server log for query details.
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 2033502 waits for ShareLock on transaction 3783; blocked by process 2033490.
HINT: See server log for query details.
session 1 (REINDEX TABLE pg_toast.pg_toast_28785): ok
xidrewrite REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 2033578 waits for ShareLock on transaction 3789; blocked by process 2033554.
HINT: See server log for query details.
session 1 (CLUSTER pg_toast.pg_toast_28800 USING pg_toast_28800_index): ok
xidalter REPACK deadlock detected 1.0s value NEWNEWNEW,x
DETAIL: Process 2033692 waits for ShareLock on transaction 3795; blocked by process 2033686.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 16:30:00.653 -03 [2026286] ERROR: deadlock detected
log: 2026-09-25 16:30:00.655 -03 [2026285] ERROR: deadlock detected
log: 2026-09-25 16:30:04.858 -03 [2026398] ERROR: deadlock detected
== build: /home/manu/pgtoast-i-ctl5 (postgres (PostgreSQL) 20devel), deadlock_timeout 5s
race 1 REPACK ok 2.5s value oldoldold
race 2 REPACK ok 2.5s value oldoldold
race 3 REPACK ok 2.5s value oldoldold
race 4 REPACK ok 2.5s value oldoldold
race 5 REPACK ok 2.5s value oldoldold
hammer 20s REPACK ok 3.5s value NEWNEWNEW
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK ok 2.0s value NEWNEWNEW,x
session 1 (REINDEX TABLE pg_toast.pg_toast_32397): ok
xidrewrite REPACK ok 2.0s value oldoldold,x
session 1 (CLUSTER pg_toast.pg_toast_32422 USING pg_toast_32422_index): ok
xidalter REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 2042720 waits for ShareLock on transaction 4692; blocked by process 2042715.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 16:32:02.791 -03 [2042720] ERROR: deadlock detected
log: 2026-09-25 16:32:02.794 -03 [2042719] ERROR: deadlock detected
== build: /home/manu/pgtoast-i-v5 (postgres (PostgreSQL) 20devel), deadlock_timeout 5s
race 1 REPACK ok 6.0s value NEWNEWNEW
race 2 REPACK ok 6.0s value NEWNEWNEW
race 3 REPACK ok 6.0s value NEWNEWNEW
race 4 REPACK ok 6.0s value NEWNEWNEW
race 5 REPACK ok 6.0s value NEWNEWNEW
hammer 20s REPACK ok 5.1s value NEWNEWNEW
none 1 REPACK ok 2.5s value NEWNEWNEW
none 2 REPACK ok 2.5s value NEWNEWNEW
none 3 REPACK ok 2.5s value NEWNEWNEW
xidlock REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 2049799 waits for ShareLock on transaction 3479; blocked by process 2049792.
HINT: See server log for query details.
session 1 (REINDEX TABLE pg_toast.pg_toast_27577): ok
xidrewrite REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 2049978 waits for ShareLock on transaction 3485; blocked by process 2049971.
HINT: See server log for query details.
session 1 (CLUSTER pg_toast.pg_toast_27592 USING pg_toast_27592_index): ok
xidalter REPACK deadlock detected 5.0s value NEWNEWNEW,x
DETAIL: Process 2050106 waits for ShareLock on transaction 3491; blocked by process 2050100.
HINT: See server log for query details.
session 1 (ALTER TABLE test ADD COLUMN extra int): ok
log: 2026-09-25 16:32:10.940 -03 [2042937] ERROR: deadlock detected
log: 2026-09-25 16:32:18.487 -03 [2043020] ERROR: deadlock detected
log: 2026-09-25 16:32:26.034 -03 [2043162] ERROR: deadlock detected
== injection_points check, ctl5
ok 1 - injection_points 69 ms
ok 2 - hashagg 10 ms
ok 3 - reindex_conc 10 ms
ok 4 - vacuum 8 ms
# All 4 tests passed.
ok 1 - basic 43 ms
ok 2 - inplace 588 ms
ok 3 - reindex_concurrently_deferred 95 ms
ok 4 - repack 167 ms
ok 5 - repack_commit_race 492 ms
ok 6 - repack_decode 248 ms
ok 7 - repack_missingval 128 ms
ok 8 - repack_temporal 140 ms
ok 9 - repack_temporal_multirange 139 ms
not ok 10 - repack_toast 497 ms
ok 11 - ri_fastpath_reindex 317 ms
ok 12 - ri_fastpath_snapshot 33 ms
ok 13 - syscache-update-pruned 516 ms
ok 14 - wait_cleanup 61 ms
ok 15 - heap_lock_update 119 ms
ok 16 - on_conflict_probe_window 146 ms
# 1 of 16 tests failed.
== injection_points check, v5
ok 1 - injection_points 70 ms
ok 2 - hashagg 12 ms
ok 3 - reindex_conc 10 ms
ok 4 - vacuum 10 ms
# All 4 tests passed.
ok 1 - basic 57 ms
ok 2 - inplace 605 ms
ok 3 - reindex_concurrently_deferred 101 ms
ok 4 - repack 172 ms
ok 5 - repack_commit_race 497 ms
ok 6 - repack_decode 249 ms
ok 7 - repack_missingval 130 ms
ok 8 - repack_temporal 140 ms
ok 9 - repack_temporal_multirange 140 ms
ok 10 - repack_toast 506 ms
ok 11 - ri_fastpath_reindex 326 ms
ok 12 - ri_fastpath_snapshot 59 ms
ok 13 - syscache-update-pruned 441 ms
ok 14 - wait_cleanup 65 ms
ok 15 - heap_lock_update 122 ms
ok 16 - on_conflict_probe_window 185 ms
# All 16 tests passed.
== injection_points check, v5r19
ok 1 - injection_points 70 ms
ok 2 - hashagg 12 ms
ok 3 - reindex_conc 11 ms
ok 4 - vacuum 9 ms
# All 4 tests passed.
ok 1 - basic 49 ms
ok 2 - inplace 493 ms
ok 3 - reindex_concurrently_deferred 92 ms
ok 4 - repack 174 ms
ok 5 - repack_commit_race 457 ms
ok 6 - repack_decode 251 ms
ok 7 - repack_missingval 131 ms
ok 8 - repack_temporal 142 ms
ok 9 - repack_temporal_multirange 140 ms
ok 10 - repack_toast 511 ms
ok 11 - ri_fastpath_reindex 241 ms
ok 12 - ri_fastpath_snapshot 59 ms
ok 13 - syscache-update-pruned 427 ms
ok 14 - wait_cleanup 63 ms
ok 15 - heap_lock_update 104 ms
ok 16 - on_conflict_probe_window 149 ms
# All 16 tests passed.
== repack_toast alone on v5
repack_toast, normal: 30 passed, 0 failed of 30
repack_toast, one-cpu: 30 passed, 0 failed of 30
== core regression + isolation, v5
# All 239 tests passed.
# All 133 tests passed.
== core regression + isolation, v5r19
# All 239 tests passed.
# All 133 tests passed.
V5-ALL-DONE
===== repack_toast.diffs on ctl5 =====
diff -U3 /home/manu/pgtoast-ctl5/src/test/modules/injection_points/expected/repack_toast.out /home/manu/pgtoast-ctl5/src/test/modules/injection_points/output_iso/results/repack_toast.out
--- /home/manu/pgtoast-ctl5/src/test/modules/injection_points/expected/repack_toast.out 2026-09-25 16:27:31.345804460 -0300
+++ /home/manu/pgtoast-ctl5/src/test/modules/injection_points/output_iso/results/repack_toast.out 2026-09-25 16:33:39.158553380 -0300
@@ -153,7 +153,6 @@
$$;
<waiting ...>
step s3_rewrite_toast: <... completed>
-ERROR: canceling statement due to lock timeout
step s3_noop:
step s2_commit:
COMMIT;
@@ -264,9 +263,29 @@
4
(1 row)
-i|d1_j|d1_k|i|d2_j|d2_k|d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst
--+----+----+-+----+----+--------+--------+--------+--------
-(0 rows)
+ i|d1_j |d1_k | i|d2_j |d2_k |d1_j_tst|d2_j_tst|d1_k_tst|d2_k_tst
+--+------------+------------+--+------------+------------+--------+--------+--------+--------
+ | | | 3|MTLOQXVAQMNK|e93028bdc1aa| | 21233| | 0
+ 3|NAEPUVPXSSYJ|e93028bdc1aa| | | | 21212| | 0|
+ | | | 4|a9b7ba70783b|1bd69c7df311| | 21235| | 0
+ 4|ENOUIRMPNQBG|1bd69c7df311| | | | 21213| | 0|
+ | | | 5|08f90c1a4171|a35fe7f7fe82| | 21237| | 0
+ 5|OXXFSAYTLVSM|a35fe7f7fe82| | | | 21214| | 0|
+ | | | 6|CTSKKFLJLRKC|a8c6dd982010| | 21240| | 21239
+ 6|UNTFMJLQFINS|a8c6dd982010| | | | 21215| | 0|
+ | | | 7|DSWDHEUAMEJS|WOTJBBSSWUXQ| | 21244| | 21243
+ 7|UCVTCQCOMSSV|708be71b9ab6| | | | 21216| | 0|
+ | | |13|KOMFHVXDHWMF|276756f75e1e| | 21234| | 0
+13|XQRQLOJAIWIK|276756f75e1e| | | | 21222| | 0|
+ | | |14|a9b7ba70783b|276756f75e1e| | 21236| | 0
+14|CWSRBLODIBBP|276756f75e1e| | | | 21223| | 0|
+ | | |15|08f90c1a4171|276756f75e1e| | 21238| | 0
+15|MOEWNGRTFMJW|276756f75e1e| | | | 21224| | 0|
+ | | |16|ELUKGVABNCQN|276756f75e1e| | 21242| | 21241
+16|HBTFGNMCAHJF|276756f75e1e| | | | 21225| | 0|
+ | | |17|OKQUBIQHPEII|APAYNYNUEWFD| | 21246| | 21245
+17|SGFDJRVYLYUF|276756f75e1e| | | | 21226| | 0|
+(20 rows)
injection_points_detach
-----------------------
===== build_v5.sh =====
#!/bin/bash
# Three builds for the v5 review, --enable-cassert --enable-injection-points:
# v5 current master + v5-0001 + v5-0002
# ctl5 the same, with src/backend reverted: v5's test without the fix
# v5r19 current REL_19_STABLE + v5-0001 + v5-0002 (does it apply there?)
set -eu
SRC=$HOME/Proyectos/postgresql
git -C $SRC fetch -q origin
P=$(cd "$(dirname "$0")" && pwd)/shihao-v5
build() { # name base [revert]
local name=$1 base=$2 revert=${3:-}
local tree=$HOME/pgtoast-$name
if [ ! -d $tree ]; then
git -C $SRC worktree add -q --detach $tree $base
git -C $tree am -q $P/v5-0001-*.patch $P/v5-0002-*.patch
if [ -n "$revert" ]; then
git -C $tree checkout -q HEAD~2 -- src/backend
fi
fi
cd $tree
./configure --prefix=$HOME/pgtoast-i-$name --enable-cassert --enable-injection-points \
--enable-tap-tests > $HOME/pgtoast-configure-$name.log 2>&1
make -j"$(nproc)" -s > $HOME/pgtoast-make-$name.log 2>&1
make -s install > $HOME/pgtoast-install-$name.log 2>&1
make -C src/test/modules/injection_points -s install >> $HOME/pgtoast-install-$name.log 2>&1
echo "$name: base $(git -C $tree rev-parse --short HEAD~2), warnings=$(grep -c 'warning:' $HOME/pgtoast-make-$name.log), backend diff vs base: $(git -C $tree diff --stat HEAD~2 -- src/backend | tail -1)"
}
build v5 origin/master &
build ctl5 origin/master revert &
build v5r19 origin/REL_19_STABLE &
wait
echo BUILD-DONE
===== v5_all.sh =====
#!/bin/bash
# Everything for the v5 review, in one run:
# 1. behaviour checks (v3_check.sh) on ctl5 (v5's test, no fix) and on v5,
# at deadlock_timeout 1s and 5s
# 2. the injection_points tests on ctl5, v5 and v5r19 (REL_19_STABLE):
# repack_toast should fail on ctl5 only
# 3. repack_toast alone RUNS times on v5, then RUNS times with the whole
# run pinned to one CPU, to look for timing-dependent output (the new
# permutation relies on lock_timeout = 10 and the (*) marker)
# 4. core regression + isolation on v5 and v5r19
set -u
A=$(cd "$(dirname "$0")" && pwd)
RUNS=${RUNS:-30}
for dt in 1s 5s; do
for b in ctl5 v5; do
DT=$dt bash $A/v3_check.sh $HOME/pgtoast-i-$b
echo
done
done
for b in ctl5 v5 v5r19; do
echo "== injection_points check, $b"
make -C $HOME/pgtoast-$b/src/test/modules/injection_points check 2>&1 \
| grep -E '^(ok|not ok|# All|# [0-9]+ of)|tests (passed|failed)' | tail -30
[ $b = ctl5 ] && cp $HOME/pgtoast-ctl5/src/test/modules/injection_points/output_iso/regression.diffs \
$A/v5_ctl5_repack_toast.diffs 2>/dev/null
done
rt() { # label prefix...
local label=$1; shift
local pass=0 fail=0
for i in $(seq $RUNS); do
if "$@" make -C $HOME/pgtoast-v5/src/test/modules/injection_points check \
REGRESS= ISOLATION=repack_toast > /tmp/claude-1000/rt5_$i.log 2>&1; then
pass=$((pass + 1))
else
fail=$((fail + 1)); cp /tmp/claude-1000/rt5_$i.log $A/v5_repack_toast_fail_${label}_$i.log
cp $HOME/pgtoast-v5/src/test/modules/injection_points/output_iso/regression.diffs \
$A/v5_repack_toast_fail_${label}_$i.diffs 2>/dev/null
fi
done
echo "repack_toast, $label: $pass passed, $fail failed of $RUNS"
}
echo "== repack_toast alone on v5"
rt normal env
rt one-cpu taskset -c 0
for b in v5 v5r19; do
echo "== core regression + isolation, $b"
make -C $HOME/pgtoast-$b -s check 2>&1 | grep -E '# All|# [0-9]+ of|not ok' | tail -5
make -C $HOME/pgtoast-$b/src/test/isolation -s check 2>&1 | grep -E '# All|# [0-9]+ of|not ok' | tail -5
done
echo V5-ALL-DONE
===== v3_check.sh (behaviour checks, unchanged since the v3 review) =====
#!/usr/bin/env bash
# The same checks as v2_check.sh, for v3 of the fix: REPACK (CONCURRENTLY)
# now locks the TOAST relation before the decoding worker starts, so a
# rewrite of it waits for REPACK. Every case runs against a build and
# reports REPACK's own outcome (ok or its error), how long it took, and the
# final value of the updated rows.
#
# New in v3_check.sh:
# xidalter a transaction that already has an XID runs ALTER TABLE on the
# table itself while the worker waits for it (v3's commit
# message says master already deadlocks here).
#
# race Thom's case without an injection point: an open transaction
# keeps the worker waiting, VACUUM FULL rewrites the TOAST
# relation meanwhile, an UPDATE of the TOASTed column commits
# right after the transaction ends. N attempts.
# hammer VACUUM FULL of the TOAST relation in a loop for SECS seconds,
# over the whole startup of REPACK (the v1 retry loop spun here).
# none the same, with no rewrite at all: the normal path.
# xidlock a transaction that already has an XID locks the TOAST
# relation (REINDEX of it) while the worker waits for it: taking
# the TOAST lock before starting the worker deadlocks here.
# xidrewrite the same, but the transaction rewrites the TOAST relation
# (CLUSTER of it) before committing.
#
# v2_check.sh <install dir> [case ...]
set -u
B=$1; shift
CASES=${*:-race hammer none xidlock xidrewrite xidalter}
N=${N:-5}
SECS=${SECS:-20}
D=${D:-$HOME/pgprog/data_v3check}
P=${P:-55711}
LOG=$HOME/pgprog/v3check.log
OUT=$HOME/pgprog/v3check-repack.out
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
rm -rf "$D" "$LOG"
"$B/bin/initdb" -D "$D" -U postgres --no-sync -A trust >/dev/null 2>&1
cat >> "$D/postgresql.conf" <<EOF
wal_level = logical
max_replication_slots = 10
max_wal_senders = 10
deadlock_timeout = ${DT:-1s}
EOF
"$B/bin/pg_ctl" -D "$D" -o "-p $P" -l "$LOG" -w start >/dev/null 2>&1
q() { "$B/bin/psql" -p $P -U postgres -qtAX -c "$1" 2>&1; }
fresh() { # a new table with three TOASTed rows; sets TOAST
q "DROP TABLE IF EXISTS test" >/dev/null
q "CREATE TABLE test (id int PRIMARY KEY, big text)" >/dev/null
q "ALTER TABLE test ALTER COLUMN big SET STORAGE EXTERNAL" >/dev/null
q "INSERT INTO test SELECT g, repeat('old', 3000) FROM generate_series(1,3) g" >/dev/null
TOAST=$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class c2 ON c2.oid = c1.reltoastrelid WHERE c1.relname='test'")
}
start_repack() { # in the background; sets REPACK and T0
T0=$(date +%s.%N)
( q "REPACK (CONCURRENTLY) test" > "$OUT" 2>&1; date +%s.%N > "$OUT.end" ) &
REPACK=$!
}
report() { # label
local limit=$((SECS + 60)) waited=0
while kill -0 $REPACK 2>/dev/null && [ $waited -lt $limit ]; do sleep 1; waited=$((waited+1)); done
wait 2>/dev/null
local secs=$(echo "$(cat "$OUT.end" 2>/dev/null || date +%s.%N) - $T0" | bc)
local repack=$(grep -m1 -E 'ERROR|FATAL' "$OUT" | sed 's/^.*\(ERROR\|FATAL\): *//')
local value=$(q "SELECT string_agg(DISTINCT left(big, 9), ',') FROM test")
printf ' %-12s REPACK %-50s %5.1fs value %s\n' "$1" "${repack:-ok}" "$secs" "$value"
[ -z "$repack" ] || grep -E '^(DETAIL|HINT):' "$OUT" | sed 's/^/ /'
}
open_xact() { # seconds: a transaction with an XID, closed after the sleep
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "SELECT pg_current_xact_id()" -c "SELECT pg_sleep($1)" -c "COMMIT" >/dev/null 2>&1 ) &
XACT=$!
}
update_new() {
q "UPDATE test SET big = repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/null
}
echo "== build: $B ($("$B/bin/postgres" --version)), deadlock_timeout ${DT:-1s}"
for c in $CASES; do
case $c in
race)
for i in $(seq 1 $N); do
fresh; open_xact 3; sleep 0.5
start_repack; sleep 1
q "VACUUM FULL $TOAST" >/dev/null
wait $XACT; update_new
report "race $i"
done ;;
hammer)
fresh; open_xact 4; sleep 0.5
( until_t=$((SECONDS + SECS))
while [ $SECONDS -lt $until_t ]; do q "VACUUM FULL $TOAST" >/dev/null; done ) &
HAMMER=$!
start_repack
wait $XACT; update_new
report "hammer ${SECS}s"
wait $HAMMER 2>/dev/null ;;
none)
for i in $(seq 1 3); do
fresh; open_xact 3; sleep 0.5
start_repack
wait $XACT; update_new
report "none $i"
done ;;
xidlock|xidrewrite|xidalter)
fresh
# LOCK TABLE is refused on a TOAST relation, so use commands that
# lock it for real: REINDEX takes ShareLock on it without a rewrite,
# CLUSTER rewrites it (new relfilenumber). xidalter locks the table
# itself instead.
if [ $c = xidlock ]; then
stmt="REINDEX TABLE $TOAST"
elif [ $c = xidalter ]; then
stmt="ALTER TABLE test ADD COLUMN extra int"
else
idx=$(q "SELECT c.relname FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid WHERE i.indrelid = '$TOAST'::regclass")
stmt="CLUSTER $TOAST USING $idx"
fi
( "$B/bin/psql" -p $P -U postgres -qtAX \
-c "BEGIN" -c "INSERT INTO test VALUES (100, 'x')" -c "SELECT pg_sleep(1.5)" \
-c "$stmt" -c "SELECT pg_sleep(1)" -c "COMMIT" > "$OUT.s1" 2>&1 ) &
S1=$!
sleep 0.5; start_repack
wait $S1
s1=$(grep -m1 -E 'ERROR' "$OUT.s1" | sed 's/^.*ERROR: *//')
update_new
report "$c"
echo " session 1 ($stmt): ${s1:-ok}" ;;
esac
done
grep -E 'deadlock detected' "$LOG" | head -3 | sed 's/^/ log: /'
"$B/bin/pg_ctl" -D "$D" -m immediate -w stop >/dev/null 2>&1
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
@ 2026-09-23 15:06 ` Melanie Plageman <melanieplageman@gmail.com>
2026-09-23 17:09 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
1 sibling, 1 reply; 28+ messages in thread
From: Melanie Plageman @ 2026-09-23 15:06 UTC (permalink / raw)
To: Thom Brown <thom@linux.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Sep 22, 2026 at 8:23 PM Thom Brown <thom@linux.com> wrote:
>
> Whilst stress-testing REPACK (CONCURRENTLY) I managed to get it to silently
> throw away committed updates to a TOASTed column. There's no error, and both
> verify_heapam() and bt_index_check() seem to think everything is fine.
Should this be added as an open item? [1]
- Melanie
[1] https://wiki.postgresql.org/wiki/PostgreSQL_19_Open_Items
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 15:06 ` Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Melanie Plageman <melanieplageman@gmail.com>
@ 2026-09-23 17:09 ` Thom Brown <thom@linux.com>
0 siblings, 0 replies; 28+ messages in thread
From: Thom Brown @ 2026-09-23 17:09 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Wed, 23 Sept 2026 at 16:07, Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> On Tue, Sep 22, 2026 at 8:23 PM Thom Brown <thom@linux.com> wrote:
> >
> > Whilst stress-testing REPACK (CONCURRENTLY) I managed to get it to silently
> > throw away committed updates to a TOASTed column. There's no error, and both
> > verify_heapam() and bt_index_check() seem to think everything is fine.
>
> Should this be added as an open item? [1]
Yes. I've now added it.
Thanks.
Thom
^ permalink raw reply [nested|flat] 28+ messages in thread
end of thread, other threads:[~2026-09-25 19:47 UTC | newest]
Thread overview: 28+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23 00:23 REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten Thom Brown <thom@linux.com>
2026-09-23 00:42 ` Manu <manuelreyesbravo@gmail.com>
2026-09-23 05:09 ` shihao zhong <zhong950419@gmail.com>
2026-09-23 08:26 ` Thom Brown <thom@linux.com>
2026-09-23 12:08 ` shihao zhong <zhong950419@gmail.com>
2026-09-23 14:16 ` Manu <manuelreyesbravo@gmail.com>
2026-09-23 16:22 ` Antonin Houska <ah@cybertec.at>
2026-09-23 17:18 ` Thom Brown <thom@linux.com>
2026-09-24 07:57 ` Antonin Houska <ah@cybertec.at>
2026-09-24 08:32 ` Thom Brown <thom@linux.com>
2026-09-25 10:40 ` Thom Brown <thom@linux.com>
2026-09-23 18:27 ` Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-24 01:45 ` shihao zhong <zhong950419@gmail.com>
2026-09-24 03:08 ` Manu <manuelreyesbravo@gmail.com>
2026-09-24 03:51 ` Robert Treat <rob@xzilla.net>
2026-09-24 08:41 ` Antonin Houska <ah@cybertec.at>
2026-09-25 04:32 ` shihao zhong <zhong950419@gmail.com>
2026-09-25 06:13 ` Manu <manuelreyesbravo@gmail.com>
2026-09-25 06:24 ` Antonin Houska <ah@cybertec.at>
2026-09-25 12:12 ` Thom Brown <thom@linux.com>
2026-09-25 14:06 ` shihao zhong <zhong950419@gmail.com>
2026-09-25 14:56 ` Álvaro Herrera <alvherre@kurilemu.de>
2026-09-25 15:29 ` shihao zhong <zhong950419@gmail.com>
2026-09-25 17:20 ` Masahiko Sawada <sawada.mshk@gmail.com>
2026-09-25 18:10 ` shihao zhong <zhong950419@gmail.com>
2026-09-25 19:47 ` Manu <manuelreyesbravo@gmail.com>
2026-09-23 15:06 ` Melanie Plageman <melanieplageman@gmail.com>
2026-09-23 17:09 ` Thom Brown <thom@linux.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox