Re: Add checkpoint and redo LSN to LogCheckpointEnd log message - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
Date
Msg-id 20220315.172340.1059971522574284501.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: Add checkpoint and redo LSN to LogCheckpointEnd log message  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: Add checkpoint and redo LSN to LogCheckpointEnd log message  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
At Tue, 15 Mar 2022 12:19:47 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in 
> On Fri, Mar 4, 2022 at 10:40 AM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> 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);

Ugh.. Yes. It is a copy-pasto from older versions.  They may have the
same copy-pasto..



> > 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))));

I thought the comment explains that. But it would be better to be more
specific.  It is changed as the follows.

> * ControlFileLock is not required as we are the only
> * updator of these variables.


> > 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?

Okay. It's the following message.

https://www.postgresql.org/message-id/20220209.115204.1794224638476710282.horikyota.ntt@gmail.com

> The old 0003 (attached 0004):
> 
> 
> 
> +++ b/src/backend/access/rmgrdesc/xlogdesc.c
> -        appendStringInfo(buf, "redo %X/%X; "
> +        appendStringInfo(buf, "redo lsn %X/%X; "
> 
> 
> 
> It is shown in the context of a checkpoint record, so I think it is
> not needed or rather lengthning the dump line uselessly. 
> 
> 
> 
> +++ b/src/backend/access/transam/xlog.c
> -                (errmsg("request to flush past end of generated WAL; request %X/%X, current position %X/%X",
> +                (errmsg("request to flush past end of generated WAL; request lsn %X/%X, current lsn %X/%X",
> 
> 
> 
> +++ b/src/backend/replication/walsender.c
> -                    (errmsg("requested starting point %X/%X is ahead of the WAL flush position of this server
%X/%X",
> +                    (errmsg("requested starting point %X/%X is ahead of the WAL flush LSN of this server %X/%X",
> 
> 
> 
> "WAL" is upper-cased. So it seems rather strange that the "lsn" is
> lower-cased.  In the first place the message doesn't look like a
> user-facing error message and I feel we don't need position or lsn
> there..
> 
> 
> 
> +++ b/src/bin/pg_rewind/pg_rewind.c
> -        pg_log_info("servers diverged at WAL location %X/%X on timeline %u",
> +        pg_log_info("servers diverged at WAL LSN %X/%X on timeline %u",
> 
> 
> 
> I feel that we don't need "WAL" there.
> 
> 
> 
> +++ b/src/bin/pg_waldump/pg_waldump.c
> -    printf(_("  -e, --end=RECPTR       stop reading at WAL location RECPTR\n"));
> +    printf(_("  -e, --end=RECPTR       stop reading at WAL LSN RECPTR\n"));
> 
> 
> 
> Mmm.. "WAL LSN RECPTR" looks strange to me.  In the first place I
> don't think "RECPTR" is a user-facing term. Doesn't something like the
> follows work?
> 
> 
> 
> +    printf(_("  -e, --end=WAL-LSN       stop reading at WAL-LSN\n"));
> 
> 
> 
> In some changes in this patch shorten the main message text of
> fprintf-ish functions.  That makes the succeeding parameters can be
> inlined.


> > 0005: Unhyphenate the word archive-recovery and similars.
> 
> 0005 - How about replacing "crash-recovery" to "crash recovery" in
> postgres-ref.sgml too?

Oh, that's a left-over. Fixed. Thanks!

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 37f7433dce1ea9e41aa54f2c18c0bf3d2aa3c814 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Fri, 4 Mar 2022 13:18:30 +0900
Subject: [PATCH v12 1/5] Correctly update contfol file at the end of archive
 recovery

CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function.  If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file.  As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.

Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint.  It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
 src/backend/access/transam/xlog.c | 69 ++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 0d2bd7a357..bd962763cc 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6899,6 +6899,9 @@ CreateRestartPoint(int flags)
     XLogSegNo    _logSegNo;
     TimestampTz xtime;
 
+    /* we don't assume concurrent checkpoint/restartpoint to run */
+    Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER);
+
     /* Get a local copy of the last safe checkpoint record. */
     SpinLockAcquire(&XLogCtl->info_lck);
     lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
@@ -6964,7 +6967,7 @@ CreateRestartPoint(int flags)
 
     /* Also update the info_lck-protected copy */
     SpinLockAcquire(&XLogCtl->info_lck);
-    XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+    XLogCtl->RedoRecPtr = RedoRecPtr;
     SpinLockRelease(&XLogCtl->info_lck);
 
     /*
@@ -6983,7 +6986,10 @@ CreateRestartPoint(int flags)
     /* Update the process title */
     update_checkpoint_display(flags, true, false);
 
-    CheckPointGuts(lastCheckPoint.redo, flags);
+    CheckPointGuts(RedoRecPtr, flags);
+
+    /* Update pg_control */
+    LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
 
     /*
      * Remember the prior checkpoint's redo ptr for
@@ -6991,30 +6997,26 @@ CreateRestartPoint(int flags)
      */
     PriorRedoPtr = ControlFile->checkPointCopy.redo;
 
+    Assert (PriorRedoPtr < RedoRecPtr);
+
+    ControlFile->checkPoint = lastCheckPointRecPtr;
+    ControlFile->checkPointCopy = lastCheckPoint;
+
     /*
-     * Update pg_control, using current time.  Check that it still shows
-     * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
-     * this is a quick hack to make sure nothing really bad happens if somehow
-     * we get here after the end-of-recovery checkpoint.
+     * Ensure minRecoveryPoint is past the checkpoint record while archive
+     * recovery is still ongoing.  Normally, this will have happened already
+     * while writing out dirty buffers, but not necessarily - e.g. because no
+     * buffers were dirtied.  We do this because a non-exclusive base backup
+     * uses minRecoveryPoint to determine which WAL files must be included in
+     * the backup, and the file (or files) containing the checkpoint record
+     * must be included, at a minimum. Note that for an ordinary restart of
+     * recovery there's no value in having the minimum recovery point any
+     * earlier than this anyway, because redo will begin just after the
+     * checkpoint record.  This is a quick hack to make sure nothing really bad
+     * happens if somehow we get here after the end-of-recovery checkpoint.
      */
-    LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
-    if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
-        ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+    if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
     {
-        ControlFile->checkPoint = lastCheckPointRecPtr;
-        ControlFile->checkPointCopy = lastCheckPoint;
-
-        /*
-         * Ensure minRecoveryPoint is past the checkpoint record.  Normally,
-         * this will have happened already while writing out dirty buffers,
-         * but not necessarily - e.g. because no buffers were dirtied.  We do
-         * this because a non-exclusive base backup uses minRecoveryPoint to
-         * determine which WAL files must be included in the backup, and the
-         * file (or files) containing the checkpoint record must be included,
-         * at a minimum. Note that for an ordinary restart of recovery there's
-         * no value in having the minimum recovery point any earlier than this
-         * anyway, because redo will begin just after the checkpoint record.
-         */
         if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
         {
             ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -7026,8 +7028,25 @@ CreateRestartPoint(int flags)
         }
         if (flags & CHECKPOINT_IS_SHUTDOWN)
             ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
-        UpdateControlFile();
     }
+    else
+    {
+        /* recovery mode is not supposed to end during shutdown restartpoint */
+        Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+        /*
+         * Aarchive recovery has ended. Crash recovery ever after should
+         * always recover to the end of WAL
+         */
+        ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+        ControlFile->minRecoveryPointTLI = 0;
+
+        /* also update local copy */
+        LocalMinRecoveryPoint = InvalidXLogRecPtr;
+        LocalMinRecoveryPointTLI = 0;
+    }
+
+    UpdateControlFile();
     LWLockRelease(ControlFileLock);
 
     /*
@@ -7104,7 +7123,7 @@ CreateRestartPoint(int flags)
     xtime = GetLatestXTime();
     ereport((log_checkpoints ? LOG : DEBUG2),
             (errmsg("recovery restart point at %X/%X",
-                    LSN_FORMAT_ARGS(lastCheckPoint.redo)),
+                    LSN_FORMAT_ARGS(RedoRecPtr)),
              xtime ? errdetail("Last completed transaction was at log time %s.",
                                timestamptz_to_str(xtime)) : 0));
 
-- 
2.27.0

From dc1db06e3638f6c200c86bf09fded562cef6701a Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Fri, 4 Mar 2022 13:22:41 +0900
Subject: [PATCH v12 2/5] Add checkpoint and redo LSN to LogCheckpointEnd log
 message

It is useful (for debugging purposes) if the checkpoint end message
has the checkpoint LSN(end) and REDO LSN(start). It gives more
context while analyzing checkpoint-related issues. The pg_controldata
gives the last checkpoint LSN and REDO LSN, but having this info
alongside the log message helps analyze issues that happened
previously, connect the dots and identify the root cause.
---
 src/backend/access/transam/xlog.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index bd962763cc..ce3815ea99 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6120,7 +6120,8 @@ LogCheckpointEnd(bool restartpoint)
                         "%d WAL file(s) added, %d removed, %d recycled; "
                         "write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
                         "sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
-                        "distance=%d kB, estimate=%d kB",
+                        "distance=%d kB, estimate=%d kB; "
+                        "lsn=%X/%X, redo lsn=%X/%X",
                         CheckpointStats.ckpt_bufs_written,
                         (double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
                         CheckpointStats.ckpt_segs_added,
@@ -6133,14 +6134,21 @@ LogCheckpointEnd(bool restartpoint)
                         longest_msecs / 1000, (int) (longest_msecs % 1000),
                         average_msecs / 1000, (int) (average_msecs % 1000),
                         (int) (PrevCheckPointDistance / 1024.0),
-                        (int) (CheckPointDistanceEstimate / 1024.0))));
+                        (int) (CheckPointDistanceEstimate / 1024.0),
+                        /*
+                         * ControlFileLock is not required as we are the only
+                         * updator of these variables.
+                         */
+                        LSN_FORMAT_ARGS(ControlFile->checkPoint),
+                        LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo))));
     else
         ereport(LOG,
                 (errmsg("checkpoint complete: wrote %d buffers (%.1f%%); "
                         "%d WAL file(s) added, %d removed, %d recycled; "
                         "write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
                         "sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
-                        "distance=%d kB, estimate=%d kB",
+                        "distance=%d kB, estimate=%d kB; "
+                        "lsn=%X/%X, redo lsn=%X/%X",
                         CheckpointStats.ckpt_bufs_written,
                         (double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
                         CheckpointStats.ckpt_segs_added,
@@ -6153,7 +6161,13 @@ LogCheckpointEnd(bool restartpoint)
                         longest_msecs / 1000, (int) (longest_msecs % 1000),
                         average_msecs / 1000, (int) (average_msecs % 1000),
                         (int) (PrevCheckPointDistance / 1024.0),
-                        (int) (CheckPointDistanceEstimate / 1024.0))));
+                        (int) (CheckPointDistanceEstimate / 1024.0),
+                        /*
+                         * ControlFileLock is not required as we are the only
+                         * updator of these variables.
+                         */
+                        LSN_FORMAT_ARGS(ControlFile->checkPoint),
+                        LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo))));
 }
 
 /*
-- 
2.27.0

From 1941cdf0d48194044ef8f30d28f532beb51526dc Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 1 Feb 2022 03:57:04 +0000
Subject: [PATCH v12 3/5] Change "location" to "lsn" in pg_controldata

---
 src/bin/pg_controldata/pg_controldata.c    | 10 +++++-----
 src/test/recovery/t/016_min_consistency.pl |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index f911f98d94..59f39267df 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -235,9 +235,9 @@ main(int argc, char *argv[])
            dbState(ControlFile->state));
     printf(_("pg_control last modified:             %s\n"),
            pgctime_str);
-    printf(_("Latest checkpoint location:           %X/%X\n"),
+    printf(_("Latest checkpoint LSN:                %X/%X\n"),
            LSN_FORMAT_ARGS(ControlFile->checkPoint));
-    printf(_("Latest checkpoint's REDO location:    %X/%X\n"),
+    printf(_("Latest checkpoint's REDO LSN:         %X/%X\n"),
            LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo));
     printf(_("Latest checkpoint's REDO WAL file:    %s\n"),
            xlogfilename);
@@ -274,13 +274,13 @@ main(int argc, char *argv[])
            ckpttime_str);
     printf(_("Fake LSN counter for unlogged rels:   %X/%X\n"),
            LSN_FORMAT_ARGS(ControlFile->unloggedLSN));
-    printf(_("Minimum recovery ending location:     %X/%X\n"),
+    printf(_("Minimum recovery ending LSN:          %X/%X\n"),
            LSN_FORMAT_ARGS(ControlFile->minRecoveryPoint));
     printf(_("Min recovery ending loc's timeline:   %u\n"),
            ControlFile->minRecoveryPointTLI);
-    printf(_("Backup start location:                %X/%X\n"),
+    printf(_("Backup start LSN:                     %X/%X\n"),
            LSN_FORMAT_ARGS(ControlFile->backupStartPoint));
-    printf(_("Backup end location:                  %X/%X\n"),
+    printf(_("Backup end LSN:                       %X/%X\n"),
            LSN_FORMAT_ARGS(ControlFile->backupEndPoint));
     printf(_("End-of-backup record required:        %s\n"),
            ControlFile->backupEndRequired ? _("yes") : _("no"));
diff --git a/src/test/recovery/t/016_min_consistency.pl b/src/test/recovery/t/016_min_consistency.pl
index 5e0655c2a9..4ee20309cd 100644
--- a/src/test/recovery/t/016_min_consistency.pl
+++ b/src/test/recovery/t/016_min_consistency.pl
@@ -126,7 +126,7 @@ my @control_data = split("\n", $stdout);
 my $offline_recovery_lsn = undef;
 foreach (@control_data)
 {
-    if ($_ =~ /^Minimum recovery ending location:\s*(.*)$/mg)
+    if ($_ =~ /^Minimum recovery ending LSN:\s*(.*)$/mg)
     {
         $offline_recovery_lsn = $1;
         last;
-- 
2.27.0

From 985fc119636a12cf7293ed058ef8c1c10bc47cb2 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Fri, 4 Mar 2022 13:49:16 +0900
Subject: [PATCH v12 4/5] Change "location" to "lsn" in user facing text

---
 src/backend/access/rmgrdesc/xlogdesc.c    |  2 +-
 src/backend/access/transam/xlog.c         |  4 ++--
 src/backend/access/transam/xlogrecovery.c |  8 ++++----
 src/backend/replication/repl_scanner.l    |  2 +-
 src/backend/replication/walsender.c       |  2 +-
 src/bin/pg_basebackup/pg_basebackup.c     |  6 +++---
 src/bin/pg_basebackup/pg_receivewal.c     |  2 +-
 src/bin/pg_basebackup/pg_recvlogical.c    |  4 ++--
 src/bin/pg_basebackup/streamutil.c        |  2 +-
 src/bin/pg_rewind/pg_rewind.c             |  2 +-
 src/bin/pg_rewind/timeline.c              |  2 +-
 src/bin/pg_waldump/pg_waldump.c           | 14 +++++++-------
 src/include/catalog/pg_proc.dat           | 16 ++++++++--------
 13 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c
index e7452af679..a7ffbd7590 100644
--- a/src/backend/access/rmgrdesc/xlogdesc.c
+++ b/src/backend/access/rmgrdesc/xlogdesc.c
@@ -44,7 +44,7 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
     {
         CheckPoint *checkpoint = (CheckPoint *) rec;
 
-        appendStringInfo(buf, "redo %X/%X; "
+        appendStringInfo(buf, "redo lsn %X/%X; "
                          "tli %u; prev tli %u; fpw %s; xid %u:%u; oid %u; multi %u; offset %u; "
                          "oldest xid %u in DB %u; oldest multi %u in DB %u; "
                          "oldest/newest commit timestamp xid: %u/%u; "
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ce3815ea99..bdf03f07ab 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1470,7 +1470,7 @@ WaitXLogInsertionsToFinish(XLogRecPtr upto)
     if (upto > reservedUpto)
     {
         ereport(LOG,
-                (errmsg("request to flush past end of generated WAL; request %X/%X, current position %X/%X",
+                (errmsg("request to flush past end of generated WAL; request lsn %X/%X, current lsn %X/%X",
                         LSN_FORMAT_ARGS(upto), LSN_FORMAT_ARGS(reservedUpto))));
         upto = reservedUpto;
     }
@@ -4915,7 +4915,7 @@ StartupXLOG(void)
      */
     if (!XRecOffIsValid(ControlFile->checkPoint))
         ereport(FATAL,
-                (errmsg("control file contains invalid checkpoint location")));
+                (errmsg("control file contains invalid checkpoint LSN")));
 
     switch (ControlFile->state)
     {
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index f9f212680b..dc822da402 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -530,7 +530,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
                             recoveryTargetName)));
         else if (recoveryTarget == RECOVERY_TARGET_LSN)
             ereport(LOG,
-                    (errmsg("starting point-in-time recovery to WAL location (LSN) \"%X/%X\"",
+                    (errmsg("starting point-in-time recovery to WAL LSN \"%X/%X\"",
                             LSN_FORMAT_ARGS(recoveryTargetLSN))));
         else if (recoveryTarget == RECOVERY_TARGET_IMMEDIATE)
             ereport(LOG,
@@ -611,7 +611,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
                 if (!ReadRecord(xlogreader, LOG, false,
                                 checkPoint.ThisTimeLineID))
                     ereport(FATAL,
-                            (errmsg("could not find redo location referenced by checkpoint record"),
+                            (errmsg("could not find redo LSN referenced by checkpoint record"),
                              errhint("If you are restoring from a backup, touch \"%s/recovery.signal\" and add
requiredrecovery options.\n"
 
                                      "If you are not restoring from a backup, try removing the file
\"%s/backup_label\".\n"
                                      "Be careful: removing \"%s/backup_label\" will result in a corrupt cluster if
restoringfrom a backup.",
 
@@ -2405,7 +2405,7 @@ recoveryStopsBefore(XLogReaderState *record)
         recoveryStopTime = 0;
         recoveryStopName[0] = '\0';
         ereport(LOG,
-                (errmsg("recovery stopping before WAL location (LSN) \"%X/%X\"",
+                (errmsg("recovery stopping before WAL LSN \"%X/%X\"",
                         LSN_FORMAT_ARGS(recoveryStopLSN))));
         return true;
     }
@@ -2568,7 +2568,7 @@ recoveryStopsAfter(XLogReaderState *record)
         recoveryStopTime = 0;
         recoveryStopName[0] = '\0';
         ereport(LOG,
-                (errmsg("recovery stopping after WAL location (LSN) \"%X/%X\"",
+                (errmsg("recovery stopping after WAL LSN \"%X/%X\"",
                         LSN_FORMAT_ARGS(recoveryStopLSN))));
         return true;
     }
diff --git a/src/backend/replication/repl_scanner.l b/src/backend/replication/repl_scanner.l
index 4b64c0d768..a0f1c44cc7 100644
--- a/src/backend/replication/repl_scanner.l
+++ b/src/backend/replication/repl_scanner.l
@@ -138,7 +138,7 @@ WAIT                { return K_WAIT; }
                     uint32    hi,
                             lo;
                     if (sscanf(yytext, "%X/%X", &hi, &lo) != 2)
-                        yyerror("invalid streaming start location");
+                        yyerror("invalid streaming start LSN");
                     yylval.recptr = ((uint64) hi) << 32 | lo;
                     return RECPTR;
                 }
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 2d0292a092..95a043525f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -813,7 +813,7 @@ StartReplication(StartReplicationCmd *cmd)
         if (FlushPtr < cmd->startpoint)
         {
             ereport(ERROR,
-                    (errmsg("requested starting point %X/%X is ahead of the WAL flush position of this server %X/%X",
+                    (errmsg("requested starting point %X/%X is ahead of the WAL flush LSN of this server %X/%X",
                             LSN_FORMAT_ARGS(cmd->startpoint),
                             LSN_FORMAT_ARGS(FlushPtr))));
         }
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index d265ee3b3c..44d3ae1322 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -488,7 +488,7 @@ reached_end_position(XLogRecPtr segendpos, uint32 timeline,
 
             if (sscanf(xlogend, "%X/%X", &hi, &lo) != 2)
             {
-                pg_log_error("could not parse write-ahead log location \"%s\"",
+                pg_log_error("could not parse WAL LSN \"%s\"",
                              xlogend);
                 exit(1);
             }
@@ -643,7 +643,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier)
     /* Convert the starting position */
     if (sscanf(startpos, "%X/%X", &hi, &lo) != 2)
     {
-        pg_log_error("could not parse write-ahead log location \"%s\"",
+        pg_log_error("could not parse WAL LSN \"%s\"",
                      startpos);
         exit(1);
     }
@@ -2363,7 +2363,7 @@ BaseBackup(void)
          */
         if (sscanf(xlogend, "%X/%X", &hi, &lo) != 2)
         {
-            pg_log_error("could not parse write-ahead log location \"%s\"",
+            pg_log_error("could not parse WAL LSN \"%s\"",
                          xlogend);
             exit(1);
         }
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 8a4c2b8964..30d43ca598 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -753,7 +753,7 @@ main(int argc, char **argv)
             case 'E':
                 if (sscanf(optarg, "%X/%X", &hi, &lo) != 2)
                 {
-                    pg_log_error("could not parse end position \"%s\"", optarg);
+                    pg_log_error("could not parse end LSN \"%s\"", optarg);
                     exit(1);
                 }
                 endpos = ((uint64) hi) << 32 | lo;
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index cc35d16f32..0bfb9edb94 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -781,7 +781,7 @@ main(int argc, char **argv)
             case 'I':
                 if (sscanf(optarg, "%X/%X", &hi, &lo) != 2)
                 {
-                    pg_log_error("could not parse start position \"%s\"", optarg);
+                    pg_log_error("could not parse start LSN \"%s\"", optarg);
                     exit(1);
                 }
                 startpos = ((uint64) hi) << 32 | lo;
@@ -789,7 +789,7 @@ main(int argc, char **argv)
             case 'E':
                 if (sscanf(optarg, "%X/%X", &hi, &lo) != 2)
                 {
-                    pg_log_error("could not parse end position \"%s\"", optarg);
+                    pg_log_error("could not parse end LSN \"%s\"", optarg);
                     exit(1);
                 }
                 endpos = ((uint64) hi) << 32 | lo;
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 4a6afd1a06..41d0473138 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -447,7 +447,7 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
     {
         if (sscanf(PQgetvalue(res, 0, 2), "%X/%X", &hi, &lo) != 2)
         {
-            pg_log_error("could not parse write-ahead log location \"%s\"",
+            pg_log_error("could not parse WAL LSN \"%s\"",
                          PQgetvalue(res, 0, 2));
 
             PQclear(res);
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index b39b5c1aac..61e5c38629 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -344,7 +344,7 @@ main(int argc, char **argv)
         XLogRecPtr    chkptendrec;
 
         findCommonAncestorTimeline(&divergerec, &lastcommontliIndex);
-        pg_log_info("servers diverged at WAL location %X/%X on timeline %u",
+        pg_log_info("servers diverged at WAL LSN %X/%X on timeline %u",
                     LSN_FORMAT_ARGS(divergerec),
                     targetHistory[lastcommontliIndex].tli);
 
diff --git a/src/bin/pg_rewind/timeline.c b/src/bin/pg_rewind/timeline.c
index df8f82a50c..0b3c524f16 100644
--- a/src/bin/pg_rewind/timeline.c
+++ b/src/bin/pg_rewind/timeline.c
@@ -79,7 +79,7 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
         if (nfields != 3)
         {
             pg_log_error("syntax error in history file: %s", fline);
-            pg_log_error("Expected a write-ahead log switchpoint location.");
+            pg_log_error("Expected a WAL switchpoint LSN.");
             exit(1);
         }
         if (entries && tli <= lasttli)
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index f128050b4e..5ed1942e24 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -764,7 +764,7 @@ usage(void)
     printf(_("  %s [OPTION]... [STARTSEG [ENDSEG]]\n"), progname);
     printf(_("\nOptions:\n"));
     printf(_("  -b, --bkp-details      output detailed information about backup blocks\n"));
-    printf(_("  -e, --end=RECPTR       stop reading at WAL location RECPTR\n"));
+    printf(_("  -e, --end=RECPTR       stop reading at WAL LSN RECPTR\n"));
     printf(_("  -f, --follow           keep retrying after reaching end of WAL\n"));
     printf(_("  -n, --limit=N          number of records to display\n"));
     printf(_("  -p, --path=PATH        directory in which to find log segment files or a\n"
@@ -773,7 +773,7 @@ usage(void)
     printf(_("  -q, --quiet            do not print any output, except for errors\n"));
     printf(_("  -r, --rmgr=RMGR        only show records generated by resource manager RMGR;\n"
              "                         use --rmgr=list to list valid resource manager names\n"));
-    printf(_("  -s, --start=RECPTR     start reading at WAL location RECPTR\n"));
+    printf(_("  -s, --start=RECPTR     start reading at WAL LSN RECPTR\n"));
     printf(_("  -t, --timeline=TLI     timeline from which to read log records\n"
              "                         (default: 1 or the value used in STARTSEG)\n"));
     printf(_("  -V, --version          output version information, then exit\n"));
@@ -882,7 +882,7 @@ main(int argc, char **argv)
             case 'e':
                 if (sscanf(optarg, "%X/%X", &xlogid, &xrecoff) != 2)
                 {
-                    pg_log_error("could not parse end WAL location \"%s\"",
+                    pg_log_error("could not parse end WAL LSN \"%s\"",
                                  optarg);
                     goto bad_argument;
                 }
@@ -934,7 +934,7 @@ main(int argc, char **argv)
             case 's':
                 if (sscanf(optarg, "%X/%X", &xlogid, &xrecoff) != 2)
                 {
-                    pg_log_error("could not parse start WAL location \"%s\"",
+                    pg_log_error("could not parse start WAL LSN \"%s\"",
                                  optarg);
                     goto bad_argument;
                 }
@@ -1025,7 +1025,7 @@ main(int argc, char **argv)
             XLogSegNoOffsetToRecPtr(segno, 0, WalSegSz, private.startptr);
         else if (!XLByteInSeg(private.startptr, segno, WalSegSz))
         {
-            pg_log_error("start WAL location %X/%X is not inside file \"%s\"",
+            pg_log_error("start WAL LSN %X/%X is not inside file \"%s\"",
                          LSN_FORMAT_ARGS(private.startptr),
                          fname);
             goto bad_argument;
@@ -1067,7 +1067,7 @@ main(int argc, char **argv)
         if (!XLByteInSeg(private.endptr, segno, WalSegSz) &&
             private.endptr != (segno + 1) * WalSegSz)
         {
-            pg_log_error("end WAL location %X/%X is not inside file \"%s\"",
+            pg_log_error("end WAL LSN %X/%X is not inside file \"%s\"",
                          LSN_FORMAT_ARGS(private.endptr),
                          argv[argc - 1]);
             goto bad_argument;
@@ -1079,7 +1079,7 @@ main(int argc, char **argv)
     /* we don't know what to print */
     if (XLogRecPtrIsInvalid(private.startptr))
     {
-        pg_log_error("no start WAL location given");
+        pg_log_error("no start WAL LSN given");
         goto bad_argument;
     }
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d8e8715ed1..3e83f68daf 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6299,24 +6299,24 @@
   proname => 'pg_create_restore_point', provolatile => 'v',
   prorettype => 'pg_lsn', proargtypes => 'text',
   prosrc => 'pg_create_restore_point' },
-{ oid => '2849', descr => 'current wal write location',
+{ oid => '2849', descr => 'current wal write lsn',
   proname => 'pg_current_wal_lsn', provolatile => 'v', prorettype => 'pg_lsn',
   proargtypes => '', prosrc => 'pg_current_wal_lsn' },
-{ oid => '2852', descr => 'current wal insert location',
+{ oid => '2852', descr => 'current wal insert lsn',
   proname => 'pg_current_wal_insert_lsn', provolatile => 'v',
   prorettype => 'pg_lsn', proargtypes => '',
   prosrc => 'pg_current_wal_insert_lsn' },
-{ oid => '3330', descr => 'current wal flush location',
+{ oid => '3330', descr => 'current wal flush lsn',
   proname => 'pg_current_wal_flush_lsn', provolatile => 'v',
   prorettype => 'pg_lsn', proargtypes => '',
   prosrc => 'pg_current_wal_flush_lsn' },
 { oid => '2850',
-  descr => 'wal filename and byte offset, given a wal location',
+  descr => 'wal filename and byte offset, given a wal lsn',
   proname => 'pg_walfile_name_offset', prorettype => 'record',
   proargtypes => 'pg_lsn', proallargtypes => '{pg_lsn,text,int4}',
   proargmodes => '{i,o,o}', proargnames => '{lsn,file_name,file_offset}',
   prosrc => 'pg_walfile_name_offset' },
-{ oid => '2851', descr => 'wal filename, given a wal location',
+{ oid => '2851', descr => 'wal filename, given a wal lsn',
   proname => 'pg_walfile_name', prorettype => 'text', proargtypes => 'pg_lsn',
   prosrc => 'pg_walfile_name' },
 
@@ -6332,11 +6332,11 @@
   proname => 'pg_is_in_recovery', provolatile => 'v', prorettype => 'bool',
   proargtypes => '', prosrc => 'pg_is_in_recovery' },
 
-{ oid => '3820', descr => 'current wal flush location',
+{ oid => '3820', descr => 'current wal flush lsn',
   proname => 'pg_last_wal_receive_lsn', provolatile => 'v',
   prorettype => 'pg_lsn', proargtypes => '',
   prosrc => 'pg_last_wal_receive_lsn' },
-{ oid => '3821', descr => 'last wal replay location',
+{ oid => '3821', descr => 'last wal replay lsn',
   proname => 'pg_last_wal_replay_lsn', provolatile => 'v',
   prorettype => 'pg_lsn', proargtypes => '',
   prosrc => 'pg_last_wal_replay_lsn' },
@@ -11515,7 +11515,7 @@
   proparallel => 'r', prorettype => 'void', proargtypes => '',
   prosrc => 'pg_replication_origin_xact_reset' },
 
-{ oid => '6012', descr => 'advance replication origin to specific location',
+{ oid => '6012', descr => 'advance replication origin to specific lsn',
   proname => 'pg_replication_origin_advance', provolatile => 'v',
   proparallel => 'u', prorettype => 'void', proargtypes => 'text pg_lsn',
   prosrc => 'pg_replication_origin_advance' },
-- 
2.27.0

From 535816060ef99816814198921017ab622a795d25 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Fri, 4 Mar 2022 13:53:38 +0900
Subject: [PATCH v12 5/5] Get rid of uses of some hyphenated words

There are use of both "archive-recovery" and "archive recovery" in the
tree. Unify the similar uses to unhyphenated.  The use of
"point-in-time-recovery" is left alone as it is used as the
explanation for the acronym "PITR".
---
 doc/src/sgml/ref/pg_ctl-ref.sgml   | 2 +-
 doc/src/sgml/ref/postgres-ref.sgml | 2 +-
 src/backend/commands/trigger.c     | 2 +-
 src/backend/utils/cache/relcache.c | 2 +-
 src/test/regress/parallel_schedule | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
index 3946fa52ea..7cbe5c3048 100644
--- a/doc/src/sgml/ref/pg_ctl-ref.sgml
+++ b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -194,7 +194,7 @@ PostgreSQL documentation
    rolled back and clients are forcibly disconnected, then the
    server is shut down.  <quote>Immediate</quote> mode will abort
    all server processes immediately, without a clean shutdown.  This choice
-   will lead to a crash-recovery cycle during the next server start.
+   will lead to a crash recovery cycle during the next server start.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/postgres-ref.sgml b/doc/src/sgml/ref/postgres-ref.sgml
index 55a3f6c69d..fc22fbc4fe 100644
--- a/doc/src/sgml/ref/postgres-ref.sgml
+++ b/doc/src/sgml/ref/postgres-ref.sgml
@@ -719,7 +719,7 @@ PostgreSQL documentation
    is also unwise to send <literal>SIGKILL</literal> to a server
    process — the main <command>postgres</command> process will
    interpret this as a crash and will force all the sibling processes
-   to quit as part of its standard crash-recovery procedure.
+   to quit as part of its standard crash recovery procedure.
   </para>
  </refsect1>
 
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index e08bd9a370..ce170b4c6e 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -1887,7 +1887,7 @@ RelationBuildTriggers(Relation relation)
     /*
      * Note: since we scan the triggers using TriggerRelidNameIndexId, we will
      * be reading the triggers in name order, except possibly during
-     * emergency-recovery operations (ie, IgnoreSystemIndexes). This in turn
+     * emergency recovery operations (ie, IgnoreSystemIndexes). This in turn
      * ensures that triggers will be fired in name order.
      */
     ScanKeyInit(&skey,
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index fccffce572..2419cf5285 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -6598,7 +6598,7 @@ RelationCacheInitFilePostInvalidate(void)
  * Remove the init files during postmaster startup.
  *
  * We used to keep the init files across restarts, but that is unsafe in PITR
- * scenarios, and even in simple crash-recovery cases there are windows for
+ * scenarios, and even in simple crash recovery cases there are windows for
  * the init files to become out-of-sync with the database.  So now we just
  * remove them during startup and expect the first backend launch to rebuild
  * them.  Of course, this has to happen in each database of the cluster.
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 6d8f524ae9..8251744bbf 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -13,7 +13,7 @@ test: test_setup
 
 # run tablespace by itself, and early, because it forces a checkpoint;
 # we'd prefer not to have checkpoints later in the tests because that
-# interferes with crash-recovery testing.
+# interferes with crash recovery testing.
 test: tablespace
 
 # ----------
-- 
2.27.0


pgsql-hackers by date:

Previous
From: Julien Rouhaud
Date:
Subject: Re: Can we consider "24 Hours" for "next day" in INTERVAL datatype ?
Next
From: Tomas Vondra
Date:
Subject: Re: POC: GROUP BY optimization