Re: Stefan's bug (was: max_standby_delay considered harmful)

On Mon, May 24, 2010 at 10:35 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Mon, May 24, 2010 at 10:26 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> This looks pretty reasonable to me, but I guess I feel like it would
>> be better to drive the CancelBackup() decision off of whether we've
>> ever reached PM_RUN rather than consulting XLogCtl.  It just feels
>> cleaner to me to drive all of the postmaster decisions off of the same
>> signalling mechanism rather than having a separate one (that only
>> works because it's used very late in shutdown when we theoretically
>> don't need a lock) just for this one case.
>
> Okay, how about the attached patch? It uses the postmaster-local flag
> "ReachedEndOfRecovery" (better name?) instead of XLogCtl one.

I've committed part of this patch, with the naming change that Tom
suggested. The parts I haven't committed are:

1. I don't see why we need to reset ReachedEndOfRecovery starting over
from PM_NO_CHILDREN. It seems to me that once we reach PM_RUN, we
CAN'T go back to needing the backup label file, even if we have a
subsequent backend crash. If I'm wrong, please let me know why and
I'll go put this back (with an appropriate comment).

2. The changes to avoid launching WALReceiver except during certain
PM_* states. It seems fairly sensible, but what is the case where
adding this logic prevents a problem?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company