From: Amit Kapila <amit.kapila16@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Nathan Bossart <nathandbossart@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>
Cc: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_upgrade and logical replication
Date: Mon, 22 Jul 2024 15:45:19 +0530
Message-ID: <CAA4eK1+P6Up5c9C5ORBCsAFvad-Rf847LmjsfohpQopVUUb5DQ@mail.gmail.com> (raw)
In-Reply-To: <Zp2-XDir9teSyAW6@paquier.xyz>
References: <TYCPR01MB12077B16EEDA360BA645B96F8F54C2@TYCPR01MB12077.jpnprd01.prod.outlook.com>
<CALDaNm3h7ReG5wB9bhPpU=GpVp9Cfotx36tRErw5oKEXu+cKTA@mail.gmail.com>
<TYCPR01MB12077C4D38046B89599322EBBF54C2@TYCPR01MB12077.jpnprd01.prod.outlook.com>
<CALDaNm1kdAnfx8N1AUQUuxc0zXbghFKi3-4vFAym_Y3hPOm1hA@mail.gmail.com>
<TYCPR01MB120778982E38315F541D61AD1F5512@TYCPR01MB12077.jpnprd01.prod.outlook.com>
<CAA4eK1+B47ooT2HD1OTbSkVedK1BJLbr2K-seTd9pg9-wTgOdQ@mail.gmail.com>
<CALDaNm0z=fLtio1h50K8WossUGXU+gy0H9y9=RYh1DDZiq2EDw@mail.gmail.com>
<CAA4eK1LrO2qGcQot-9EgxpQRiT1O5+enHb57oN7OSNKSTn2hNQ@mail.gmail.com>
<ZprQJv_TxccN3tkr@nathan>
<ZpxsWx5jwlMCBZfn@nathan>
<Zp2-XDir9teSyAW6@paquier.xyz>
On Mon, Jul 22, 2024 at 7:35 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Sat, Jul 20, 2024 at 09:03:07PM -0500, Nathan Bossart wrote:
> >> This is an extremely expensive way to perform that check, and so I'm
> >> wondering why we don't just do
> >>
> >> SELECT count(*) FROM pg_catalog.pg_subscription;
> >>
> >> once in count_old_cluster_subscriptions().
> >
> > Like so...
Isn't it better to directly invoke get_subscription_count() in
check_new_cluster_subscription_configuration() where it is required
rather than in a db-specific general function?
>
> Ah, good catch. That sounds like a good thing to do because we don't
> care about the number of subscriptions for each database in the
> current code.
>
> This is something that qualifies as an open item, IMO, as this code
> is new to PG17.
>
> A comment in get_db_rel_and_slot_infos() becomes incorrect where
> get_old_cluster_logical_slot_infos() is called; it is still referring
> to the subscription count.
>
> Actually, on the same grounds, couldn't we do the logical slot info
> retrieval in get_old_cluster_logical_slot_infos() in a single pass as
> well? pg_replication_slots reports some information about all the
> slots, and the current code has a qual on current_database(). It
> looks to me that this could be replaced by a single query, ordering
> the slots by database names, assigning the slot infos in each
> database's DbInfo at the end.
>
Unlike subscriptions, logical slots are database-specific objects. We
have some checks in the code like the one in CreateDecodingContext()
for MyDatabaseId which may or may not create a problem for this case
as we don't consume changes when checking
LogicalReplicationSlotHasPendingWal via
binary_upgrade_logical_slot_has_caught_up() but I think this needs
more analysis than what Nathan has proposed. So, I suggest taking up
this task for PG18 if we want to optimize this code path.
--
With Regards,
Amit Kapila.
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: amit.kapila16@gmail.com, michael@paquier.xyz, nathandbossart@gmail.com, vignesh21@gmail.com, kuroda.hayato@fujitsu.com, pryzby@telsasoft.com, sawada.mshk@gmail.com, smithpb2250@gmail.com, rjuju123@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_upgrade and logical replication
In-Reply-To: <CAA4eK1+P6Up5c9C5ORBCsAFvad-Rf847LmjsfohpQopVUUb5DQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox