public inbox for [email protected]  
help / color / mirror / Atom feed
From: Masahiko Sawada <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: shveta malik <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Subject: Re: Synchronizing slots from primary to standby
Date: Tue, 23 Jan 2024 00:12:04 +0900
Message-ID: <CAD21AoDV7_eErvd2ZgBG4u9BNFm0PPKoqetk7ya5uqA+0hpOcQ@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1L92BC3uaFeZJWC5fUJXZLSkFbnoa_ijmXoXWjP9FB6RA@mail.gmail.com>
References: <CAA4eK1L74EUU7+Dr11joY+Xact_MsjZu9G12sX+vRUWJsptP8Q@mail.gmail.com>
	<CAA4eK1+ORhnb2zUUpO_FcgWUUbwvQX5+=tdCehx7x2nB2x8VMg@mail.gmail.com>
	<CAD21AoCoX+jhy_i3v+T2s78NG_0HH1oXOUiTOWhDdxVPBtDHKA@mail.gmail.com>
	<CAJpy0uAwz9iNw3=5QC8S3z8Bf-w0z27rE32SFxjDMer7aTQMPg@mail.gmail.com>
	<CAA4eK1+q8R=yEpTMiOZ_Xksdd_Jg3SXvBMxRutHQtkCbdeK_yQ@mail.gmail.com>
	<CAJpy0uAU+nzufQwSzyB2YmhPHHn71_=1zkauP4Ra32a_eqbcig@mail.gmail.com>
	<CAA4eK1KtwFg6Mr1MRnHUUfPotzc2=5UCTq_D2t8x0PM1Lyhk8g@mail.gmail.com>
	<CAD21AoA03f0QKYjiSymGcnROvh1X-3YSqyB-19mkRXt5dfaxFw@mail.gmail.com>
	<CAJpy0uCneiKeNuz3B91Wf2LRxAenD3fpaxYfdL09y9qHmvVp+w@mail.gmail.com>
	<CAJpy0uCJ7MKgZnKLAACd-AXN0VbWM5gVJ+GRJ1Za_A2UmF3R0A@mail.gmail.com>
	<[email protected]>
	<CAJpy0uBc6H22RbNVU213AZ_BPw+uDptcTVHakKegpTCv74yN=w@mail.gmail.com>
	<CAA4eK1JZDtRFPJaeaLvj74760pcvaEAVxNPN0+oW5A4jL1WCBg@mail.gmail.com>
	<CAFiTN-vNyO879VfvgcxD=bUYNjCRKQR8DGGHD8zHycv8uD1QeQ@mail.gmail.com>
	<CAA4eK1KwFbg10cPt4gBxKdvemMKjaehnoc5e+J6FXXS4=vxQNw@mail.gmail.com>
	<CAD21AoCh4WYySQKe6Zx4tRDi-8DTrA6Qv3KU7iLuoOaRPBJPuw@mail.gmail.com>
	<CAA4eK1L92BC3uaFeZJWC5fUJXZLSkFbnoa_ijmXoXWjP9FB6RA@mail.gmail.com>

On Mon, Jan 22, 2024 at 9:26 PM Amit Kapila <[email protected]> wrote:
>
> On Mon, Jan 22, 2024 at 5:28 PM Masahiko Sawada <[email protected]> wrote:
> >
> > On Sat, Jan 20, 2024 at 7:44 PM Amit Kapila <[email protected]> wrote:
> > >
> > > On Sat, Jan 20, 2024 at 10:52 AM Dilip Kumar <[email protected]> wrote:
> > > >
> > > > On Fri, Jan 19, 2024 at 5:24 PM Amit Kapila <[email protected]> wrote:
> > > > >
> > > > > On Wed, Jan 17, 2024 at 4:00 PM shveta malik <[email protected]> wrote:
> > > > > >
> > > > >
> > > > > I had some off-list discussions with Sawada-San, Hou-San, and Shveta
> > > > > on the topic of extending replication commands instead of using the
> > > > > current model where we fetch the required slot information via SQL
> > > > > using a database connection. I would like to summarize the discussion
> > > > > and would like to know the thoughts of others on this topic.
> > > > >
> > > > > In the current patch, we launch the slotsync worker on physical
> > > > > standby which connects to the specified database (currently we let
> > > > > users specify the required dbname in primary_conninfo) on the primary.
> > > > > It then fetches the required information for failover marked slots
> > > > > from the primary and also does some primitive checks on the upstream
> > > > > node via SQL (the additional checks are like whether the upstream node
> > > > > has a specified physical slot or whether the upstream node is a
> > > > > primary node or a standby node). To fetch the required information it
> > > > > uses a libpqwalreciever API which is mostly apt for this purpose as it
> > > > > supports SQL execution but for this patch, we don't need a replication
> > > > > connection, so we extend the libpqwalreciever connect API.
> > > >
> > > > What sort of extension we have done to 'libpqwalreciever'? Is it
> > > > something like by default this supports replication connections so we
> > > > have done an extension to the API so that we can provide an option
> > > > whether to create a replication connection or a normal connection?
> > > >
> > >
> > > Yeah and in the future there could be more as well. The other function
> > > added walrcv_get_dbname_from_conninfo doesn't appear to be a problem
> > > either for now.
> > >
> > > > > Now, the concerns related to this could be that users would probably
> > > > > need to change existing mechanisms/tools to update priamry_conninfo
> >
> > I'm concerned about this. In fact, a primary_conninfo value generated
> > by pg_basebackup does not work with enable_syncslot.
> >
>
> Right, but if we want can't we extend pg_basebackup to do that? It is
> just that I am not sure that it is a good idea to extend pg_basebackup
> in the first version.

Okay.

>
> > > > > and one of the alternatives proposed is to have an additional GUC like
> > > > > slot_sync_dbname. Users won't be able to drop the database this worker
> > > > > is connected to aka whatever is specified in slot_sync_dbname but as
> > > > > the user herself sets up the configuration it shouldn't be a big deal.
> > > >
> > > > Yeah for this purpose users may use template1 or so which they
> > > > generally don't plan to drop.
> > > >
> > >
> > > Using template1 has other problems like users won't be able to create
> > > a new database. See [2] (point number 2.2)
> > >
> > > >
> > > >  So in case the user wants to drop that
> > > > database user needs to turn off the slot syncing option and then it
> > > > can be done?
> > > >
> > >
> > > Right.
> >
> > If the user wants to continue using slot syncing, they need to switch
> > the database to connect. Which requires modifying primary_conninfo and
> > reloading the configuration file. Which further leads to restarting
> > the physical replication. If they use synchronous replication, it
> > means the application temporarily stops during that.
> >
>
> Yes, that would be an inconvenience but the point is we don't expect
> this to change often.
>
> > >
> > > > > Then we also discussed whether extending libpqwalreceiver's connect
> > > > > API is a good idea and whether we need to further extend it in the
> > > > > future. As far as I can see, slotsync worker's primary requirement is
> > > > > to execute SQL queries which the current API is sufficient, and don't
> > > > > see something that needs any drastic change in this API. Note that
> > > > > tablesync worker that executes SQL also uses these APIs, so we may
> > > > > need something in the future for either of those. Then finally we need
> > > > > a slotsync worker to also connect to a database to use SQL and fetch
> > > > > results.
> > > >
> > > > While looking into the patch v64-0002 I could not exactly point out
> > > > what sort of extensions are there in libpqwalreceiver.c, I just saw
> > > > one extra API for fetching the dbname from connection info?
> > > >
> > >
> > > Right, the worry was that we may need it in the future.
> >
> > Yes. IIUC the slotsync worker uses libpqwalreceiver to establish a
> > non-replication connection and to execute SQL query. But neither of
> > them are relevant with replication.
> >
>
> But we are already using libpqwalreceiver to execute SQL queries via
> tablesync worker.

IIUC tablesync workers do both SQL queries and replication commands. I
think the slotsync worker is the first background process who does
only SQL queries in a non-replication command ( using
libpqwalreceiver).

>
>  I'm a bit concerned that when we
> > need to extend the slotsync feature in the future we will end up
> > extending libpqwalreceiver, even if the new feature is not also
> > relevant with replication.
> >
> > >
> > > > > Now, let us consider if we extend the replication commands like
> > > > > READ_REPLICATION_SLOT and or introduce a new set of replication
> > > > > commands to fetch the required information then we don't need a DB
> > > > > connection with primary or a connection in slotsync worker. As per my
> > > > > current understanding, it is quite doable but I think we will slowly
> > > > > go in the direction of making replication commands something like SQL
> > > > > because today we need to extend it to fetch all slots info that have
> > > > > failover marked as true, the existence of a particular replication,
> > > > > etc. Then tomorrow, if we want to extend this work to have multiple
> > > > > slotsync workers say workers perdb then we have to extend the
> > > > > replication command to fetch per-database failover marked slots. To
> > > > > me, it sounds more like we are slowly adding SQL-like features to
> > > > > replication commands.
> >
> > Right. How about filtering slots on the standby side? That is, for
> > example, the LIST_SLOT command returns all slots and the slotsync
> > worker filters out non-failover slots.
> >
>
> Yeah, we can do that but it could be unnecessary network overhead when
> there are very few failover slots. And it may not be just one time, we
> need to fetch slot information periodically.

True.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com





view thread (119+ messages)  latest in thread

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], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Synchronizing slots from primary to standby
  In-Reply-To: <CAD21AoDV7_eErvd2ZgBG4u9BNFm0PPKoqetk7ya5uqA+0hpOcQ@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 agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox