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

shihao zhong <zhong950419(at)gmail(dot)com> wrote:

> > or whether the relfilenode should be re-checked after the snapshot is built
>
> Holding the toast lock from the start deadlocks. A session that asks for
> AccessExclusiveLock gets an XID before it waits, and the decoding worker
> waits for all XIDs while it sets up.

The same (supposedly low) deadlock risk already exists for the main table, see
this comment in rebuild_relation():

/*
* Start the worker that decodes data changes applied while we're
* copying the table contents.
*
* Note that the worker has to wait for all transactions with XID
* already assigned to finish. If some of those transactions is
* waiting for a lock conflicting with ShareUpdateExclusiveLock on our
* table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
* Not sure this risk is worth unlocking/locking the table (and its
* clustering index) and checking again if it's still eligible for
* REPACK CONCURRENTLY.
*/
start_repack_decoding_worker(tableOid);

I'm not sure if locking the TOAST relation earlier would make the situation
worse.

The reason TOAST relation is not locked until copy_table_data() does so is
that CLUSTER / VACUUM FULL in v18 did it this way (not sure what the reason
for such design was). I haven't changed that for REPACK exactly because I
failed to envision this stale relfilenode issue.

--
Antonin Houska
Web: https://www.cybertec-postgresql.com