On 29 January 2013 11:31, Heikki Linnakangas <hlinnakangas@vmware.com> wrote:
> On 29.01.2013 02:07, Simon Riggs wrote:
>>
>> + /*
>> + * If we've been explicitly promoted with fast
>> option,
>> + * end of recovery without a checkpoint if
>> possible.
>> + */
>> + if (fast_promote)
>> + {
>> + checkPointLoc =
>> ControlFile->prevCheckPoint;
>> + record = ReadCheckpointRecord(xlogreader,
>> checkPointLoc, 2, false);
>> + if (record != NULL)
>> + {
>> + checkpoint_wait = false;
>> + CreateEndOfRecoveryRecord();
>> + }
>> + }
>
>
> If we must have this ReadCheckPointRecord check, it needs more than zero
> comments. Also, if it ever fails for some reason, I'd like to have a big fat
> warning in the log to caution that something went badly wrong.
> Why does it insist that we still have not only the latest checkpoint, but
> the previous one too? At recovery, we fall back to the previous checkpoint
> if we can't access the latest one, but that's just a desperate measure to
> try to recover something if things have gone badly wrong. It's OK to not
> have the WAL containing the previous checkpoint still around. In particular,
> right after restoring from a base backup, e.g with pg_basebackup -x, or with
> good old pg_start/stop_backup, the WAL included with the backup won't
> stretch back to previous checkpoint.
As you say, there are cases where the lack of a secondary checkpoint
could be considered normal, hence no message to confuse the user.
We don't actually need a fast promotion when restoring from backup, so
we don't do it.
I want this to work for the cases we need it, and not break when we
don't need it.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
