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

Поиск
Список
Период
Сортировка
От Masahiko Sawada
Тема Re: [HACKERS] Assertion failure when the non-exclusive pg_stop_backup aborted.
Дата
Msg-id CAD21AoA-RVgTe7Hd7K_W=8OpU-v7FJGh_Kz4GDARUWW9-C+=tg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [HACKERS] Assertion failure when the non-exclusive pg_stop_backup aborted.  (Michael Paquier <michael.paquier@gmail.com>)
Ответы Re: [HACKERS] Assertion failure when the non-exclusive pg_stop_backup aborted.  (Michael Paquier <michael.paquier@gmail.com>)
Список pgsql-hackers
On Thu, Sep 21, 2017 at 2:25 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Sep 21, 2017 at 1:07 AM, Masahiko Sawada <sawada.mshk@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

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Emre Hasegeli
Дата:
Сообщение: Re: [HACKERS] close_ps, NULLs, and DirectFunctionCall
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: [HACKERS] coverage analysis improvements