From: Manu <manuelreyesbravo@gmail.com>
To: Thom Brown <thom@linux.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: REPACK (CONCURRENTLY) can silently lose updates when the toast
 table is rewritten
Date: Tue, 22 Sep 2026 21:42:19 -0300
Message-ID: <179012413951.1850281.5077495683381671561@gmail.com>
In-Reply-To: 
 <CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com>
References: 
 <CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1338542497413467292=="
Archived-At: 
 <https://www.postgresql.org/message-id/179012413951.1850281.5077495683381671561%40gmail.com>
Precedence: bulk

--===============1338542497413467292==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

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

--===============1338542497413467292==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename="nocfbot-repack-toast-race.sh.txt"
MIME-Version: 1.0

#!/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=3D/home/manu/pgprog/i-serie
D=3D/home/manu/pgprog/data_carrera
P=3D55592
N=3D${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 =3D logical
max_replication_slots =3D 10
max_wal_senders =3D 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=3D0
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=3D$(q "SELECT 'pg_toast.' || c2.relname FROM pg_class c1 JOIN pg_class=
 c2 ON c2.oid =3D c1.reltoastrelid WHERE c1.relname=3D'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 "COM=
MIT" >/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 =3D repeat('NEW', 4000) WHERE id IN (1,2,3)" >/dev/nu=
ll 2>&1
	wait 2>/dev/null

	res=3D$(q "SELECT DISTINCT left(big,9) FROM test")
	if [ "$res" =3D "oldoldold" ]; then
		ganadas=3D$((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"

--===============1338542497413467292==--