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

On Thu, Sep 21, 2017 at 1:07 AM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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 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.

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.
--
Michael