public inbox for [email protected]  

help / color / mirror / Atom feed
From: Fujii Masao <[email protected]>
To: Bharath Rupireddy <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: Stephen Frost <[email protected]>
Cc: Bossart, Nathan <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
Date: Wed, 2 Feb 2022 01:09:43 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALj2ACUtZhTb=2ENkF3BQ3wi137uaGi__qzvXC-qFYC0XwjALw@mail.gmail.com>
References: <CALj2ACXDHGfhKkwzi81BRqWft4n85AO1zmkeqf6xEg+0xTCH9g@mail.gmail.com>
	<[email protected]>
	<CALj2ACU9ynpCwfuL0a4Bx0SDbHcjRDk49pwjDwfOzoQ1-kx1pw@mail.gmail.com>
	<[email protected]>
	<CALj2ACUtZhTb=2ENkF3BQ3wi137uaGi__qzvXC-qFYC0XwjALw@mail.gmail.com>



On 2022/02/01 22:03, Bharath Rupireddy wrote:
> On Tue, Feb 1, 2022 at 11:58 AM Kyotaro Horiguchi
> <[email protected]> wrote:
>>> Modified in v8.
>>
>> 0001 looks good to me.

I found that CreateRestartPoint() already reported the redo lsn as follows after emitting the restartpoint log message. To avoid duplicated logging of the same information, we should update this code?

	ereport((log_checkpoints ? LOG : DEBUG2),
			(errmsg("recovery restart point at %X/%X",
					LSN_FORMAT_ARGS(lastCheckPoint.redo)),
			 xtime ? errdetail("Last completed transaction was at log time %s.",
							   timestamptz_to_str(xtime)) : 0));

This code reports lastCheckPoint.redo as redo lsn. OTOH, with the patch, LogCheckpointEnd() reports ControlFile->checkPointCopy.redo. They may be different, for example, when the current DB state is not DB_IN_ARCHIVE_RECOVERY. In this case, which lsn should we report as redo lsn?

+						"lsn=%X/%X, redo lsn=%X/%X",

Originally you proposed to use upper cases for "lsn". But the latest patch uses the lower cases. Why? It seems better to use upper cases, i.e., LSN and REDO LSN because LSN is basically used in other errmsg().

> Attaching the above changes 0003 (0001 and 0002 remain the same). If
> the committer doesn't agree on the text or wording in 0003, I would
> like the 0001 and 0002 to be taken here and I can start a new thread
> for discussing 0003 separately.

Personally I'm ok with 001, but regarding 0002 and 0003 patches, I'm not sure if it's really worth replacing "location" with "lsn" there. BTW, the similar idea was proposed at [1] before, but seems "location" was left as it was.

[1]
https://postgr.es/m/[email protected]

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION






view thread (6+ messages)  latest in thread

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox