What does it mean by XLOG_BACKUP_RECORD?

Started by Masahiko Sawadaover 8 years ago5 messages
#1Masahiko Sawada
sawada.mshk@gmail.com

Hi,

While reading source codes I found the following comment in xlog.c.

/*
* Have we passed our safe starting point? Note that minRecoveryPoint is
* known to be incorrectly set if ControlFile->backupEndRequired, until
* the XLOG_BACKUP_RECORD arrives to advise us of the correct
* minRecoveryPoint. All we know prior to that is that we're not
* consistent yet.
*/
if (!reachedConsistency && !ControlFile->backupEndRequired &&
minRecoveryPoint <= lastReplayedEndRecPtr &&
XLogRecPtrIsInvalid(ControlFile->backupStartPoint))

What does XLOG_BACKUP_RECORED means by? I could not find such XLOG info value.
Does it mean XLOG_BACKUP_END?

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Michael Paquier
michael.paquier@gmail.com
In reply to: Masahiko Sawada (#1)
1 attachment(s)
Re: What does it mean by XLOG_BACKUP_RECORD?

On Thu, Jun 29, 2017 at 10:28 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

While reading source codes I found the following comment in xlog.c.

/*
* Have we passed our safe starting point? Note that minRecoveryPoint is
* known to be incorrectly set if ControlFile->backupEndRequired, until
* the XLOG_BACKUP_RECORD arrives to advise us of the correct
* minRecoveryPoint. All we know prior to that is that we're not
* consistent yet.
*/
if (!reachedConsistency && !ControlFile->backupEndRequired &&
minRecoveryPoint <= lastReplayedEndRecPtr &&
XLogRecPtrIsInvalid(ControlFile->backupStartPoint))

What does XLOG_BACKUP_RECORED means by? I could not find such XLOG info value.
Does it mean XLOG_BACKUP_END?

This comment is a thinko, it refers to XLOG_BACKUP_END. This comment
block could be reworded a bit, it looks cleaner to me to say
"ControlFile->backupEndRequired is false" instead of just referring to
the variable itself. Worse, the current comment implies that
minRecoveryPoint is incorrectly set if it is true. Bleh.
--
Michael

Attachments:

xlog-comment-fix.patchapplication/octet-stream; name=xlog-comment-fix.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 0a6314a642..afbd6d536a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7817,8 +7817,8 @@ CheckRecoveryConsistency(void)
 
 	/*
 	 * Have we passed our safe starting point? Note that minRecoveryPoint is
-	 * known to be incorrectly set if ControlFile->backupEndRequired, until
-	 * the XLOG_BACKUP_RECORD arrives to advise us of the correct
+	 * known to be incorrectly set if ControlFile->backupEndRequired is false,
+	 * until the XLOG_BACKUP_END record arrives to advise us of the correct
 	 * minRecoveryPoint. All we know prior to that is that we're not
 	 * consistent yet.
 	 */
#3Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Michael Paquier (#2)
Re: What does it mean by XLOG_BACKUP_RECORD?

On Thu, Jun 29, 2017 at 10:49 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Thu, Jun 29, 2017 at 10:28 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

While reading source codes I found the following comment in xlog.c.

/*
* Have we passed our safe starting point? Note that minRecoveryPoint is
* known to be incorrectly set if ControlFile->backupEndRequired, until
* the XLOG_BACKUP_RECORD arrives to advise us of the correct
* minRecoveryPoint. All we know prior to that is that we're not
* consistent yet.
*/
if (!reachedConsistency && !ControlFile->backupEndRequired &&
minRecoveryPoint <= lastReplayedEndRecPtr &&
XLogRecPtrIsInvalid(ControlFile->backupStartPoint))

What does XLOG_BACKUP_RECORED means by? I could not find such XLOG info value.
Does it mean XLOG_BACKUP_END?

This comment is a thinko, it refers to XLOG_BACKUP_END. This comment
block could be reworded a bit, it looks cleaner to me to say
"ControlFile->backupEndRequired is false" instead of just referring to
the variable itself.

Thanks, I agree to use XLOG_BACKUP_END instead.

Worse, the current comment implies that
minRecoveryPoint is incorrectly set if it is true. Bleh.

Looking at the condition, we use minRecoveryPoint only when
ControlFile->backupEndRequired is *false*. So I guess that it means
that minRecoveryPoint is incorrectly set if
ControlFile->backupEndReuired is true. Am I missing something?

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Masahiko Sawada (#3)
Re: [HACKERS] What does it mean by XLOG_BACKUP_RECORD?

On 6/29/17 06:09, Masahiko Sawada wrote:

Thanks, I agree to use XLOG_BACKUP_END instead.

Worse, the current comment implies that
minRecoveryPoint is incorrectly set if it is true. Bleh.

Looking at the condition, we use minRecoveryPoint only when
ControlFile->backupEndRequired is *false*. So I guess that it means
that minRecoveryPoint is incorrectly set if
ControlFile->backupEndReuired is true. Am I missing something?

I agree with you that the logic in the comment is correct. I've
committed just the symbol change.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#5Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Peter Eisentraut (#4)
Re: [HACKERS] What does it mean by XLOG_BACKUP_RECORD?

On Sat, Dec 9, 2017 at 1:25 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 6/29/17 06:09, Masahiko Sawada wrote:

Thanks, I agree to use XLOG_BACKUP_END instead.

Worse, the current comment implies that
minRecoveryPoint is incorrectly set if it is true. Bleh.

Looking at the condition, we use minRecoveryPoint only when
ControlFile->backupEndRequired is *false*. So I guess that it means
that minRecoveryPoint is incorrectly set if
ControlFile->backupEndReuired is true. Am I missing something?

I agree with you that the logic in the comment is correct. I've
committed just the symbol change.

Thank you for picking up an old thread and committing it!

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center