MIME-Version: 1.0
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>
 <ZZa4pLFCe2mAks1m@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAJpy0uBGSjwhZTBo6MxT6TkwO2AX5fa1kw7UC3RQLiapUJReuw@mail.gmail.com>
 <CAFiTN-vTaWUh0Eq8UpETMk=ZGFEMkD-1gXtosODZeCiCfi92kQ@mail.gmail.com>
 <CAA4eK1KLR2BP1GULkA9FMj3aKFbOcQLvu6b5fAht+YjCVRkc1w@mail.gmail.com>
 <CAFiTN-skn=ffyC8PMzg7gfpyNLfx7v9roae-+W8HYbpzoO0mkg@mail.gmail.com>
 <OS0PR01MB57169DD55EC8D9D1EDB7A0C2946A2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <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>
In-Reply-To: <CAD21AoA03f0QKYjiSymGcnROvh1X-3YSqyB-19mkRXt5dfaxFw@mail.gmail.com>
From: shveta malik <shveta.malik@gmail.com>
Date: Tue, 16 Jan 2024 15:10:31 +0530
Message-ID: <CAJpy0uCneiKeNuz3B91Wf2LRxAenD3fpaxYfdL09y9qHmvVp+w@mail.gmail.com>
Subject: Re: Synchronizing slots from primary to standby
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, 
	"Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Dilip Kumar <dilipbalaut@gmail.com>, 
	Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Peter Smith <smithpb2250@gmail.com>, 
	Nisha Moond <nisha.moond412@gmail.com>, 
	"Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, 
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, 
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Bruce Momjian <bruce@momjian.us>, 
	Ashutosh Sharma <ashu.coek88@gmail.com>, Andres Freund <andres@anarazel.de>, 
	pgsql-hackers <pgsql-hackers@postgresql.org>, Ajin Cherian <itsajin@gmail.com>, 
	Alvaro Herrera <alvherre@alvh.no-ip.org>, shveta malik <shveta.malik@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAJpy0uCneiKeNuz3B91Wf2LRxAenD3fpaxYfdL09y9qHmvVp%2Bw%40mail.gmail.com>
Precedence: bulk

On Tue, Jan 16, 2024 at 12:59=E2=80=AFPM Masahiko Sawada <sawada.mshk@gmail=
.com> wrote:
>
> On Tue, Jan 16, 2024 at 1:07=E2=80=AFPM Amit Kapila <amit.kapila16@gmail.=
com> wrote:
> >
> > On Tue, Jan 16, 2024 at 9:03=E2=80=AFAM shveta malik <shveta.malik@gmai=
l.com> wrote:
> > >
> > > On Sat, Jan 13, 2024 at 12:54=E2=80=AFPM Amit Kapila <amit.kapila16@g=
mail.com> wrote:
> > > >
> > > > On Fri, Jan 12, 2024 at 5:50=E2=80=AFPM shveta malik <shveta.malik@=
gmail.com> wrote:
> > > > >
> > > > > There are multiple approaches discussed and tried when it comes t=
o
> > > > > starting a slot-sync worker. I am summarizing all here:
> > > > >
> > > > >  1) Make slotsync worker as an Auxiliary Process (like checkpoint=
er,
> > > > > walwriter, walreceiver etc). The benefit this approach provides i=
s, it
> > > > > can control begin and stop in a more flexible way as each auxilia=
ry
> > > > > process could have different checks before starting and can have
> > > > > different stop conditions. But it needs code duplication for proc=
ess
> > > > > management(start, stop, crash handling, signals etc) and currentl=
y it
> > > > > does not support db-connection smoothly (none of the auxiliary pr=
ocess
> > > > > has one so far)
> > > > >
> > > >
> > > > As slotsync worker needs to perform transactions and access syscach=
e,
> > > > we can't make it an auxiliary process as that doesn't initialize th=
e
> > > > required stuff like syscache. Also, see the comment "Auxiliary
> > > > processes don't run transactions ..." in AuxiliaryProcessMain() whi=
ch
> > > > means this is not an option.
> > > >
> > > > >
> > > > > 2) Make slotsync worker as a 'special' process like AutoVacLaunch=
er
> > > > > which is neither an Auxiliary process nor a bgworker one. It allo=
ws
> > > > > db-connection and also provides flexibility to have start and sto=
p
> > > > > conditions for a process.
> > > > >
> > > >
> > > > Yeah, due to these reasons, I think this option is worth considerin=
g
> > > > and another plus point is that this allows us to make enable_syncsl=
ot
> > > > a PGC_SIGHUP GUC rather than a PGC_POSTMASTER.
> > > >
> > > > >
> > > > > 3) Make slotysnc worker a bgworker. Here we just need to register=
 our
> > > > > process as a bgworker (RegisterBackgroundWorker()) by providing a
> > > > > relevant start_time and restart_time and then the process managem=
ent
> > > > > is well taken care of. It does not need any code-duplication and
> > > > > allows db-connection smoothly in registered process. The only thi=
ng it
> > > > > lacks is that it does not provide flexibility of having
> > > > > start-condition which then makes us to have 'enable_syncslot' as
> > > > > PGC_POSTMASTER parameter rather than PGC_SIGHUP. Having said this=
, I
> > > > > feel enable_syncslot is something which will not be changed frequ=
ently
> > > > > and with the benefits provided by bgworker infra, it seems a
> > > > > reasonably good option to choose this approach.
> > > > >
> > > >
> > > > I agree but it may be better to make it a PGC_SIGHUP parameter.
> > > >
> > > > > 4) Another option is to have Logical Replication Launcher(or a ne=
w
> > > > > process) to launch slot-sync worker. But going by the current des=
ign
> > > > > where we have only 1 slotsync worker, it may be an overhead to ha=
ve an
> > > > > additional manager process maintained.
> > > > >
> > > >
> > > > I don't see any good reason to have an additional launcher process =
here.
> > > >
> > > > >
> > > > > Thus weighing pros and cons of all these options, we have current=
ly
> > > > > implemented the bgworker approach (approach 3).  Any feedback is
> > > > > welcome.
> > > > >
> > > >
> > > > I vote to go for (2) unless we face difficulties in doing so but (3=
)
> > > > is also okay especially if others also think so.
> > >
> > > I am not against any of the approaches but I still feel that when we
> > > have a standard way of doing things (bgworker) we should not keep
> > > adding code to do things in a special way unless there is a strong
> > > reason to do so. Now we need to decide if 'enable_syncslot' being
> > > PGC_POSTMASTER is a strong reason to go the non-standard way?
> > >
> >
> > Agreed and as said earlier I think it is better to make it a
> > PGC_SIGHUP. Also, not sure we can say it is a non-standard way as
> > already autovacuum launcher is handled in the same way. One more minor
> > thing is it will save us for having a new bgworker state
> > BgWorkerStart_ConsistentState_HotStandby as introduced by this patch.
>
> Why do we need to add a new BgWorkerStart_ConsistentState_HotStandby
> for the slotsync worker? Isn't it sufficient that the slotsync worker
> exits if not in hot standby mode?

It is doable, but that will mean starting slot-sync worker even on
primary on every server restart which does not seem like a good idea.
We wanted to have a way where-in it does not start itself in
non-standby mode.

> Is there any technical difficulty or obstacle to make the slotsync
> worker start using bgworker after reloading the config file?

When we register slotsync worker as bgworker, we can only register the
bgworker before initializing shared memory, we cannot register
dynamically in the cycle of ServerLoop and thus we do not have
flexibility of registering/deregistering the bgworker  (or controlling
the bgworker start) based on config parameters each time they change.
We can always start slot-sync worker and let it check if
enable_syncslot is ON. If not, exit and retry the next time when
postmaster will restart it after restart_time(60sec). The downside of
this approach is, even if any user does not want slot-sync
functionality and thus has permanently disabled 'enable_syncslot', it
will keep on restarting and exiting there.


thanks
Shveta