On Tue, Nov 21, 2017 at 8:03 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Nov 21, 2017 at 3:11 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
>> On Mon, Nov 20, 2017 at 4:12 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>>> I agree with your approach. It makes sense to me.
>>>
>>> Attached updated patch. Please review it.
>>
>> Thanks for updating the patch! The patch basically looks good to me.
>
> I am not seeing problems either. The start and stop logic of base
> backups is what I would expect they should.
Thank you for reviewing.
>> + /*
>> + * Clean up session-level lock. To avoid calling CHECK_FOR_INTERRUPTS by
>> + * LWLockReleaseClearVar before changing the backup state we change it
>> + * while holding the WAL insert lock.
>> + */
>>
>> I think that you should comment *why* we need to avoid calling
>> CHECK_FOR_INTERRUPTS before changing the backup state, here.
>
> You could just add "as this allows to keep backup counters kept in
> shared memory consistent with the state of the session starting or
> stopping a backup.".
Thank you for the suggestion, Michael-san. Attached updated patch.
Please review it.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center