pgsql: Basic Recovery Control functions for use in Hot Standby. Pause,
Basic Recovery Control functions for use in Hot Standby. Pause, Resume,
Status check functions only. Also, new recovery.conf parameter to
pause_at_recovery_target, default on.
Simon Riggs, reviewed by Fujii Masao
Branch
------
master
Details
-------
http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=8c6e3adbf792c2bba448e88cbf2c8e03fb802e73
Modified Files
--------------
doc/src/sgml/func.sgml | 58 +++++++++++++++
doc/src/sgml/recovery-config.sgml | 25 +++++++
src/backend/access/transam/xlog.c | 137 ++++++++++++++++++++++++++++++++++++
src/include/access/xlog_internal.h | 3 +
src/include/catalog/pg_proc.h | 7 ++
5 files changed, 230 insertions(+), 0 deletions(-)
On Wed, Feb 9, 2011 at 3:30 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
Basic Recovery Control functions for use in Hot Standby. Pause, Resume,
Status check functions only. Also, new recovery.conf parameter to
pause_at_recovery_target, default on.
Why did you change the default to on? This would surprise people who are
used to PITR.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Tue, Feb 8, 2011 at 1:30 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Basic Recovery Control functions for use in Hot Standby. Pause, Resume,
Status check functions only. Also, new recovery.conf parameter to
pause_at_recovery_target, default on.
I guess that new parameter should also be in recovery.conf.sample, right?
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte y capacitación de PostgreSQL
On Wed, Feb 9, 2011 at 1:00 PM, Jaime Casanova <jaime@2ndquadrant.com> wrote:
On Tue, Feb 8, 2011 at 1:30 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Basic Recovery Control functions for use in Hot Standby. Pause, Resume,
Status check functions only. Also, new recovery.conf parameter to
pause_at_recovery_target, default on.I guess that new parameter should also be in recovery.conf.sample, right?
Right.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Wed, 2011-02-09 at 12:50 +0900, Fujii Masao wrote:
On Wed, Feb 9, 2011 at 3:30 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
Basic Recovery Control functions for use in Hot Standby. Pause, Resume,
Status check functions only. Also, new recovery.conf parameter to
pause_at_recovery_target, default on.Why did you change the default to on? This would surprise people who are
used to PITR.
You pointed out that the code did not match the documented default. So I
made them match according to the docs.
Making it pause at target by default is more natural behaviour, even if
it is a change of behaviour. It can waste a lot of time if it leaves
recovery at the wrong point so I don't see the change as a bad one? Only
PITR is affected, not replication or standalone operation.
--
Simon Riggs http://www.2ndQuadrant.com/books/
PostgreSQL Development, 24x7 Support, Training and Services
On Wed, Feb 9, 2011 at 2:02 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Why did you change the default to on? This would surprise people who are
used to PITR.You pointed out that the code did not match the documented default. So I
made them match according to the docs.
Well, I meant changing the docs rather than the code.
Making it pause at target by default is more natural behaviour, even if
it is a change of behaviour. It can waste a lot of time if it leaves
recovery at the wrong point so I don't see the change as a bad one? Only
PITR is affected, not replication or standalone operation.
I agree that new option is useful to reduce the waste of time as you described.
But I'm still not sure that the change of default behavior is better.
Because I can
easily imagine the case where a user feels confused about the pause of PITR
when he starts PITR as he did in previous version. It would take some time for
him to learn what to do in that situation (i.e., execute pg_xlog_replay_resume).
On the second thought, I think it's useful to emit the NOTICE message when
recovery reaches the pause point, as follows.
NOTICE: Recovery will not complete until pg_xlog_replay_resume() is called.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Wed, Feb 9, 2011 at 07:22, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Feb 9, 2011 at 2:02 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Why did you change the default to on? This would surprise people who are
used to PITR.You pointed out that the code did not match the documented default. So I
made them match according to the docs.Well, I meant changing the docs rather than the code.
Making it pause at target by default is more natural behaviour, even if
it is a change of behaviour. It can waste a lot of time if it leaves
recovery at the wrong point so I don't see the change as a bad one? Only
PITR is affected, not replication or standalone operation.I agree that new option is useful to reduce the waste of time as you described.
But I'm still not sure that the change of default behavior is better.
FWIW, I like the change of behavior. We obviously need to put it
prominently in the release notes, but it makes life significantly
easier.
Because I can
easily imagine the case where a user feels confused about the pause of PITR
when he starts PITR as he did in previous version. It would take some time for
him to learn what to do in that situation (i.e., execute pg_xlog_replay_resume).On the second thought, I think it's useful to emit the NOTICE message when
recovery reaches the pause point, as follows.NOTICE: Recovery will not complete until pg_xlog_replay_resume() is called.
Combined with this, yes.
I was also worried about the non-hot-standby case, but I see that the
patch makes sure you can't enable pause when not in hot standby mode.
Which in itself might be surprising - perhaps we need a NOTICE for
when that happens as well?
And it definitely needs to be mentioned in the docs for
pause_at_recovery_target that it only works in hot standby.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
On Wed, 2011-02-09 at 15:22 +0900, Fujii Masao wrote:
On the second thought, I think it's useful to emit the NOTICE message when
recovery reaches the pause point, as follows.NOTICE: Recovery will not complete until pg_xlog_replay_resume() is called.
I'm OK with adding a message, but NOTICE is the wrong level.
My proposal is this message
LOG: Recovery has paused. Execute pg_xlog_replay_resume() to continue.
--
Simon Riggs http://www.2ndQuadrant.com/books/
PostgreSQL Development, 24x7 Support, Training and Services
On Tue, Feb 15, 2011 at 9:21 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On Wed, 2011-02-09 at 15:22 +0900, Fujii Masao wrote:
On the second thought, I think it's useful to emit the NOTICE message when
recovery reaches the pause point, as follows.NOTICE: Recovery will not complete until pg_xlog_replay_resume() is called.
I'm OK with adding a message, but NOTICE is the wrong level.
My proposal is this message
LOG: Recovery has paused. Execute pg_xlog_replay_resume() to continue.
I agree to use LOG level. But "Execute pg_xlog_replay_resume() to continue."
looks like a hint rather than log. So what about:
LOG: Recovery has paused.
HINT: Execute pg_xlog_replay_resume() to continue.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Wed, Feb 9, 2011 at 5:12 PM, Magnus Hagander <magnus@hagander.net> wrote:
I was also worried about the non-hot-standby case, but I see that the
patch makes sure you can't enable pause when not in hot standby mode.
Which in itself might be surprising - perhaps we need a NOTICE for
when that happens as well?And it definitely needs to be mentioned in the docs for
pause_at_recovery_target that it only works in hot standby.
+1 with all the above.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Tue, Feb 8, 2011 at 11:00 PM, Jaime Casanova <jaime@2ndquadrant.com> wrote:
On Tue, Feb 8, 2011 at 1:30 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Basic Recovery Control functions for use in Hot Standby. Pause, Resume,
Status check functions only. Also, new recovery.conf parameter to
pause_at_recovery_target, default on.I guess that new parameter should also be in recovery.conf.sample, right?
This one is still not in recovery.conf.sample
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte y capacitación de PostgreSQL
On Tue, Mar 8, 2011 at 9:20 AM, Jaime Casanova <jaime@2ndquadrant.com> wrote:
On Tue, Feb 8, 2011 at 11:00 PM, Jaime Casanova <jaime@2ndquadrant.com> wrote:
On Tue, Feb 8, 2011 at 1:30 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Basic Recovery Control functions for use in Hot Standby. Pause, Resume,
Status check functions only. Also, new recovery.conf parameter to
pause_at_recovery_target, default on.I guess that new parameter should also be in recovery.conf.sample, right?
This one is still not in recovery.conf.sample
What about the attached patch?
And it definitely needs to be mentioned in the docs for
pause_at_recovery_target that it only works in hot standby.
Done.
On the second thought, I think it's useful to emit the NOTICE message when
recovery reaches the pause point, as follows.NOTICE: Recovery will not complete until pg_xlog_replay_resume() is called.
I'm OK with adding a message, but NOTICE is the wrong level.
My proposal is this message
LOG: Recovery has paused. Execute pg_xlog_replay_resume() to continue.
Done.
And, I found that <indexterm> tag has not been added for recovery
control functions
yet. So I added that into func.sgml.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
change_recovery_conf_sample_v1.patchapplication/octet-stream; name=change_recovery_conf_sample_v1.patchDownload+24-0
Fujii Masao <masao.fujii@gmail.com> writes:
What about the attached patch?
+ ereport(LOG, + (errmsg("Recovery has paused. Execute pg_xlog_replay_resume() to continue.")));
This isn't even close to following our message style guidelines ...
regards, tom lane
On Wed, Feb 9, 2011 at 5:12 PM, Magnus Hagander <magnus@hagander.net> wrote:
I was also worried about the non-hot-standby case, but I see that the
patch makes sure you can't enable pause when not in hot standby mode.
Which in itself might be surprising - perhaps we need a NOTICE for
when that happens as well?
I didn't include this fix in the patch because I prefer FATAL to
NOTICE for that.
NOTICE doesn't stop recovery. So we might be unable to notice such a NOTICE
message and stop the recovery before it's too late, i.e., the recovery has
completed at the undesirable point. So I think that emitting FATAL is safer.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Tue, Mar 8, 2011 at 12:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
What about the attached patch?
+ ereport(LOG, + (errmsg("Recovery has paused. Execute pg_xlog_replay_resume() to continue.")));This isn't even close to following our message style guidelines ...
Thanks! I lower-cased the leading character of the first sentence, and treated
the second as HINT. I attached the updated patch.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
change_recovery_conf_sample_v2.patchapplication/octet-stream; name=change_recovery_conf_sample_v2.patchDownload+25-0
On Tue, Mar 8, 2011 at 12:04 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Feb 9, 2011 at 5:12 PM, Magnus Hagander <magnus@hagander.net> wrote:
I was also worried about the non-hot-standby case, but I see that the
patch makes sure you can't enable pause when not in hot standby mode.
Which in itself might be surprising - perhaps we need a NOTICE for
when that happens as well?I didn't include this fix in the patch because I prefer FATAL to
NOTICE for that.
NOTICE doesn't stop recovery. So we might be unable to notice such a NOTICE
message and stop the recovery before it's too late, i.e., the recovery has
completed at the undesirable point. So I think that emitting FATAL is safer.
I included this fix in the patch, which emits FATAL if pause_at_recovery_target
is enabled while hot standby is disabled and the recovery target is set.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
change_recovery_conf_sample_v3.patchapplication/octet-stream; name=change_recovery_conf_sample_v3.patchDownload+31-0
On Tue, Mar 8, 2011 at 5:48 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, Mar 8, 2011 at 12:04 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Feb 9, 2011 at 5:12 PM, Magnus Hagander <magnus@hagander.net> wrote:
I was also worried about the non-hot-standby case, but I see that the
patch makes sure you can't enable pause when not in hot standby mode.
Which in itself might be surprising - perhaps we need a NOTICE for
when that happens as well?I didn't include this fix in the patch because I prefer FATAL to
NOTICE for that.
NOTICE doesn't stop recovery. So we might be unable to notice such a NOTICE
message and stop the recovery before it's too late, i.e., the recovery has
completed at the undesirable point. So I think that emitting FATAL is safer.I included this fix in the patch, which emits FATAL if pause_at_recovery_target
is enabled while hot standby is disabled and the recovery target is set.
Eh, this is problematic, because you can't claim in the documentation
(and the comments in recovery.conf.sample) that the parameter has no
effect when Hot Standby is not enabled, and then at the same time make
that combination a FATAL error. I don't have a strong opinion on
whether to change the docs or make it not FATAL, but the two have to
match.
Committing the rest.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Fri, Mar 11, 2011 at 4:36 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Mar 8, 2011 at 5:48 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, Mar 8, 2011 at 12:04 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Feb 9, 2011 at 5:12 PM, Magnus Hagander <magnus@hagander.net> wrote:
I was also worried about the non-hot-standby case, but I see that the
patch makes sure you can't enable pause when not in hot standby mode.
Which in itself might be surprising - perhaps we need a NOTICE for
when that happens as well?I didn't include this fix in the patch because I prefer FATAL to
NOTICE for that.
NOTICE doesn't stop recovery. So we might be unable to notice such a NOTICE
message and stop the recovery before it's too late, i.e., the recovery has
completed at the undesirable point. So I think that emitting FATAL is safer.I included this fix in the patch, which emits FATAL if pause_at_recovery_target
is enabled while hot standby is disabled and the recovery target is set.Eh, this is problematic, because you can't claim in the documentation
(and the comments in recovery.conf.sample) that the parameter has no
effect when Hot Standby is not enabled, and then at the same time make
that combination a FATAL error. I don't have a strong opinion on
whether to change the docs or make it not FATAL, but the two have to
match.
Yeah, since I like the former, I changed the wordings in the doc and
recovery.conf.sample. What about the attached patch?
Committing the rest.
Thanks!
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
change_recovery_conf_sample_v4.patchapplication/octet-stream; name=change_recovery_conf_sample_v4.patchDownload+20-0
Fujii Masao <masao.fujii@gmail.com> writes:
Yeah, since I like the former, I changed the wordings in the doc and
recovery.conf.sample. What about the attached patch?
Please stop plastering the code with elog(FATAL) calls. Those are
hardly ever appropriate. In contexts where it might be reasonable
to do that, the error handler will treat ERROR like FATAL anyway.
regards, tom lane
On Fri, Mar 11, 2011 at 9:31 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fujii Masao <masao.fujii@gmail.com> writes:
Yeah, since I like the former, I changed the wordings in the doc and
recovery.conf.sample. What about the attached patch?Please stop plastering the code with elog(FATAL) calls. Those are
hardly ever appropriate. In contexts where it might be reasonable
to do that, the error handler will treat ERROR like FATAL anyway.
Another problem here is that we are defaulting to hot_standby=off and
pause_at_recovery_target=on. So AIUI, with this patch, if someone
sets a recovery target without making any other changes to the
configuration, their database won't start up. That seems poor.
Even without the FATAL error, this whole pause_at_recovery_target
thing is a little weird. If someone sets a recovery target without
making any other configuration changes, and Hot Standby is not
enabled, then we will enter normal running, but if Hot Standby *is*
enabled, then we'll replay to that point and pause recovery. That
seems a bit confusing.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
