On Wed, Nov 15, 2017 at 5:39 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Wed, Nov 15, 2017 at 5:21 PM, Masahiko Sawada <sawada.m...@gmail.com> > wrote: >> On Wed, Nov 15, 2017 at 2:38 PM, Michael Paquier >> <michael.paqu...@gmail.com> wrote: >>> On Wed, Nov 15, 2017 at 12:12 PM, Masahiko Sawada <sawada.m...@gmail.com> >>> wrote: >>>> I think we need to check only sessionBackupState and don't need to >>>> check XLogCtl->Insert.exclusiveBackupState in do_pg_abort_backup(). We >>>> can quickly return if sessionBackupState != >>>> SESSION_BACKUP_NON_EXCLUSIVE. In your suggestion, I think we can still >>>> get an assertion failure when pg_stop_backup(false) waiting for >>>> archiving is terminated while concurrent an exclusive backup is in >>>> progress. >>> >>> I have just gone through the thread once again, and noticed that it is >>> actually what I suggested upthread: >>> https://www.postgresql.org/message-id/cab7npqtm5cdrr5y7yyfky+pvdz6dws_jkg1kstan5m95gam...@mail.gmail.com >>> But your v2 posted here did not do that so it is incorrect from the start: >>> https://www.postgresql.org/message-id/cad21aoa+isxyl1_zxmnk9xjhyel5h6rxjttovli7fumqfmc...@mail.gmail.com >> >> Sorry, it's my fault. I didn't mean it but I forgot. > > My review was wrong as well :) > >>> We both got a bit confused here. As do_pg_abort_backup() is only used >>> for non-exclusive backups (including those taken through the >>> replication protocol), going through the session lock for checks is >>> fine. Could you update your patch accordingly please? >> >> One question is, since we need to check only the session lock I think >> that the following change is not necessary. Even if calling >> CHECK_FOR_INTERRUPTS after set sessionBackupState = >> SESSION_BACKUP_EXCLUSIVE; we never call do_pg_abort_backup(). Is that >> right? > > Yeah, this one is not strictly necessary for this bug, but it seems to > me that it would be a good idea for robustness wiht interrupts to be > consistent with the stop phase when updating the session lock.
Agreed. Attached the updated patch, please review it. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
fix_do_pg_abort_backup_v5.patch
Description: Binary data
