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
