public inbox for [email protected]  
help / color / mirror / Atom feed
From: Zhijie Hou (Fujitsu) <[email protected]>
To: Amit Kapila <[email protected]>
To: shveta malik <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Dilip Kumar <[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: Alvaro Herrera <[email protected]>
Subject: RE: Synchronizing slots from primary to standby
Date: Mon, 29 Jan 2024 13:47:35 +0000
Message-ID: <OS0PR01MB571642B938DAB06F77131ACA947E2@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <OS0PR01MB5716303C750A3D2FB71B77CD947E2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
References: <[email protected]>
	<CAJpy0uBc6H22RbNVU213AZ_BPw+uDptcTVHakKegpTCv74yN=w@mail.gmail.com>
	<CAA4eK1JZDtRFPJaeaLvj74760pcvaEAVxNPN0+oW5A4jL1WCBg@mail.gmail.com>
	<[email protected]>
	<CAA4eK1JPB-zpGYTbVOP5Qp26tNQPMjDuYzNZ+a9RFiN5nE1tEA@mail.gmail.com>
	<CAJpy0uAmbh_1pH-eG5bTbHXXq9w-uNV45DT-12TTh9kGj2HeCw@mail.gmail.com>
	<CAFPTHDbsZ+pxAubb9d9BwVNt5OB3_2s77bG6nHcAgUPPhEVmMQ@mail.gmail.com>
	<CAJpy0uBqS_HD30mBR0yD1SVCvUZDrw1=dwn1xH8ANEQw7gnPdw@mail.gmail.com>
	<CAA4eK1Lxvfq9RwOEsguiMCrKPUc1He9UGz1_wi0N0cJaXFa4Eg@mail.gmail.com>
	<OS0PR01MB5716E304C26E6ACE0BE84925947A2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<[email protected]>
	<CAA4eK1K0jRXa2AhKXRVDY+Y42MG4RTbfAEcTBEGN2ov=Yjco+A@mail.gmail.com>
	<OS0PR01MB571623B1D01003F99A7BB983947A2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAA4eK1+3jV839U3+DGgHrgrhtDguaVwAyb0Hucz9UbpjjVL=Sg@mail.gmail.com>
	<OS0PR01MB5716DE2A364CB23F9144AED194782@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAHut+PsaPJzbTAFjXkRE0uJYk+xNAdOHEnoBFTvazsRZfYSU3Q@mail.gmail.com>
	<CAA4eK1+LnPajBsCXwCKbmOtDSJ=WWnXR+sHnTWav5OL3rebyyw@mail.gmail.com>
	<CAJpy0uAv2dS+NTw1X1uZep11WM24Wog2kN1oqJu4Zx4xjpgwig@mail.gmail.com>
	<CAA4eK1L6yoEs0OveL9xB+jopyp=Eps4ViU3JXhfpz0-2H3XCrg@mail.gmail.com>
	<OS0PR01MB5716303C750A3D2FB71B77CD947E2@OS0PR01MB5716.jpnprd01.prod.outlook.com>

On Monday, January 29, 2024 9:17 PM Zhijie Hou (Fujitsu) <[email protected]> wrote:
> 
> On Monday, January 29, 2024 7:30 PM Amit Kapila <[email protected]>
> wrote:
> >
> > On Mon, Jan 29, 2024 at 3:11 PM shveta malik <[email protected]>
> > wrote:
> > >
> > > PFA v71 patch set with above changes.
> > >
> >
> > Few comments on 0001
> 
> Thanks for the comments.
> 
> > ===================
> > 1.
> > parse_subscription_options()
> > {
> > ...
> > /*
> > * We've been explicitly asked to not connect, that requires some
> > * additional processing.
> > */
> > if (!opts->connect && IsSet(supported_opts, SUBOPT_CONNECT)) {
> >
> > Here, along with other options, we need an explicit check for
> > failover, so that if connect=false and failover=true, the statement
> > should give error. I was expecting the below statement to fail but it passed
> with WARNING.
> > postgres=# create subscription sub2 connection 'dbname=postgres'
> > publication pub2 with(connect=false, failover=true);
> > WARNING:  subscription was created, but is not connected
> > HINT:  To initiate replication, you must manually create the
> > replication slot, enable the subscription, and refresh the subscription.
> > CREATE SUBSCRIPTION
> 
> Added.
> 
> >
> > 2.
> > @@ -148,6 +153,10 @@ typedef struct Subscription
> >   List    *publications; /* List of publication names to subscribe to */
> >   char    *origin; /* Only publish data originating from the
> >   * specified origin */
> > + bool failover; /* True if the associated replication slots
> > + * (i.e. the main slot and the table sync
> > + * slots) in the upstream database are enabled
> > + * to be synchronized to the standbys. */
> >  } Subscription;
> >
> > Let's add this new field immediately after "bool runasowner;" as is
> > done for other boolean members. This will help avoid increasing the
> > size of the structure due to alignment when we add any new pointer
> > field in the future. Also, that would be consistent with what we do for other
> new boolean members.
> 
> Moved this field as suggested.
> 
> Attach the V72-0001 which addressed above comments, other patches will be
> rebased and posted after pushing first patch. Thanks Shveta for helping
> address the comments.

Apart from above comments. The new V72 patch also includes the followings changes.

1. Moved the test 'altering failover for enabled sub' to the tap-test where most
of the alter-sub behaviors are tested.

2. Rename the tap-test from 050_standby_failover_slots_sync.pl to
040_standby_failover_slots_sync.pl (the big number 050 was used to avoid
conflict with other newly committed tests). And add the test into meson.build
which was missed.

Best Regards,
Hou zj


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: <OS0PR01MB571642B938DAB06F77131ACA947E2@OS0PR01MB5716.jpnprd01.prod.outlook.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