On Tue, Nov 21, 2017 at 8:03 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Tue, Nov 21, 2017 at 3:11 AM, Fujii Masao <masao.fu...@gmail.com> wrote: >> On Mon, Nov 20, 2017 at 4:12 PM, Masahiko Sawada <sawada.m...@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
fix_do_pg_abort_backup_v8.patch
Description: Binary data