SEO

Magnus Hagander <magnus@hagander.net> 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 holds?

Oh, yes, "could not" is better. My mistake.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> 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 holds?
>
> Oh, yes, "could not" is better. My mistake.

Ok, thanks. Committed new error messages as such then.

//Magnus

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

"Albe Laurenz" <laurenz.albe@wien.gv.at> 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 issue pg_stop_backup().

Er, I was complaining about the fast-shutdown code path, not the
smart-shutdown one.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

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, I was complaining about the fast-shutdown code path, not the
> smart-shutdown one.

Yes, I got that.
So you suggest that a fast shutdown *first* calls CancelBackup and
then goes into WAIT_BACKUP state, right?

That should work, but isn't it better if backup_label is removed
only if we know we're going to shutdown cleanly?

I think the patch in http://archives.postgresql.org/pgsql...4/msg00458.php
should meet your justified complaints, by calling CancelBackup immediately
before PostmasterExit(0) in PostmasterStateMachine.

Yours,
Laurenz Albe

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

"Albe Laurenz" <laurenz.albe@wien.gv.at> 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 (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

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 shutdown that fails will end up as a crash or immediate
shutdown.


If you think that is is not important to only cancel backup mode
if we are sure that the shutdown will be clean, we might as well
also cancel online backup mode during an immediate shutdown.

In that case, I'd agree that the call to CancelBackup() could be moved
to WAIT_BACKUP (and executed only if it is no smart shutdown).
It would have the advantage of having all backup mode related
code in postmaster.c concentrated in one spot.


Make a suggestion, and I'll implement it that way.

Yours,
Laurenz Albe

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

"Albe Laurenz" <laurenz.albe@wien.gv.at> 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 child is gone.

> 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, as coded a SIGINT delivered to the postmaster after SIGTERM
would fail to do anything at all, when of course it really ought to
force us into fast shutdown. Also, it's not really that hard to
disallow non-superusers from connecting in PM_WAIT_BACKUP state.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

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, as coded a SIGINT delivered to the postmaster after SIGTERM
> would fail to do anything at all, when of course it really ought to
> force us into fast shutdown. Also, it's not really that hard to
> disallow non-superusers from connecting in PM_WAIT_BACKUP state.

Thank you for helping.
You know, this is my first patch for server code. I know that I
still have a lot to learn until I grok how all that works
together.

Yours,
Laurenz Albe

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches