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
v4-0002-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch
Description: Binary data
v4-0001-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch
Description: Binary data
