Обсуждение: [HACKERS] What does it mean by XLOG_BACKUP_RECORD?
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 weknow 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
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
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
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
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