public inbox for [email protected]
help / color / mirror / Atom feedFrom: shveta malik <[email protected]>
To: Bertrand Drouvot <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Masahiko Sawada <[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]>
Cc: shveta malik <[email protected]>
Subject: Re: Synchronizing slots from primary to standby
Date: Fri, 5 Jan 2024 08:59:31 +0530
Message-ID: <CAJpy0uBGSjwhZTBo6MxT6TkwO2AX5fa1kw7UC3RQLiapUJReuw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAA4eK1+AFY4E0f9h0-JF-navrf-vRMTMrN6H7PDKiR=C6p84Nw@mail.gmail.com>
<CAJpy0uCcFKiofcp5V_hRvHrY_qVNX+eKvB0CPDNxSk2FbyMJCA@mail.gmail.com>
<OS0PR01MB5716E731AC1B97A96B014B929460A@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAJpy0uDhR=m2Pr76aJZnvtKZpCEiLbMbjbPReCv5fS0MA3ch2w@mail.gmail.com>
<CAJpy0uDWgSBUsTav1Eeo7E7i38GjXr_o0WdWamm3jGdctcoC4A@mail.gmail.com>
<[email protected]>
On Thu, Jan 4, 2024 at 7:24 PM Bertrand Drouvot
<[email protected]> wrote:
>
> Hi,
>
> On Thu, Jan 04, 2024 at 10:27:31AM +0530, shveta malik wrote:
> > On Thu, Jan 4, 2024 at 9:18 AM shveta malik <[email protected]> wrote:
> > >
> > > On Wed, Jan 3, 2024 at 6:33 PM Zhijie Hou (Fujitsu)
> > > <[email protected]> wrote:
> > > >
> > > > On Tuesday, January 2, 2024 6:32 PM shveta malik <[email protected]> wrote:
> > > > > On Fri, Dec 29, 2023 at 10:25 AM Amit Kapila <[email protected]>
> > > > >
> > > > > The topup patch has also changed app_name to
> > > > > {cluster_name}_slotsyncworker so that we do not confuse between walreceiver
> > > > > and slotsyncworker entry.
> > > > >
> > > > > Please note that there is no change in rest of the patches, changes are in
> > > > > additional 0004 patch alone.
> > > >
> > > > Attach the V56 patch set which supports ALTER SUBSCRIPTION SET (failover).
> > > > This is useful when user want to refresh the publication tables, they can now alter the
> > > > failover option to false and then execute the refresh command.
> > > >
> > > > Best Regards,
> > > > Hou zj
> > >
> > > The patches no longer apply to HEAD due to a recent commit 007693f. I
> > > am working on rebasing and will post the new patches soon
> > >
> > > thanks
> > > Shveta
> >
> > Commit 007693f has changed 'conflicting' to 'conflict_reason', so
> > adjusted the code around that in the slotsync worker.
> >
> > Also removed function 'pg_get_slot_invalidation_cause' as now
> > conflict_reason tells the same.
> >
> > PFA rebased patches with above changes.
> >
>
> Thanks!
>
> Looking at 0004:
>
> 1 ====
>
> -libpqrcv_connect(const char *conninfo, bool logical, bool must_use_password,
> - const char *appname, char **err)
> +libpqrcv_connect(const char *conninfo, bool replication, bool logical,
> + bool must_use_password, const char *appname, char **err)
>
> What about adjusting the preceding comment a bit to describe what the new replication
> parameter is for?
>
> 2 ====
>
> + /* We can not have logical w/o replication */
>
> what about replacing w/o by without?
>
> 3 ===
>
> + if(!replication)
> + Assert(!logical);
> +
> + if (replication)
> {
>
> what about using "if () else" instead (to avoid unnecessary test)?
>
>
> Having said that the patch seems a reasonable way to implement non-replication
> connection in slotsync worker.
>
> 4 ===
>
> Looking closer, the only place where walrcv_connect() is called with replication
> set to false and logical set to false is in ReplSlotSyncWorkerMain().
>
> That does make sense, but what do you think about creating dedicated libpqslotsyncwrkr_connect
> and slotsyncwrkr_connect (instead of using the libpqrcv_connect / walrcv_connect ones)?
>
> That way we could make use of slotsyncwrkr_connect() in ReplSlotSyncWorkerMain()
> as I think it's confusing to use "rcv" functions while the process using them is
> not of backend type walreceiver.
>
> I'm not sure that worth the extra complexity though, what do you think?
I gave it a thought earlier, but then I was not sure even if I create
a new function w/o "rcv" in it then where should it be placed as the
existing file name itself is libpq'walreceiver'.c. Shall we be
creating a new file then? But it does not seem good to create a new
setup (new file, function pointers other stuff) around 1 function.
And thus reusing the same function with 'replication' (new arg) felt
like a better choice than other options. If in future, there is any
other module trying to do the same, then it can use current
walrcv_connect() with rep=false. If I make it specific to slot-sync
worker, then it will not be reusable by other modules (if needed).
thanks
Shveta
view thread (27+ 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]
Subject: Re: Synchronizing slots from primary to standby
In-Reply-To: <CAJpy0uBGSjwhZTBo6MxT6TkwO2AX5fa1kw7UC3RQLiapUJReuw@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