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. Regards, Amul
From e72137d632afd4c916da20d56491f782d62b605e Mon Sep 17 00:00:00 2001 From: Amul Sul <amul.sul@enterprisedb.com> Date: Wed, 3 Feb 2021 23:03:20 -0500 Subject: [PATCH] Correct code comment in StartupXLOG --- src/backend/access/transam/xlog.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f03bd473e2b..eee664597ea 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7998,17 +7998,16 @@ StartupXLOG(void) * All done with end-of-recovery actions. * * Now allow backends to write WAL and update the control file status in - * consequence. The boolean flag allowing backends to write WAL is - * updated while holding ControlFileLock to prevent other backends to look + * 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. There * is still a small window during which backends can write WAL and the * control file is still referring to a system not in DB_IN_PRODUCTION * state while looking at the on-disk control file. * - * Also, although the boolean flag to allow WAL is probably atomic in - * itself, we use the info_lck here to ensure that there are no race - * conditions concerning visibility of other recent updates to shared - * memory. + * Also, we use the info_lck to update the recovery state to ensure that + * there are no race conditions concerning visibility of other recent + * updates to shared memory. */ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE); ControlFile->state = DB_IN_PRODUCTION; -- 2.18.0
