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

On Mon, May 24, 2010 at 9:28 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Mon, 2010-05-24 at 09:26 -0400, Robert Haas wrote:
>> On Mon, May 24, 2010 at 1:27 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> > On Wed, May 19, 2010 at 2:47 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> >>>> Oh, right. How about allowing the postmaster only in PM_STARTUP,
>> >>>> PM_RECOVERY, PM_HOT_STANDBY or PM_WAIT_READONLY state to invoke
>> >>>> walreceiver? We can keep walreceiver alive until all read only
>> >>>> backends have gone, and prevent unexpected startup of walreceiver.
>> >>>
>> >>> Yes, that seems like something we should be checking, if we aren't already.
>> >>
>> >> I'll do that.
>> >
>> > Here is the updated version. I added the above-mentioned check
>> > into the patch.
>>
>> 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.
>
> That is exactly what XLogCtl tells us and why it is suggested for use.

Sure. My only point is that the postmaster doesn't (and can't) use
that method of getting the information at any other time when it is
needed, so I don't know why we'd want to use it in just this one case.
Maybe there's a reason, but it's not obvious to me.

>>  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.
>>
>> I could be all wet, though.

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