From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Magnus Hagander <magnus(at)hagander(dot)net> |
Cc: | Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at>, pgsql-patches(at)postgresql(dot)org |
Subject: | Re: Improve shutdown during online backup, take 4 |
Date: | 2008-04-23 15:59:11 |
Message-ID: | 12407.1208966351@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
Magnus Hagander <magnus(at)hagander(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Joshua D. Drake | 2008-04-23 21:41:20 | Patch to change psql default banner |
Previous Message | Alvaro Herrera | 2008-04-23 15:16:45 | Re: Snapshot management, final |