Re: Hot standby, recovery infra

Hi,

On Wed, Jan 28, 2009 at 11:19 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> I feel quite good about this patch now. Given the amount of code churn, it
>> requires testing, and I'll read it through one more time after sleeping over
>> it. Simon, do you see anything wrong with this?
>
> I also read this patch and found something odd. I apologize if I misread it..

Sorry for my random reply.

Though this is a matter of taste, I think that it's weird that bgwriter
runs with ThisTimeLineID = 0 during recovery. This is because
XLogCtl->ThisTimeLineID is set at the end of recovery. ISTM this will
be a cause of bug in the near future, though this is harmless currently.

> + /*
> + * XXX: Should we try to perform restartpoints if we're not in consistent
> + * recovery? The bgwriter isn't doing it for us in that case.
> + */

I think yes. This is helpful for the system which it takes a long time to get
a base backup, ie. it also takes a long time to reach a consistent recovery
point.

> +CreateRestartPoint(int flags)
<snip>
> + * We rely on this lock to ensure that the startup process doesn't exit
> + * Recovery while we are half way through a restartpoint. XXX ?
> */
> + LWLockAcquire(CheckpointLock, LW_EXCLUSIVE);

Is this comment correct? CheckpointLock cannot prevent the startup process
from exiting recovery because the startup process doesn't acquire that lock.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center