Thread: pgsql: Must not reach consistency before XLOG_BACKUP_RECORD

pgsql: Must not reach consistency before XLOG_BACKUP_RECORD

From
Simon Riggs
Date:
Must not reach consistency before XLOG_BACKUP_RECORD
When waiting for an XLOG_BACKUP_RECORD the minRecoveryPoint
will be incorrect, so we must not declare recovery as consistent
before we have seen the record. Major bug allowing recovery to end
too early in some cases, allowing people to see inconsistent db.
This patch to HEAD and 9.2, other fix required for 9.1 and 9.0

Simon Riggs and Andres Freund, bug report by Jeff Janes

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/6aa2e49a878d28fbbbe8efe53c3a729a51a01090

Modified Files
--------------
src/backend/access/transam/xlog.c |    7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)


Re: pgsql: Must not reach consistency before XLOG_BACKUP_RECORD

From
Tom Lane
Date:
Simon Riggs <simon@2ndQuadrant.com> writes:
> Must not reach consistency before XLOG_BACKUP_RECORD

> When waiting for an XLOG_BACKUP_RECORD the minRecoveryPoint
> will be incorrect, so we must not declare recovery as consistent
> before we have seen the record. Major bug allowing recovery to end
> too early in some cases, allowing people to see inconsistent db.

Is this actually a "major bug fix", or a useless redundant test?
I had thought that that if-statement's check of
XLogRecPtrIsInvalid(ControlFile->backupStartPoint)
was sufficient, because that will keep us from declaring consistency
before we get out of the backup in any case.

            regards, tom lane


Re: pgsql: Must not reach consistency before XLOG_BACKUP_RECORD

From
Andres Freund
Date:
On 2012-12-05 11:04:39 -0500, Tom Lane wrote:
> Simon Riggs <simon@2ndQuadrant.com> writes:
> > Must not reach consistency before XLOG_BACKUP_RECORD
>
> > When waiting for an XLOG_BACKUP_RECORD the minRecoveryPoint
> > will be incorrect, so we must not declare recovery as consistent
> > before we have seen the record. Major bug allowing recovery to end
> > too early in some cases, allowing people to see inconsistent db.
>
> Is this actually a "major bug fix", or a useless redundant test?
> I had thought that that if-statement's check of
> XLogRecPtrIsInvalid(ControlFile->backupStartPoint)
> was sufficient, because that will keep us from declaring consistency
> before we get out of the backup in any case.

I think youre right that its redundant. Adding the check seems to be
sensible from a robustness perspective in the long run, but its
certainly not relevant for the release process.

Greetings,

Andres

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services