Stefan's bug (was: max_standby_delay considered harmful)
in [PgSql]
|
From: Robert Haas on 17 May 2010 07:02 On Mon, May 17, 2010 at 3:38 AM, Fujii Masao <masao.fujii(a)gmail.com> wrote: > On Mon, May 17, 2010 at 10:25 AM, Robert Haas <robertmhaas(a)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. Can you explain in a little more detail how this can cause a problem? I'm not very familiar with how the backup label is used. Also, why is this different in PM_STARTUP than in PM_RECOVERY? PM_RECOVERY doesn't guarantee that we've reached consistency. > (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? Hmm, I'm not sure whether that's a good idea or not. Perhaps we should discuss for 9.1? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Fujii Masao on 17 May 2010 22:40 On Mon, May 17, 2010 at 10:20 PM, Robert Haas <robertmhaas(a)gmail.com> wrote: > OK, I think I understand now. But, the SIGTERM sent by the postmaster > doesn't kill the recovery process unconditionally. It will invoke > StartupProcShutdownHandler(), which will set set shutdown_requested = > true. That gets checked by RestoreArchivedFile() and > HandleStartupProcInterrupts(), and I think that neither of those can > get invoked until after the control file has been updated. Do you see > a way it can happen? Yeah, the way is: StartupXLOG() --> ReadCheckpointRecord() --> ReadRecord() --> XLogPageRead() --> XLogFileReadAnyTLI() --> XLogFileRead() --> RestoreArchivedFile() ReadCheckpointRecord() is called before pg_control is updated. ISTM that walreceiver might be invoked even after shutdown is requested. We should prevent the postmaster from starting up walreceiver if Shutdown > NoShutdown? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Fujii Masao on 17 May 2010 23:02 On Mon, May 17, 2010 at 9:01 PM, Simon Riggs <simon(a)2ndquadrant.com> wrote: >> (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. ISTM that we can use XLogCtl->SharedRecoveryInProgress for that. Is this OK? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Simon Riggs on 18 May 2010 01:25 On Tue, 2010-05-18 at 11:40 +0900, Fujii Masao wrote: > ISTM that walreceiver might be invoked even after shutdown is > requested. We should prevent the postmaster from starting up > walreceiver if Shutdown > NoShutdown? +1 -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Simon Riggs on 18 May 2010 01:26
On Tue, 2010-05-18 at 12:02 +0900, Fujii Masao wrote: > On Mon, May 17, 2010 at 9:01 PM, Simon Riggs <simon(a)2ndquadrant.com> wrote: > >> (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. > > ISTM that we can use XLogCtl->SharedRecoveryInProgress for that. > Is this OK? That can change state at any time. Would that work? -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |