Re: PITR potentially broken in 9.2

On 2012-12-05 17:24:42 +0000, Simon Riggs wrote:
> On 5 December 2012 17:17, Simon Riggs <simon@2ndquadrant.com> wrote:
>
> > The recovery target and the consistency point are in some ways in
> > conflict. If the recovery target is before the consistency point there
> > is no point in stopping there, whether or not we pause. What we should
> > do is say "recovery target reached, yet recovery not yet consistent,
> > continuing".
> > So ISTM that we should make recoveryStopsHere() return false while we
> > are inconsistent. Problems solved.

I prefer the previous (fixed) behaviour where we error out if we reach a
recovery target before we are consistent:

    /*
     * Complain if we did not roll forward far enough to render the backup
     * dump consistent.  Note: it is indeed okay to look at the local variable
     * minRecoveryPoint here, even though ControlFile->minRecoveryPoint might
     * be further ahead --- ControlFile->minRecoveryPoint cannot have been
     * advanced beyond the WAL we processed.
     */
    if (InRecovery &&
        (XLByteLT(EndOfLog, minRecoveryPoint) ||
         !XLogRecPtrIsInvalid(ControlFile->backupStartPoint)))
    {
        if (reachedStopPoint)
        {
            /* stopped because of stop request */
            ereport(FATAL,
                    (errmsg("requested recovery stop point is before consistent recovery point")));
        }

        /*
         * Ran off end of WAL before reaching end-of-backup WAL record, or
         * minRecoveryPoint. That's usually a bad sign, indicating that you
         * tried to recover from an online backup but never called
         * pg_stop_backup(), or you didn't archive all the WAL up to that
         * point. However, this also happens in crash recovery, if the system
         * crashes while an online backup is in progress. We must not treat
         * that as an error, or the database will refuse to start up.
         */
        if (InArchiveRecovery || ControlFile->backupEndRequired)
        {
            if (ControlFile->backupEndRequired)
                ereport(FATAL,
                        (errmsg("WAL ends before end of online backup"),
                         errhint("All WAL generated while online backup was taken must be available at recovery.")));
            else if (!XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
                ereport(FATAL,
                        (errmsg("WAL ends before end of online backup"),
                         errhint("Online backup started with pg_start_backup() must be ended with pg_stop_backup(), and
allWAL up to that point must be available at recovery."))); 
            else
                ereport(FATAL,
                      (errmsg("WAL ends before consistent recovery point")));
        }
    }

Seems to be good enough to me. Once the pause logic is fixed obviously.


Greetings,

Andres Freund

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