Stefan's bug (was: max_standby_delay considered harmful)
in [PgSql]
|
From: Robert Haas on 24 May 2010 10:40 On Mon, May 24, 2010 at 10:35 AM, Fujii Masao <masao.fujii(a)gmail.com> wrote: > On Mon, May 24, 2010 at 10:26 PM, Robert Haas <robertmhaas(a)gmail.com> wrote: >> This looks pretty reasonable to me, but I guess I feel like it would >> be better to drive the CancelBackup() decision off of whether we've >> ever reached PM_RUN rather than consulting XLogCtl. It just feels >> cleaner to me to drive all of the postmaster decisions off of the same >> signalling mechanism rather than having a separate one (that only >> works because it's used very late in shutdown when we theoretically >> don't need a lock) just for this one case. > > Okay, how about the attached patch? It uses the postmaster-local flag > "ReachedEndOfRecovery" (better name?) instead of XLogCtl one. Looks good to me. I will test and apply. -- 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: Tom Lane on 24 May 2010 10:48 Fujii Masao <masao.fujii(a)gmail.com> writes: > Okay, how about the attached patch? It uses the postmaster-local flag > "ReachedEndOfRecovery" (better name?) instead of XLogCtl one. ReachedNormalRunning, perhaps? regards, tom lane -- 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 25 May 2010 06:19 On Tue, May 18, 2010 at 3:09 PM, Fujii Masao <masao.fujii(a)gmail.com> wrote: >>> (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? > > Though I'm not sure if this should be fixed for 9.0, I attached the > patch (move_bkp_cancel_warning_v1.patch). This patch is worth applying for 9.0? If not, I'll add it into the next CF for 9.1. 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: Robert Haas on 26 May 2010 08:40 On Mon, May 24, 2010 at 10:35 AM, Fujii Masao <masao.fujii(a)gmail.com> wrote: > On Mon, May 24, 2010 at 10:26 PM, Robert Haas <robertmhaas(a)gmail.com> wrote: >> This looks pretty reasonable to me, but I guess I feel like it would >> be better to drive the CancelBackup() decision off of whether we've >> ever reached PM_RUN rather than consulting XLogCtl. It just feels >> cleaner to me to drive all of the postmaster decisions off of the same >> signalling mechanism rather than having a separate one (that only >> works because it's used very late in shutdown when we theoretically >> don't need a lock) just for this one case. > > Okay, how about the attached patch? It uses the postmaster-local flag > "ReachedEndOfRecovery" (better name?) instead of XLogCtl one. I've committed part of this patch, with the naming change that Tom suggested. The parts I haven't committed are: 1. I don't see why we need to reset ReachedEndOfRecovery starting over from PM_NO_CHILDREN. It seems to me that once we reach PM_RUN, we CAN'T go back to needing the backup label file, even if we have a subsequent backend crash. If I'm wrong, please let me know why and I'll go put this back (with an appropriate comment). 2. The changes to avoid launching WALReceiver except during certain PM_* states. It seems fairly sensible, but what is the case where adding this logic prevents a problem? -- 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: Robert Haas on 26 May 2010 08:54
On Tue, May 25, 2010 at 6:19 AM, Fujii Masao <masao.fujii(a)gmail.com> wrote: > On Tue, May 18, 2010 at 3:09 PM, Fujii Masao <masao.fujii(a)gmail.com> wrote: >>>> (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? >> >> Though I'm not sure if this should be fixed for 9.0, I attached the >> patch (move_bkp_cancel_warning_v1.patch). > > This patch is worth applying for 9.0? If not, I'll add it into > the next CF for 9.1. I'm not convinced that this is a good idea, because ISTM it will make the error message to be less likely to be seen by the person running pg_ctl. In any case, it's a behavior change, so I think that means it's a no-go for 9.0. In terms of 9.1, it might make sense to log something to both places. But maybe we shouldn't just do it once - maybe it should happen every 30 s or so until we actually manage to shut down, with a list of what's still blocking shutdown a la errdetail_busy_db. -- 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 |