public inbox for [email protected]  

help / color / mirror / Atom feed

From: Fujii Masao <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
Date: Tue, 22 Feb 2022 01:59:45 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CALj2ACXkuMYfa1_RMtVyD+F8rg1F8fuPbku7wSyzdK79RvgdZw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>



On 2022/02/14 14:40, Kyotaro Horiguchi wrote:
> For backbranches, the attached for pg14 does part of the full patch.

Thanks for updating the patch!


> Of the following, I think we should do (a) and (b) to make future
> backpatchings easier.
> 
> a) Use RedoRecPtr and PriorRedoPtr after they are assigned.
> 
> b) Move assignment to PriorRedoPtr into the ControlFileLock section.

I failed to understand how (a) and (b) can make the backpatching easier. How easy to backpatch seems the same whether we apply (a) and (b) or not...


> c) Skip udpate of minRecoveryPoint only when the checkpoint gets old.

Yes.


> d) Skip call to UpdateCheckPointDistanceEstimate() when RedoRecPtr <=
>    PriorRedoPtr.

But "RedoRecPtr <= PriorRedoPtr" will never happen, will it? Because a restartpoint is skipped at the beginning of CreateRestartPoint() in that case. If this understanding is right, the check of "RedoRecPtr <= PriorRedoPtr" is not necessary before calling UpdateCheckPointDistanceEstimate().


+		ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+		ControlFile->minRecoveryPointTLI = 0;

Don't we need to update LocalMinRecoveryPoint and LocalMinRecoveryPointTLI after this? Maybe it's not necessary, but ISTM that it's safer and better to always update them whether the state is DB_IN_ARCHIVE_RECOVERY or not.


  		if (flags & CHECKPOINT_IS_SHUTDOWN)
  			ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;

Same as above. IMO it's safer and better to always update the state (whether the state is DB_IN_ARCHIVE_RECOVERY or not) if CHECKPOINT_IS_SHUTDOWN flag is passed.

Regards,

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

view thread (5+ messages)

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