On Fri, 25 Sept 2026 at 07:24, Antonin Houska <[email protected]> wrote: > > shihao zhong <[email protected]> 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
