Re: Stefan's bug (was: max_standby_delay considered harmful)
Simon Riggs <simon@2ndquadrant.com>
From: Simon Riggs <simon@2ndQuadrant.com>
To: Fujii Masao <masao.fujii@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Stefan Kaltenbrunner <stefan@kaltenbrunner.cc>, Andres Freund <andres@anarazel.de>, Florian Pflug <fgp@phlo.org>, Dimitri Fontaine <dfontaine@hi-media.com>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>, Bruce Momjian <bruce@momjian.us>, Greg Smith <greg@2ndquadrant.com>, Josh Berkus <josh@agliodbs.com>
Date: 2010-05-17T12:01:46Z
Lists: pgsql-hackers
On Mon, 2010-05-17 at 16:38 +0900, Fujii Masao wrote: > On Mon, May 17, 2010 at 10:25 AM, Robert Haas <robertmhaas@gmail.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