Re: [HACKERS] Assertion failure when the non-exclusive pg_stop_backup aborted.

On Tue, Nov 21, 2017 at 8:03 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Tue, Nov 21, 2017 at 3:11 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> On Mon, Nov 20, 2017 at 4:12 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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