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
