On Wed, Dec 14, 2016 at 11:10 PM, Fujii Masao <masao.fu...@gmail.com> wrote: > On Tue, Dec 13, 2016 at 10:24 PM, Michael Paquier > <michael.paqu...@gmail.com> wrote: >> On Tue, Dec 13, 2016 at 12:49 PM, Fujii Masao <masao.fu...@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
base-backup-crash-v7.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers