On Fri, Mar 4, 2022 at 10:40 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> At Mon, 14 Feb 2022 14:52:15 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
> > In this version , 0001 gets one fix and two comment updates.
>
> While the disucssion on back-patching of 0001 proceeding on another
> branch, the main patch iself gets looks like as if rotten on
> CF-App. So I rebased v10 on the current master. 0001 is replaced by
> an adjusted patch based on the latest "control file update fix" patch.
>
> 0001: Fix possible incorrect controlfile update that leads to
> unrecoverable database.
0001 - I don't think you need to do this as UpdateControlFile
(update_controlfile) will anyway update it, no?
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
> 0002: Add REDO/Checkpiont LSNs to checkpoinkt-end log message.
> (The main patch in this thread)
0002 - If at all the intention is to say that no ControlFileLock is
required while reading ControlFile->checkPoint and
ControlFile->checkPointCopy.redo, let's say it, no? How about
something like "No ControlFileLock is required while reading
ControlFile->checkPoint and ControlFile->checkPointCopy.redo as there
can't be any other process updating them concurrently."?
+ /* we are the only updator of these variables */
+ LSN_FORMAT_ARGS(ControlFile->checkPoint),
+ LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo))));
> 0003: Replace (WAL-)location to LSN in pg_controldata.
>
> 0004: Replace (WAL-)location to LSN in user-facing texts.
> (This doesn't reflect my recent comments.)
If you don't mind, can you please put the comments here?
> 0005: Unhyphenate the word archive-recovery and similars.
0005 - How about replacing "crash-recovery" to "crash recovery" in
postgres-ref.sgml too?
Regards,
Bharath Rupireddy.
