On Mon, Jan 31, 2022 at 8:41 PM Stephen Frost <sfr...@snowman.net> wrote:
>
> > Thanks for your review. In summary, we have these options to choose
> > checkpoint LSN and last REDO LSN:
> >
> > 1) "start=%X/%X, end=%X/%X" (ControlFile->checkPointCopy.redo,
> > ControlFile->checkPoint)
> > 2) "lsn=%X/%X, redo lsn=%X/%X"
> > 3) "location=%X/%X, REDO location=%X/%X" -- similar to what
> > pg_controldata and pg_control_checkpoint shows currently.
> > 4) "location=%X/%X, REDO start location=%X/%X"
> >
> > I will leave that decision to the committer.
>
> I'd also vote for #2. Regarding 3 and 4, I'd argue that those should
> have been changed when we changed a number of other things from the
> generic 'location' to be 'lsn' in system views and functions, and
> therefore we should go change those to also specify 'lsn' rather than
> just saying 'location'.
Thanks. Here are 2 patches, 0001 for adding checkpoint lsn and redo
lsn in the checkpoint completed message and 0002 for changing the
"location" to LSN in pg_controdata's output. With the 0002,
pg_control_checkpont, pg_controldata and checkpoint completed message
will all be in sync with the checkpoint lsn and redo lsn.
Regards,
Bharath Rupireddy.
From beafea457cd80266c6e517862ed819da7b3c8ec0 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Mon, 31 Jan 2022 16:51:47 +0000
Subject: [PATCH v5] 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 | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index dfe2a0bcce..71a5239619 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8919,11 +8919,14 @@ LogCheckpointEnd(bool restartpoint)
if (restartpoint)
ereport(LOG,
- (errmsg("restartpoint complete: wrote %d buffers (%.1f%%); "
+ (errmsg("restartpoint complete: lsn=%X/%X, redo lsn=%X/%X; "
+ "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",
+ LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo),
+ LSN_FORMAT_ARGS(ControlFile->checkPoint),
CheckpointStats.ckpt_bufs_written,
(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
CheckpointStats.ckpt_segs_added,
@@ -8939,11 +8942,14 @@ LogCheckpointEnd(bool restartpoint)
(int) (CheckPointDistanceEstimate / 1024.0))));
else
ereport(LOG,
- (errmsg("checkpoint complete: wrote %d buffers (%.1f%%); "
+ (errmsg("checkpoint complete: lsn=%X/%X, redo lsn=%X/%X; "
+ "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",
+ LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo),
+ LSN_FORMAT_ARGS(ControlFile->checkPoint),
CheckpointStats.ckpt_bufs_written,
(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
CheckpointStats.ckpt_segs_added,
--
2.25.1
From fd466fb14f16db15292974b475338335668148bf Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Mon, 31 Jan 2022 17:50:40 +0000
Subject: [PATCH v5] 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..0c5ef7804c 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 86fd6f5546..7ab4d4429d 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.25.1
