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
