Subject: Re: Add checkpoint and redo LSN to LogCheckpointEnd log message

From: Nathan Bossart <[email-masked]>
Date: Thu, 27 Jan 2022 21:46:20 -0800

On Fri, Jan 28, 2022 at 08:43:36AM +0530, Bharath Rupireddy wrote:
> 2022-01-28 03:06:10.213 UTC [2409486] LOG:  checkpoint starting:
> immediate force wait
> 2022-01-28 03:06:10.257 UTC [2409486] LOG:  checkpoint complete:
> start=0/14D9510, end=0/14D9548; wrote 4 buffers (0.0%); 0 WAL file(s)
> added, 0 removed, 0 recycled; write=0.007 s, sync=0.008 s, total=0.044
> s; sync files=3, longest=0.005 s, average=0.003 s; distance=0 kB,
> estimate=0 kB
> 
> 2022-01-28 03:06:42.254 UTC [2409486] LOG:  checkpoint starting:
> immediate force wait
> 2022-01-28 03:06:42.279 UTC [2409486] LOG:  checkpoint complete:
> start=0/14DBDB8, end=0/14DBDF0; wrote 2 buffers (0.0%); 0 WAL file(s)
> added, 0 removed, 0 recycled; write=0.004 s, sync=0.004 s, total=0.025
> s; sync files=2, longest=0.003 s, average=0.002 s; distance=10 kB,
> estimate=10 kB

I know I voted for "start=%X/%X, end=%X/%X," but looking at this again, I
wonder if it could be misleading.  "start" is the redo location, and "end"
is the location of the checkpoint record, but I could understand why
someone might think that "start" is the location of the previous checkpoint
record and "end" is the redo location of the new one.  I think your
original idea of "lsn=%X/%X, redo lsn=%X/%X" could be a good alternative.

Іn any case, this patch is small and otherwise looks reasonable to me, so I
am going to mark it as ready-for-committer.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com/

---

From: Bharath Rupireddy <[email-masked]>
Date: Fri, 28 Jan 2022 12:09:28 +0530

> am going to mark it as ready-for-committer.

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.

Regards,
Bharath Rupireddy.

---

From: Stephen Frost <[email-masked]>
Date: Mon, 31 Jan 2022 10:11:00 -0500

Greetings,

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

Stephen