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

On Fri, 25 Sept 2026 at 07:24, Antonin Houska <ah(at)cybertec(dot)at> wrote:
>
> shihao zhong <zhong950419(at)gmail(dot)com> wrote:
>
> > > (What I said does not mean that I'm in favor of restarting the decoding worker
> > > either. I still prefer locking the TOAST relation early, as I noted elsewhere
> > > in the thread.)
> >
> > OK. v3 locks the TOAST relation before the worker starts, as Sawada-san
> > first suggested. A rewrite of the TOAST relation now waits for REPACK,
> > which I think is also what Robert asked for.
>
> Thanks for the patch. I'm just not sure this is the best place to lock the
> TOAST table: note that copy_table_data() locks it again.
>
> I'd prefer locking it close to the place we lock the main table (perhaps in
> cluster_rel(), after all the checks have been done?) and replace the locking
> statements (both in the copy_table_data() and in your patch) with
> Assert(CheckRelationLockedByMe(...)).

Moving it into cluster_rel() makes sense, but does it need to be the
full ShareUpdateExclusiveLock there?

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? If so, I guess it would also
avoid the case Manu found, where a REINDEX of the TOAST relation from
a transaction that already has an XID deadlocks during startup, as
REINDEX doesn't need a lock that strong.

The stronger lock could presumably still be taken later, where it is
now, once the worker has finished waiting for other transactions, so
waiting for it there couldn't turn into a deadlock. But I may well be
missing a reason it has to be the stronger one from the start.

Thom