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 | 20220316.102453.119602362889742790.horikyota.ntt@gmail.com Whole thread Raw |
| In response to | Add checkpoint and redo LSN to LogCheckpointEnd log message (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>) |
| Responses |
Reword "WAL location" as "WAL LSN"
(Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
|
| List | pgsql-hackers |
Hello, this is a follow-up topic of [1] (add LSNs to checkpint logs). Many user-facing texts contains wording like "WAL location" or such like. The attached is WIP patches that fixes such wordings to "WAL LSN" or alikes. The attached patches is v13 but it is not changed at all from v12. My lastest comments on this version are as follows. 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. regards. [1] https://www.postgresql.org/message-id/20220316.091913.806120467943749797.horikyota.ntt%40gmail.com -- Kyotaro Horiguchi NTT Open Source Software Center From afed063f09b788bc07a0e2665499c3e3c3edc214 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 v13 1/2] 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 4cc657baf834bdab64b7aa4665962f7634ae73fb 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 v13 2/2] 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 ed16f279b1..5e7516f67b 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1471,7 +1471,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; } @@ -4916,7 +4916,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 2943d9ec1a..7951ae4312 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 e2ceafeb0f..20425699af 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
pgsql-hackers by date: