agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Antonin Houska <ah@cybertec.at>
To: Robert Treat <rob@xzilla.net>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: shihao zhong <zhong950419@gmail.com>
Cc: Manu <manuelreyesbravo@gmail.com>
Cc: Thom Brown <thom@linux.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten
Date: Thu, 24 Sep 2026 10:41:27 +0200
Message-ID: <10459.1790239287@localhost> (raw)
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>
Robert Treat <rob@xzilla.net> wrote:
> On Wed, Sep 23, 2026 at 2:28 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Wed, Sep 23, 2026 at 9:23 AM Antonin Houska <ah@cybertec.at> wrote:
> > >
> > > shihao zhong <zhong950419@gmail.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.
> >
> > 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.
pg_squeeze gives up as soon as it notices a "disrupting" catalog
change. Although I haven't heard complaints about this behavior (it's probably
not common to run conflicting DDL commands during maintenance window), I admit
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 worker
either. I still prefer locking the TOAST relation early, as I noted elsewhere
in the thread.)
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
view thread (17+ messages)
Message-ID: <10459.1790239287@localhost>
Permalink: ../10459.1790239287@localhost/
Also on: postgresql.org/message-id/10459.1790239287@localhost
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ah@cybertec.at, rob@xzilla.net, sawada.mshk@gmail.com, zhong950419@gmail.com, manuelreyesbravo@gmail.com, 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: <10459.1790239287@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox