agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Antonin Houska <ah@cybertec.at>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Robert Treat <rob@xzilla.net>
Subject: Re: Adding REPACK [concurrently]
Date: Sun, 05 Apr 2026 09:54:49 +0200
Message-ID: <19989.1775375689@localhost> (raw)
In-Reply-To: <202604042326.nwa4o5sezj77@alvherre.pgsql>
References: <202604042326.nwa4o5sezj77@alvherre.pgsql>
Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2026-Apr-04, Antonin Houska wrote:
>
> > Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> >
> > > On 2026-Apr-04, Antonin Houska wrote:
> > >
> > > > I agree that the global variable is not handy, but instead of modifying
> > > > CreateInitDecodingContext(), how about adding a boolean returning callback to
> > > > OutputPluginCallbacks? The point is that whether shared catalogs are needed
> > > > during the decoding or not is actually property of the plugin.
> > >
> > > Oh, yeah, that sounds good to me.
> >
> > This is it. New callback was actually not needed, I just added a new flag to
> > the OutputPluginOptions structure.
>
> Thank you, I removed the previous one and picked up this one (it's 0001
> here.) The only potentially troublesome thing I see with it is this change:
>
> /*
> * Update range of interesting xids based on the running xacts
> * information. We don't increase ->xmax using it, because once we are in
> * a consistent state we can do that ourselves and much more efficiently
> * so, because we only need to do it for catalog transactions since we
> * only ever look at those.
> *
> * NB: We only increase xmax when a catalog modifying transaction commits
> * (see SnapBuildCommitTxn). Because of this, xmax can be lower than
> * xmin, which looks odd but is correct and actually more efficient, since
> * we hit fast paths in heapam_visibility.c.
> + *
> + * If database specific transaction info was used during startup, the info
> + * for the whole cluster can make xmin go backwards. That would be bad
> + * because we might no longer have older XIDs in ->committed.
> */
> - builder->xmin = running->oldestRunningXid;
> + if (NormalTransactionIdFollows(running->oldestRunningXid, builder->xmin))
> + builder->xmin = running->oldestRunningXid;
>
>
> I can't see any problem with advancing the ->xmin only when it goes
> forward, but I wonder if it's possible to introduce any bugs this way.
>
>
> This bit looks funny though:
>
> /*
> * Advance the xmin limit for the current replication slot, to allow
> * vacuum to clean up the tuples this slot has been protecting.
> *
> * The reorderbuffer might have an xmin among the currently running
> * snapshots; use it if so. If not, we need only consider the snapshots
> * we'll produce later, which can't be less than the oldest running xid in
> * the record we're reading now.
> */
> xmin = ReorderBufferGetOldestXmin(builder->reorder);
> - if (xmin == InvalidTransactionId)
> + /*
> + * Like above, do not let slot xmin go backwards.
> + */
> + if (xmin == InvalidTransactionId && !db_specific)
> xmin = running->oldestRunningXid;
>
> I probably need some sleep, but this doesn't make sense to me.
ok, maybe just skip the whole cleanup in that special case.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
[text/x-diff] nocfbot.v53-0001.2-Introduce-an-option-to-make-logical-replication-.patch (0B, ../19989.1775375689@localhost/2-nocfbot.v53-0001.2-Introduce-an-option-to-make-logical-replication-.patch)
download
view thread (416+ messages) latest in thread
Message-ID: <19989.1775375689@localhost>
Permalink: ../19989.1775375689@localhost/
Also on: postgresql.org/message-id/19989.1775375689@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, alvherre@alvh.no-ip.org, srinath2133@gmail.com, amit.kapila16@gmail.com, mihailnikalayeu@gmail.com, boekewurm+postgres@gmail.com, pgsql-hackers@lists.postgresql.org, rob@xzilla.net
Subject: Re: Adding REPACK [concurrently]
In-Reply-To: <19989.1775375689@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