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
> exa
"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 t
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 shutdow
"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
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 issue pg_stop_backup().
>
> Er
"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 super
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.
>
"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?
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
> > r
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 i
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 comple
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 calle
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
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 determi
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 new
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 wond
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 use
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 use
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.
You
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 no
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, s
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
"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
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_
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 "b
[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 nobod
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.
>
> If we add a function called something like BackupI
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
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 to
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 tha
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, t
32 matches
Mail list logo
