On 29.01.2013 13:46, Simon Riggs wrote:
> 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.
You might want to bring the database up ASAP after restoring. If the
user requests that, the system shouldn't second-guess that.
PS. I think the implicit judgment you made that "pg_ctl promote" is now
the preferred method of promoting the server, over the trigger file
method, needs more discussion. I'm not sure I agree with that, but if we
do that, the docs should emphasize the pg_ctl promote more than the
trigger file. Also, I don't like conflating the shutdown mode argument
with promotion mode either, in pg_ctl. Perhaps it would be best to
revert this and take some more time to discuss the right behavior and
user interface for this (if it needs one).
PPS. doc changes are missing...
- Heikki
