On Thu, Feb 4, 2021 at 9:39 AM Amul Sul <sula...@gmail.com> wrote: > > On Thu, Feb 4, 2021 at 6:18 AM Kyotaro Horiguchi > <horikyota....@gmail.com> wrote: > > > > At Wed, 3 Feb 2021 16:36:13 +0530, Amul Sul <sula...@gmail.com> wrote in > > > On Wed, Feb 3, 2021 at 2:48 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > > > > > On Wed, Feb 3, 2021 at 2:28 PM Amul Sul <sula...@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