Re: Stefan's bug (was: max_standby_delay considered harmful)

On Mon, 2010-05-17 at 16:38 +0900, Fujii Masao wrote:
> On Mon, May 17, 2010 at 10:25 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > Therefore I think
> > Fujii Masao's original idea was the best, but I have what I believe is
> > an equivalent but simpler implementation, which is attached.
>
> Seems good.
>
> I found another two problems related to shutdown in PM_STARTUP state:
>
> (1)
> Smart or fast shutdown requested in PM_STARTUP state always removes
> the backup_label file if it exists. But it might be still required
> for subsequent recovery. I changed your patch so that additionally
> the postmaster skips deleting the backup_label in that case.

Don't like the name NeedBackupLabel seems too specific. That really
corresponds to "we were in recovery". We should have a couple of
super-states that correspond to am in recovery/am not in recovery so we
can drive it from that.

> (2)
> pg_ctl -ms stop emits the following warning whenever there is the
> backup_label file in $PGDATA.
>
> WARNING: online backup mode is active
> Shutdown will not complete until pg_stop_backup() is called.
>
> This warning doesn't fit in with the shutdown during recovery case.
> Since smart shutdown might be requested by other than pg_ctl, the
> warning should be emitted in server side rather than client, I think.
> How about moving the warning to the server side?

+1

> > Thoughts? Should we try to fix this in 8.4 also, or just in HEAD?
> > 8.3 and 8.2 never handle a smart shutdown prior to entering normal
> > running, and while that seems pretty useless, doing something
> > different would be a behavior change, so that seems like a
> > non-starter. 8.4 has the same behavior as HEAD, though it's not
> > documented in the release notes, so it's not clear how intentional the
> > change was.
>
> In 8.4, smart shutdown during recovery waits until the startup process
> has exited. So the backporting to 8.4 doesn't improve any situation,
> I think.

We shouldn't be discussing backporting a behaviour change.

--
Simon Riggs www.2ndQuadrant.com