On Wed, Sep 23, 2026 at 2:28 PM Masahiko Sawada <[email protected]> wrote:
>
> On Wed, Sep 23, 2026 at 9:23 AM Antonin Houska <[email protected]> wrote:
> >
> > shihao zhong <[email protected]> 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.
>
> Agreed.
>
> So I think the simplest fix would be to acquire a lock on the TOAST
> table before starting the repack worker. It would make the case in
> question fail with a deadlock, instead of silently losing updates.
>
> The proposed patch also fixes the problem, but I'm concerned that it
> repeatedly starts and stops the repack worker without any limit. I
> think we could error out if we detect a concurrent rewrite, so that
> users can re-run REPACK CONCURRENTLY. This check could also be done on
> the repack worker side: after getting the relfilelocator of the TOAST
> table and initializing the logical decoding, the repack worker
> rechecks the relfilelocator. If they don't match, it raises an error.
>

It feels a little off to me that if I am trying to REPACKCC, and
someone (maybe even myself, but certainly not Postgres) comes along
and runs a command the conflicts with my existing REPACKCC, that my
REPACKCC is canceled rather than having the other command either wait
or error out. That's a little more complicated a fix, with likely
heavier and/or longer held locks, but feels like it would be less
surprising for users.

Robert Treat
https://xzilla.net


Reply via email to