Tom Lane wrote:
I've attached a patch that works for me. I hope I got it right.
Applied with additional cleanup. You hadn't thought very carefully
about additional state transitions that would have to be introduced
into the postmaster state machine to support a new state --- for
example,
Albe Laurenz [EMAIL PROTECTED] writes:
Tom Lane wrote:
If you actually want the behavior you propose, then the only correct way
to implement it is to embed it into the state machine logic, ie, do the
CancelBackup inside PostmasterStateMachine in some state transition
taken after the last
Albe Laurenz [EMAIL PROTECTED] writes:
That should work, but isn't it better if backup_label is removed
only if we know we're going to shutdown cleanly?
Why? That seems like an entirely arbitrary specification.
regards, tom lane
--
Sent via pgsql-patches mailing list
Tom Lane wrote:
Why? That seems like an entirely arbitrary specification.
My resoning is that I think of smart/fast/immediate shutdown
as three different things.
For an immediate shutdown/crash thought it was best not to modify
anything to facilitate an analysis of the problem.
A fast
Tom Lane wrote:
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.
Albe Laurenz wrote:
Tom Lane wrote:
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
Tom Lane wrote:
Magnus Hagander [EMAIL PROTECTED] writes:
Hmm. I've preivously been told not to use Failed to but instead
use Could not... Didn't notice that Tom used the other one in his
suggestion.
Tom (or someone else) - can you comment on if I misunderstood that
recommendation
Albe Laurenz [EMAIL PROTECTED] writes:
Tom Lane wrote:
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
Magnus Hagander [EMAIL PROTECTED] writes:
Hmm. I've preivously been told not to use Failed to but instead use
Could not... Didn't notice that Tom used the other one in his
suggestion.
Tom (or someone else) - can you comment on if I misunderstood that
recommendation earlier, or if it still
Tom Lane wrote:
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.
I suspect
Albe Laurenz [EMAIL PROTECTED] writes:
Tom Lane wrote:
Why not? It'll fall out of the state again immediately in
PostmasterStateMachine, no, if we do a CancelBackup here?
We cannot call CancelBackup there because that's exactly the state
in which a smart shutdown waits for a superuser to
Magnus Hagander wrote:
This doesn't look like our normal coding standards, and should probably
be changed:
+ if (0 != stat(BACKUP_LABEL_FILE, stat_buf))
(there's a number of similar places)
Lacking guidelines, I now tried to copy how stat(2) is used in other parts
of the code.
Yours,
Albe Laurenz wrote:
Magnus Hagander wrote:
This doesn't look like our normal coding standards, and should
probably be changed:
+ if (0 != stat(BACKUP_LABEL_FILE, stat_buf))
(there's a number of similar places)
I see. Lacking guidelines, I now copied how stat(2) is used in
other
Albe Laurenz wrote:
Magnus Hagander wrote:
This doesn't look like our normal coding standards, and should
probably be changed:
+ if (0 != stat(BACKUP_LABEL_FILE, stat_buf))
(there's a number of similar places)
I see. Lacking guidelines, I now copied how stat(2) is used in
other
Magnus Hagander wrote:
Applied with some minor changes to the error messages to make them
closer to our guidelines. (With my track record, it's not entirely
unlikely that someone will fix them further though :-P)
I think the messages should not have a newline in the middle.
Also, I am
Alvaro Herrera wrote:
Magnus Hagander wrote:
Applied with some minor changes to the error messages to make them
closer to our guidelines. (With my track record, it's not entirely
unlikely that someone will fix them further though :-P)
I think the messages should not have a newline in
Alvaro Herrera wrote:
I think the messages should not have a newline in the middle.
Also, I am wondering if in PM_WAIT_BACKUP mode we should accept new
connections from superusers only.
I spent some thought on that.
You'd need to wait until the user is authenticated before you can determine
Magnus Hagander [EMAIL PROTECTED] 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
Simon Riggs wrote:
Patch applies, and works as described. Looks good for final apply.
Few minor thoughts:
* Text in pg_ctl should be WARNING, not Warning.
* CancelBackup() API looks strange, not sure why
* Need to mention that CancelBackup() is not the right way to end a
backup, so that
Albe Laurenz wrote:
Simon Riggs wrote:
Patch applies, and works as described. Looks good for final apply.
Few minor thoughts:
* Text in pg_ctl should be WARNING, not Warning.
* CancelBackup() API looks strange, not sure why
* Need to mention that CancelBackup() is not the right
On Wed, 2008-04-09 at 14:58 +0200, Albe Laurenz wrote:
This patch replaces the previous version
http://archives.postgresql.org/pgsql-patches/2008-04/msg4.php
As suggested by Simon Riggs in
http://archives.postgresql.org/pgsql-patches/2008-04/msg00013.php ,
a smart shutdown will now wait
On Tue, 2008-04-08 at 09:16 +0200, Albe Laurenz wrote:
Heikki Linnakangas wrote:
Albe Laurenz wrote:
Moreover, if Shutdown == SmartShutdown, new connections won't be accepted,
and nobody can connect and call pg_stop_backup().
So even if I'd add a check for
(pmState == PM_WAIT_BACKENDS)
Simon Riggs [EMAIL PROTECTED] writes:
Personally, I think smart shutdown could be even smarter. It should
kick off unwanted sessions, such as an idle pgAdmin session - maybe a
rule like anything that has been idle for 30 seconds.
That's not a bad idea in itself but I don't think it's
This patch replaces the previous version
http://archives.postgresql.org/pgsql-patches/2008-04/msg4.php
As suggested by Simon Riggs in
http://archives.postgresql.org/pgsql-patches/2008-04/msg00013.php ,
a smart shutdown will now wait for online backup mode to finish.
The functions that touch
[what should happen if a smart shutdown request is received during online
backup mode?
I'll cc: the hackers list, maybe others have something to say to this]
Heikki Linnakangas wrote:
Albe Laurenz wrote:
Moreover, if Shutdown == SmartShutdown, new connections won't be accepted,
and nobody
Simon Riggs wrote:
Few comments:
* smart shutdown waits for sessions to complete, yet this just ignores
smart shutdowns which is something a little different. I think we
should wait for the backup to complete and then shutdown.
If we add a function called something like BackupInProgress()
Albe Laurenz wrote:
Moreover, if Shutdown == SmartShutdown, new connections won't be accepted,
and nobody can connect and call pg_stop_backup().
So even if I'd add a check for
(pmState == PM_WAIT_BACKENDS) !BackupInProgress() somewhere in the
ServerLoop(), it wouldn't do much good, because the
Simon Riggs wrote:
Few comments:
* smart shutdown waits for sessions to complete, yet this just ignores
smart shutdowns which is something a little different. I think we
should wait for the backup to complete and then shutdown.
That would be more consistent, I agree.
I'll undo my changes
This follows up on the discussion in
http://archives.postgresql.org/pgsql-hackers/2008-03/msg01033.php
- pg_ctl will refuse a smart shutdown during online backup.
- The postmaster will also refuse to shutdown in smart mode
in that case and log a message to that effect.
- In fast shutdown mode,
On Tue, 2008-04-01 at 15:34 +0200, Albe Laurenz wrote:
This follows up on the discussion in
http://archives.postgresql.org/pgsql-hackers/2008-03/msg01033.php
- pg_ctl will refuse a smart shutdown during online backup.
- The postmaster will also refuse to shutdown in smart mode
in that
On Tue, 2008-04-01 at 17:42 +0100, Simon Riggs wrote:
Few comments:
* smart shutdown waits for sessions to complete, yet this just ignores
smart shutdowns which is something a little different. I think we
should wait for the backup to complete and then shutdown.
* The #defines at top of
31 matches
Mail list logo
