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

On Tue, May 18, 2010 at 5:10 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On Tue, 2010-05-18 at 16:05 +0900, Fujii Masao wrote:
>> >>> >> ISTM that we can use XLogCtl->SharedRecoveryInProgress for that.
>> >>> >> Is this OK?
>> >>> >
>> >>> > That can change state at any time. Would that work?
>> >>>
>> >>> Yes. XLogCtl->SharedRecoveryInProgress is set to TRUE only when
>> >>> XLogCtl structure is initialized (i.e., XLOGShmemInit), and it's
>> >>> set to FALSE only at the end of recovery.
>> >>
>> >> You should be using RecoveryInProgress()
>> >
>> > Isn't access to a bool variable atomic?
>>
>> If it's not atomic, I'll add the following comment into CancelBackup():
>>
>>     Don't bother with lock to access XLogCtl->SharedRecoveryInProgress,
>>     because there should be no other processes running when this code
>>     is reached.
>
> Call it via a function. There is no need for postmaster to know the
> innards of xlog.c, which could change in future. Modularity.

In the patch, it's accessed in CancelBackup() which is in xlog.c.
CancelBackup() should call further wrapping function?

Regards,

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