Re: [HACKERS] Re: [sqlsmith] FailedAssertion("!(XLogCtl->Insert.exclusiveBackup)",File: "xlog.c", Line: 10200)

On Wed, Dec 14, 2016 at 11:10 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Tue, Dec 13, 2016 at 10:24 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Tue, Dec 13, 2016 at 12:49 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>> Thanks for the review.
>
> Thanks for the updated version of the patch!
>
>>> +                    (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>>> +                     errmsg("a backup is already starting")));
>>> +        }
>>> +        if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STOPPING)
>>> +        {
>>> +            WALInsertLockRelease();
>>> +            ereport(ERROR,
>>> +                    (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>>> +                     errmsg("a backup is currently stopping")));
>>>
>>> Isn't it better to use "an exclusive backup" explicitly rather than
>>> "a backup" here?
>>
>> Yes. It would be better to not touch the original in-progress messages
>> though.
>
> On second thought, do users really want to distinguish those three errornous
> states? I'm inclined to merge the checks for those three conditions into one,
> that is,
>
>         if (XLogCtl->Insert.exclusiveBackupState != EXCLUSIVE_BACKUP_IN_NONE)
>         {
>             WALInsertLockRelease();
>             ereport(ERROR,
>                     (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>                      errmsg("a backup is already in progress"),
>
> Also it may be better to handle the similar checks in pg_stop_backup,
> in the same way.

Here is the updated version of the patch that I applied the above "merge" to.

Unfortunately this patch is not applied cleanly to old versions.
So we need to create more patches for back-patch.

Regards,

-- 
Fujii Masao

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers