public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alvaro Herrera <[email protected]>
To: Antonin Houska <[email protected]>
Cc: Srinath Reddy Sadipiralla <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Mihail Nikalayeu <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Robert Treat <[email protected]>
Subject: Re: Adding REPACK [concurrently]
Date: Sun, 5 Apr 2026 01:53:09 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <95398.1775316559@localhost>
On 2026-Apr-04, Antonin Houska wrote:
> Alvaro Herrera <[email protected]> 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.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"No me acuerdo, pero no es cierto. No es cierto, y si fuera cierto,
no me acuerdo." (Augusto Pinochet a una corte de justicia)
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Adding REPACK [concurrently]
In-Reply-To: <[email protected]>
* 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