From: Antonin Houska <ah@cybertec.at>
To: Robert Treat <rob@xzilla.net>
cc: Masahiko Sawada <sawada.mshk@gmail.com>,
    shihao zhong <zhong950419@gmail.com>,
    Manu <manuelreyesbravo@gmail.com>, Thom Brown <thom@linux.com>,
    pgsql-hackers@lists.postgresql.org
Subject: Re: REPACK (CONCURRENTLY) can silently lose updates when the toast
 table is rewritten
In-reply-to: 
 <CABV9wwMQ7-hQnOFc4yYo6oJv8eLE9OV0khFrpespwAkOKugsqw@mail.gmail.com>
References: 
 <CAA-aLv5MF6BLL+BWvix2Yw+CBardtH43AofPReQunhDZPNBtuA@mail.gmail.com>
 <179012413951.1850281.5077495683381671561@gmail.com>
 <CAGRkXqRYLtBRaMzdH+e7PMO-BRaWPPo37gvOx3C=jQ1uP4Cx7w@mail.gmail.com>
 <47479.1790180571@localhost>
 <CAD21AoCYvsp5HFOXLx9BpSz6Lcyw7H9-RXtq-b=AA8joP8=-XA@mail.gmail.com>
 <CABV9wwMQ7-hQnOFc4yYo6oJv8eLE9OV0khFrpespwAkOKugsqw@mail.gmail.com>
Comments: In-reply-to Robert Treat <rob@xzilla.net>
   message dated "Wed, 23 Sep 2026 23:51:20 -0400."
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Thu, 24 Sep 2026 10:41:27 +0200
Message-ID: <10459.1790239287@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/10459.1790239287%40localhost>
Precedence: bulk

Robert Treat <rob@xzilla.net> wrote:

> On Wed, Sep 23, 2026 at 2:28=E2=80=AFPM Masahiko Sawada <sawada.mshk@gmai=
l.com> wrote:
> >
> > On Wed, Sep 23, 2026 at 9:23=E2=80=AFAM Antonin Houska <ah@cybertec.at>=
 wrote:
> > >
> > > shihao zhong <zhong950419@gmail.com> wrote:
> > >
> > > > > or whether the relfilenode should be re-checked after the snapsho=
t is built
> > > >
> > > > Holding the toast lock from the start deadlocks. A session that ask=
s for
> > > > AccessExclusiveLock gets an XID before it waits, and the decoding w=
orker
> > > > waits for all XIDs while it sets up.
> > >
> > > The same (supposedly low) deadlock risk already exists for the main t=
able, 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 o=
n our
> > >      * table (e.g.  it runs CREATE INDEX), we can end up in a deadloc=
k.
> > >      * Not sure this risk is worth unlocking/locking the table (and i=
ts
> > >      * 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 sit=
uation
> > > 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.
> >
>=20
> 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.

pg_squeeze gives up as soon as it notices a "disrupting" catalog
change. Although I haven't heard complaints about this behavior (it's proba=
bly
not common to run conflicting DDL commands during maintenance window), I ad=
mit
it's not the ideal approach.

For REPACK (CONCURRENTLY), we decided to not give up voluntarily. Even if
REPACK ends up in a deadlock, it still has some chance to win. The direction
we took here is to adjust the deadlock detector (in future versions) so that
REPACK always wins. Raising ERROR on REPACK's side in case of specific
conflict would be against that strategy.

(What I said does not mean that I'm in favor of restarting the decoding wor=
ker
either. I still prefer locking the TOAST relation early, as I noted elsewhe=
re
in the thread.)

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