Re: Improve shutdown during online backup, take 4 - Mailing list pgsql-patches

From Tom Lane
Subject Re: Improve shutdown during online backup, take 4
Date
Msg-id 12407.1208966351@sss.pgh.pa.us
Whole thread Raw
In response to Re: Improve shutdown during online backup, take 4  (Magnus Hagander <magnus@hagander.net>)
Responses Re: Improve shutdown during online backup, take 4  ("Albe Laurenz" <laurenz.albe@wien.gv.at>)
List pgsql-patches
Magnus Hagander <magnus@hagander.net> writes:
> Alvaro Herrera wrote:
>> I think the messages should not have a newline in the middle.

> Are you talking about the one in pg_ctl? We have other messages in
> pg_ctl that already do this, so I figured it was ok there...

I concur that the messages added to pg_ctl are bizarrely formatted.
Why would you put a newline in the middle of a sentence, when you
could equally well emit something like

WARNING: online backup mode is active.
Shutdown will not complete until pg_stop_backup() is called.

While we're on the subject, the messages added to xlog.c do not
follow the style guidelines: in particular, errdetail should be
a complete sentence, and the WARNING is trying to stuff independent
thoughts into one message.  I'd probably do

errmsg("online backup mode cancelled"),
errdetail("\"%s\" was renamed to \"%s\".", ...

errmsg("online backup mode was not cancelled"),
errdetail("Failed to rename \"%s\" to \"%s\": %m", ...

Lastly, the changes to pmdie's SIGINT handling seem quite bogus.
Don't you need to transition into WAIT_BACKUP rather than WAIT_BACKENDS
state in that case too?  Shouldn't you do CancelBackup *before*
PostmasterStateMachine?  The thing screams of race conditions.

            regards, tom lane

pgsql-patches by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: Snapshot management, final
Next
From: "Joshua D. Drake"
Date:
Subject: Patch to change psql default banner