On Thu, Sep 21, 2017 at 2:25 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Thu, Sep 21, 2017 at 1:07 AM, Masahiko Sawada <sawada.m...@gmail.com> > wrote: >> The bug can happen in PostgreSQL 9.1 or higher that non-exclusive >> backup has been introduced, so we should back-patch to the all >> supported versions. > > There is a typo here right? Non-exclusive backups have been introduced > in 9.6. Why would a back-patch further down be needed?
I think the non-exclusive backups infrastructure has been introduced in 9.1 for pg_basebackup. I've not checked yet that it can be reproduced using pg_basebackup in PG9.1 but I thought it could happen as far as I checked the code. > >> I changed do_pg_abort_backup() so that it decrements >> nonExclusiveBackups only if > 0. Feedback is very welcome. > > +- Assert(XLogCtl->Insert.nonExclusiveBackups >= 0); > + if (XLogCtl->Insert.nonExclusiveBackups > 0) > + XLogCtl->Insert.nonExclusiveBackups--; > Hm, no, I don't agree. I think that instead you should just leave > do_pg_abort_backup() immediately if sessionBackupState is set to > SESSION_BACKUP_NONE. This variable is the link between the global > counters and the session stopping the backup so I don't think that we > should touch this assertion of this counter. I think that this method > would be safe as well for backup start as pg_start_backup_callback > takes care of any cleanup. Also because the counters are incremented > before entering in the PG_ENSURE_ERROR_CLEANUP block, and > sessionBackupState is updated just after leaving the block. I think that the assertion failure still can happen if the process aborts after decremented the counter and before setting to SESSION_BACKUP_NONE. Am I missing something? > > About the patch: > +- Assert(XLogCtl->Insert.nonExclusiveBackups >= 0); > There is a typo on this line. > > Adding that to the next CF would be a good idea so as we don't forget > about it. Feel free to add me as reviewer. Thank you! Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
