On Fri, Mar 4, 2022 at 10:40 AM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > At Mon, 14 Feb 2022 14:52:15 +0900 (JST), Kyotaro Horiguchi > <horikyota....@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.