Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten

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