On Thu, Feb 4, 2021 at 9:39 AM Amul Sul <sulamul@gmail.com> wrote:
>
> On Thu, Feb 4, 2021 at 6:18 AM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> >
> > At Wed, 3 Feb 2021 16:36:13 +0530, Amul Sul <sulamul@gmail.com> wrote in
> > > On Wed, Feb 3, 2021 at 2:48 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >
> > > > On Wed, Feb 3, 2021 at 2:28 PM Amul Sul <sulamul@gmail.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > SharedRecoveryState member of XLogCtl is no longer a boolean flag, got changes
> > > > > in 4e87c4836ab9 to enum but, comment referring to it still referred as the
> > > > > boolean flag which is pretty confusing and incorrect.
> > > >
> > > > +1 for the comment change
> >
> > Actually the "flag" has been changed to an integer (emnum), so it
> > needs a change. However, the current proposal:
> >
> > * Now allow backends to write WAL and update the control file status in
> > - * consequence. The boolean flag allowing backends to write WAL is
> > + * consequence. The recovery state allowing backends to write WAL is
> > * updated while holding ControlFileLock to prevent other backends to look
> >
> > Looks somewhat strange. The old booean had a single task to allow
> > backends to write WAL but the current state has multple states that
> > controls recovery progress. So I thnink it needs a further change.
> >
> > ===
> > Now allow backends to write WAL and update the control file status in
> > consequence. The recovery state is updated to allow backends to write
> > WAL, while holding ControlFileLock to prevent other backends to look
> > at an inconsistent state of the control file in shared memory.
> > ===
> >
>
> This looks more accurate, added the same in the attached version. Thanks for the
> correction.
Looks good to me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com