Add shutdown_at_recovery_target option to recovery.conf
Hi,
I recently wanted several times to have slave server prepared at certain
point in time to reduce the time it takes for it to replay remaining
WALs (say I have pg_basebackup -x on busy db for example).
Currently the way to do it is to have pause_at_recovery_target true
(default) and wait until pg accepts connection and the shut it down. The
issue is that this is ugly, and also there is a chance that somebody
else connects and does bad things (tm) before my process does.
So I wrote simple patch that adds option to shut down the cluster once
recovery_target is reached. The server will still be able to continue
WAL replay if needed later or can be configured to start standalone.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
shutdown_at_recovery_target-v1.patchtext/x-diff; name=shutdown_at_recovery_target-v1.patchDownload
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 0f1ff34..278bbaa 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -309,6 +309,36 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</varlistentry>
</variablelist>
+
+ <varlistentry id="shutdown-at-recovery-target"
+ xreflabel="shutdown_at_recovery_target">
+ <term><varname>shutdown_at_recovery_target</varname> (<type>boolean</type>)
+ <indexterm>
+ <primary><varname>shutdown_at_recovery_target</> recovery parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies whether postgres should shutdown the once the recovery target
+ is reached. The default is false.
+ This is intended to have instance ready at exact replay point desired.
+ The instance will still be able to replay more WAL records.
+ Note that because reconvery.conf will not be removed if this option is
+ set to true, the above means that unless you change your configuration,
+ any subsequent start will end with immediate shutdown.
+ </para>
+ <para>
+ This setting has no effect if <xref linkend="guc-hot-standby"> is not
+ enabled, or if no recovery target is set.
+ </para>
+ <para>
+ Setting this to true will set <link linkend="pause-at-recovery-target">
+ <varname>pause_at_recovery_target</></link> to false.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
</sect1>
<sect1 id="standby-settings">
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 34f2fc0..bbe3987 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -222,6 +222,7 @@ static char *archiveCleanupCommand = NULL;
static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
static bool recoveryTargetInclusive = true;
static bool recoveryPauseAtTarget = true;
+static bool recoveryShutdownAtTarget = false;
static TransactionId recoveryTargetXid;
static TimestampTz recoveryTargetTime;
static char *recoveryTargetName;
@@ -5159,6 +5160,18 @@ readRecoveryCommandFile(void)
(errmsg_internal("pause_at_recovery_target = '%s'",
item->value)));
}
+ else if (strcmp(item->name, "shutdown_at_recovery_target") == 0)
+ {
+ if (!parse_bool(item->value, &recoveryShutdownAtTarget))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" requires a Boolean value", "shutdown_at_recovery_target")));
+ ereport(DEBUG2,
+ (errmsg_internal("shutdown_at_recovery_target = '%s'",
+ item->value)));
+ if (recoveryShutdownAtTarget)
+ recoveryPauseAtTarget = false;
+ }
else if (strcmp(item->name, "recovery_target_timeline") == 0)
{
rtliGiven = true;
@@ -6907,6 +6920,24 @@ StartupXLOG(void)
ereport(LOG,
(errmsg("last completed transaction was at log time %s",
timestamptz_to_str(xtime))));
+
+ /*
+ * Shutdown here when requested. We need to exit this early because
+ * we want to be able to continue the WAL replay when started
+ * the next time.
+ */
+ if (recoveryShutdownAtTarget && reachedStopPoint)
+ {
+ ereport(LOG, (errmsg("shutting down")));
+ /*
+ * Note that we exit with status 2 instead of 0 here to force
+ * postmaster shutdown the whole instance. This produces one
+ * unnecessary log line, but we don't have better way to
+ * shutdown postmaster from within single backend.
+ */
+ proc_exit(2);
+ }
+
InRedo = false;
}
else
On Wed, Sep 10, 2014 at 1:45 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
Hi,
I recently wanted several times to have slave server prepared at certain
point in time to reduce the time it takes for it to replay remaining WALs
(say I have pg_basebackup -x on busy db for example).
In your example, you're thinking to perform the recovery after taking
the backup and stop it at the consistent point (i.e., end of backup) by
using the proposed feature? Then you're expecting that the future recovery
will start from that consistent point and which will reduce the recovery time?
This is true if checkpoint is executed at the end of backup. But there might
be no occurrence of checkpoint during backup. In this case, even future
recovery would need to start from very start of backup. That is, we cannot
reduce the recovery time. So, for your purpose, for example, you might also
need to add new option to pg_basebackup so that checkpoint is executed
at the end of backup if the option is set.
Currently the way to do it is to have pause_at_recovery_target true
(default) and wait until pg accepts connection and the shut it down. The
issue is that this is ugly, and also there is a chance that somebody else
connects and does bad things (tm) before my process does.So I wrote simple patch that adds option to shut down the cluster once
recovery_target is reached. The server will still be able to continue WAL
replay if needed later or can be configured to start standalone.
What about adding something like action_at_recovery_target=pause|shutdown
instead of increasing the number of parameters?
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 10/09/14 13:13, Fujii Masao wrote:
On Wed, Sep 10, 2014 at 1:45 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
Hi,
I recently wanted several times to have slave server prepared at certain
point in time to reduce the time it takes for it to replay remaining WALs
(say I have pg_basebackup -x on busy db for example).In your example, you're thinking to perform the recovery after taking
the backup and stop it at the consistent point (i.e., end of backup) by
using the proposed feature? Then you're expecting that the future recovery
will start from that consistent point and which will reduce the recovery time?This is true if checkpoint is executed at the end of backup. But there might
be no occurrence of checkpoint during backup. In this case, even future
recovery would need to start from very start of backup. That is, we cannot
reduce the recovery time. So, for your purpose, for example, you might also
need to add new option to pg_basebackup so that checkpoint is executed
at the end of backup if the option is set.
For my use-case it does not matter much as I am talking here of huge
volumes where it would normally take hours to replay so being behind one
checkpoint is not too bad, but obviously I am not sure that it's good
enough for project in general. Adding checkpoint for pg_basebackup might
be useful addition, yes.
Also I forgot to write another use-case which making sure that I
actually do have all the WAL present to get to certain point in time
(this one could be done via patch to pg_receivexlog I guess, but I see
advantage in having the changes already applied compared to just having
the wal files).
So I wrote simple patch that adds option to shut down the cluster once
recovery_target is reached. The server will still be able to continue WAL
replay if needed later or can be configured to start standalone.What about adding something like action_at_recovery_target=pause|shutdown
instead of increasing the number of parameters?
That will also increase number of parameters as we can't remove the
current pause one if we want to be backwards compatible. Also there
would have to be something like action_at_recovery_target=none or off or
something since the default is that pause is on and we need to be able
to turn off pause without having to have shutdown on. What more, I am
not sure I see any other actions that could be added in the future as
promote action already works and listen (for RO queries) also already
works independently of this.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 11 September 2014 16:02, Petr Jelinek <petr@2ndquadrant.com> wrote:
What about adding something like action_at_recovery_target=pause|shutdown
instead of increasing the number of parameters?That will also increase number of parameters as we can't remove the current
pause one if we want to be backwards compatible. Also there would have to be
something like action_at_recovery_target=none or off or something since the
default is that pause is on and we need to be able to turn off pause without
having to have shutdown on. What more, I am not sure I see any other actions
that could be added in the future as promote action already works and listen
(for RO queries) also already works independently of this.
I accept your argument, though I have other thoughts.
If someone specifies
shutdown_at_recovery_target = true
pause_at_recovery_target = true
it gets a little hard to work out what to do; we shouldn't allow such
lack of clarity.
In recovery its easy to do this
if (recoveryShutdownAtTarget)
recoveryPauseAtTarget = false;
but it won't be when these become GUCs, so I think Fuji's suggestion
is a good one.
No other comments on patch, other than good idea.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi Petr,
I have spent sometime to review the patch, overall patch looks good, it
applies fine and make check run without issue. If recovery target is
specified and shutdown_at_recovery_target is set to true, it shutdown the
server at specified recovery point. I do have few points to share i.e.
1. It seems that following log message need to be more descriptive about
reason for shutdown i.e.
+ if (recoveryShutdownAtTarget && reachedStopPoint)
+ { + ereport(LOG, (errmsg("shutting down")));
2. As Simon suggesting following recovery settings are not clear i.e.
shutdown_at_recovery_target = true
pause_at_recovery_target = true
It is going to make true both but patch describe as following i.e.
+ Setting this to true will set <link
linkend="pause-at-recovery-target">
+ <varname>pause_at_recovery_target</></link> to false.
3. As it don’t rename reconvery.conf, subsequent attempt (without any
changes in reconvery.conf) to start of server keep showing the following
i.e.
...
LOG: redo starts at 0/1803620
DEBUG: checkpointer updated shared memory configuration values
LOG: consistent recovery state reached at 0/1803658
LOG: restored log file "000000010000000000000002" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000003" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000004" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000005" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000006" from archive
DEBUG: got WAL segment from archive
…
Is that right ?. Thanks.
Regards,
Muhammad Asif Naeem
On Thu, Oct 16, 2014 at 7:24 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
Show quoted text
On 11 September 2014 16:02, Petr Jelinek <petr@2ndquadrant.com> wrote:
What about adding something like
action_at_recovery_target=pause|shutdown
instead of increasing the number of parameters?
That will also increase number of parameters as we can't remove the
current
pause one if we want to be backwards compatible. Also there would have
to be
something like action_at_recovery_target=none or off or something since
the
default is that pause is on and we need to be able to turn off pause
without
having to have shutdown on. What more, I am not sure I see any other
actions
that could be added in the future as promote action already works and
listen
(for RO queries) also already works independently of this.
I accept your argument, though I have other thoughts.
If someone specifies
shutdown_at_recovery_target = true
pause_at_recovery_target = trueit gets a little hard to work out what to do; we shouldn't allow such
lack of clarity.In recovery its easy to do this
if (recoveryShutdownAtTarget)
recoveryPauseAtTarget = false;but it won't be when these become GUCs, so I think Fuji's suggestion
is a good one.No other comments on patch, other than good idea.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 29/10/14 20:27, Asif Naeem wrote:
I have spent sometime to review the patch, overall patch looks good, it
applies fine and make check run without issue. If recovery target is
specified and shutdown_at_recovery_target is set to true, it shutdown
the server at specified recovery point. I do have few points to share i.e.
Thanks!
1. It seems that following log message need to be more descriptive about
reason for shutdown i.e.+ if (recoveryShutdownAtTarget && reachedStopPoint) + { + ereport(LOG, (errmsg("shutting down")));
Agreed, I just wasn't sure on what exactly to writes, I originally had
there "shutting down by user request" or some such but that's misleading.
2. As Simon suggesting following recovery settings are not clear i.e.
shutdown_at_recovery_target = true
pause_at_recovery_target = true
Hmm I completely missed Simon's email, strange. Well other option would
be to throw error if both are set to true - error will have to happen
anyway if action_at_recovery_target is set to shutdown while
pause_at_recovery_target is true (I think we need to keep
pause_at_recovery_target for compatibility).
But considering all of you think something like
action_at_recovery_target is better solution, I will do it that way then.
3. As it don’t rename reconvery.conf, subsequent attempt (without any
changes in reconvery.conf) to start of server keep showing the following
i.e....
LOG: redo starts at 0/1803620
DEBUG: checkpointer updated shared memory configuration values
LOG: consistent recovery state reached at 0/1803658
LOG: restored log file "000000010000000000000002" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000003" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000004" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000005" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000006" from archive
DEBUG: got WAL segment from archive
…
Yes, it will still replay everything since last checkpoint, that's by
design since otherwise we would have to write checkpoint on shutdown and
that would mean the instance can't be used as hot standby anymore and I
consider that an important feature to have.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
Attached is the v2 of the patch with the review comments addressed (see
below).
On 29/10/14 21:08, Petr Jelinek wrote:
On 29/10/14 20:27, Asif Naeem wrote:
1. It seems that following log message need to be more descriptive about
reason for shutdown i.e.+ if (recoveryShutdownAtTarget && reachedStopPoint) + { + ereport(LOG, (errmsg("shutting down")));Agreed, I just wasn't sure on what exactly to writes, I originally had
there "shutting down by user request" or some such but that's misleading.
I changed it to "shutting down at recovery target" hope that's better.
2. As Simon suggesting following recovery settings are not clear i.e.
shutdown_at_recovery_target = true
pause_at_recovery_target = trueHmm I completely missed Simon's email, strange. Well other option would
be to throw error if both are set to true - error will have to happen
anyway if action_at_recovery_target is set to shutdown while
pause_at_recovery_target is true (I think we need to keep
pause_at_recovery_target for compatibility).But considering all of you think something like
action_at_recovery_target is better solution, I will do it that way then.
Done, there is now action_at_recovery_target which can be set to either
pause, continue or shutdown, defaulting to pause (which is same as old
behavior of pause_at_recovery_target defaulting to true).
I also added check that prohibits using both pause_at_recovery_target
and action_at_recovery_target in the same config, mainly to avoid users
shooting themselves in the foot.
3. As it don’t rename reconvery.conf, subsequent attempt (without any
changes in reconvery.conf) to start of server keep showing the following
i.e....
LOG: redo starts at 0/1803620
DEBUG: checkpointer updated shared memory configuration values
LOG: consistent recovery state reached at 0/1803658
LOG: restored log file "000000010000000000000002" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000003" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000004" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000005" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000006" from archive
DEBUG: got WAL segment from archive
…Yes, it will still replay everything since last checkpoint, that's by
design since otherwise we would have to write checkpoint on shutdown and
that would mean the instance can't be used as hot standby anymore and I
consider that an important feature to have.
I added note to the documentation that says this will happen.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
shutdown_at_recovery_target-v2.patchtext/x-diff; name=shutdown_at_recovery_target-v2.patchDownload
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 0f1ff34..fe42394 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -289,12 +289,39 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</term>
<listitem>
<para>
- Specifies whether recovery should pause when the recovery target
- is reached. The default is true.
- This is intended to allow queries to be executed against the
- database to check if this recovery target is the most desirable
- point for recovery. The paused state can be resumed by using
- <function>pg_xlog_replay_resume()</> (See
+ Alias for action_at_recovery_target, <literal>true</> is same as
+ action_at_recovery_target = <literal>pause</> and <literal>false</>
+ is same as action_at_recovery_target = <literal>continue</>.
+ </para>
+ <para>
+ This setting has no effect if <xref linkend="guc-hot-standby"> is not
+ enabled, or if no recovery target is set.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+
+ <varlistentry id="action-at-recovery-target"
+ xreflabel="action_at_recovery_target">
+ <term><varname>action_at_recovery_target</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>action_at_recovery_target</> recovery parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies what action should PostgreSQL do once the recovery target is
+ reached. The default is <literal>pause</>, which means recovery will
+ be paused. <literal>continue</> means there will be no special action
+ taken when recovery target is reached and normal operation will continue.
+ Finally <literal>shutdown</> will stop the PostgreSQL instance at
+ recovery target.
+ </para>
+ The intended use of <literal>pause</> setting is to allow queries to be
+ executed against the database to check if this recovery target is the
+ most desirable point for recovery. The paused state can be resumed by
+ using <function>pg_xlog_replay_resume()</> (See
<xref linkend="functions-recovery-control-table">), which then
causes recovery to end. If this recovery target is not the
desired stopping point, then shutdown the server, change the
@@ -302,6 +329,20 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
continue recovery.
</para>
<para>
+ The <literal>shutdown</> setting is useful to have instance ready at
+ exact replay point desired.
+ The instance will still be able to replay more WAL records (and in fact
+ will have to replay WAL records since last checkpoint upon next time it is
+ started).
+ </para>
+ <para>
+ Note that because <filename>recovery.conf</> will not be renamed when
+ <varname>action_at_recovery_target</> is set to <literal>shutdown</>,
+ any subsequent start will end with immediate shutdown unless the
+ configuration is changed or the <filename>recovery.conf</> is removed
+ manually.
+ </para>
+ <para>
This setting has no effect if <xref linkend="guc-hot-standby"> is not
enabled, or if no recovery target is set.
</para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3c9aeae..974e6c1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -227,7 +227,7 @@ static char *recoveryEndCommand = NULL;
static char *archiveCleanupCommand = NULL;
static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
static bool recoveryTargetInclusive = true;
-static bool recoveryPauseAtTarget = true;
+static RecoveryTargetAction actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
static TransactionId recoveryTargetXid;
static TimestampTz recoveryTargetTime;
static char *recoveryTargetName;
@@ -5068,6 +5068,9 @@ readRecoveryCommandFile(void)
ConfigVariable *item,
*head = NULL,
*tail = NULL;
+ bool recoveryPauseAtTargetSet = false;
+ bool actionAtRecoveryTargetSet = false;
+
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
if (fd == NULL)
@@ -5113,13 +5116,43 @@ readRecoveryCommandFile(void)
}
else if (strcmp(item->name, "pause_at_recovery_target") == 0)
{
+ bool recoveryPauseAtTarget;
+
if (!parse_bool(item->value, &recoveryPauseAtTarget))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires a Boolean value", "pause_at_recovery_target")));
+
ereport(DEBUG2,
(errmsg_internal("pause_at_recovery_target = '%s'",
item->value)));
+
+ actionAtRecoveryTarget = recoveryPauseAtTarget ?
+ RECOVERY_TARGET_ACTION_PAUSE :
+ RECOVERY_TARGET_ACTION_CONTINUE;
+
+ recoveryPauseAtTargetSet = true;
+ }
+ else if (strcmp(item->name, "action_at_recovery_target") == 0)
+ {
+ if (strcmp(item->value, "pause") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
+ else if (strcmp(item->value, "continue") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_CONTINUE;
+ else if (strcmp(item->value, "shutdown") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for recovery parameter \"%s\"",
+ "action_at_recovery_target"),
+ errhint("The allowed values are \"pause\", \"continue\" and \"shutdown\".")));
+
+ ereport(DEBUG2,
+ (errmsg_internal("action_at_recovery_target = '%s'",
+ item->value)));
+
+ actionAtRecoveryTargetSet = true;
}
else if (strcmp(item->name, "recovery_target_timeline") == 0)
{
@@ -5284,6 +5317,18 @@ readRecoveryCommandFile(void)
RECOVERY_COMMAND_FILE)));
}
+ /*
+ * Check for mutually exclusive parameters
+ */
+ if (recoveryPauseAtTargetSet && actionAtRecoveryTargetSet)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot set both \"%s\" and \"%s\" recovery parameters",
+ "pause_at_recovery_target",
+ "action_at_recovery_target"),
+ errhint("The \"pause_at_recovery_target\" is deprecated.")));
+
+
/* Enable fetching from archive recovery area */
ArchiveRecoveryRequested = true;
@@ -6832,7 +6877,8 @@ StartupXLOG(void)
* end of main redo apply loop
*/
- if (recoveryPauseAtTarget && reachedStopPoint)
+ if (actionAtRecoveryTarget == RECOVERY_TARGET_ACTION_PAUSE &&
+ reachedStopPoint)
{
SetRecoveryPause(true);
recoveryPausesHere();
@@ -6853,6 +6899,25 @@ StartupXLOG(void)
ereport(LOG,
(errmsg("last completed transaction was at log time %s",
timestamptz_to_str(xtime))));
+
+ /*
+ * Shutdown here when requested. We need to exit this early because
+ * we want to be able to continue the WAL replay when started
+ * the next time.
+ */
+ if (actionAtRecoveryTarget == RECOVERY_TARGET_ACTION_SHUTDOWN &&
+ reachedStopPoint)
+ {
+ ereport(LOG, (errmsg("shutting down at recovery target")));
+ /*
+ * Note that we exit with status 2 instead of 0 here to force
+ * postmaster shutdown the whole instance. This produces one
+ * unnecessary log line, but we don't have better way to
+ * shutdown postmaster from within single backend.
+ */
+ proc_exit(2);
+ }
+
InRedo = false;
}
else
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 0ae110f..7e48138 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -179,6 +179,16 @@ typedef enum
RECOVERY_TARGET_IMMEDIATE
} RecoveryTargetType;
+/*
+ * Recovery target action.
+ */
+typedef enum
+{
+ RECOVERY_TARGET_ACTION_PAUSE,
+ RECOVERY_TARGET_ACTION_CONTINUE,
+ RECOVERY_TARGET_ACTION_SHUTDOWN,
+} RecoveryTargetAction;
+
extern XLogRecPtr XactLastRecEnd;
extern bool reachedConsistency;
On 31 October 2014 15:18, Petr Jelinek <petr@2ndquadrant.com> wrote:
Attached is the v2 of the patch with the review comments addressed (see
below).
...
Done, there is now action_at_recovery_target which can be set to either
pause, continue or shutdown, defaulting to pause (which is same as old
behavior of pause_at_recovery_target defaulting to true).
One comment only: I think the actions should be called: pause, promote
and shutdown, since "continue" leads immediately to promotion of the
server.
I'm good with this patch otherwise. Barring objections I will commit tomorrow.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 18/11/14 12:57, Simon Riggs wrote:
On 31 October 2014 15:18, Petr Jelinek <petr@2ndquadrant.com> wrote:
Attached is the v2 of the patch with the review comments addressed (see
below)....
Done, there is now action_at_recovery_target which can be set to either
pause, continue or shutdown, defaulting to pause (which is same as old
behavior of pause_at_recovery_target defaulting to true).One comment only: I think the actions should be called: pause, promote
and shutdown, since "continue" leads immediately to promotion of the
server.I'm good with this patch otherwise. Barring objections I will commit tomorrow.
OK, promote works for me as well, I attached patch that changes continue
to promote so you don't have to find and replace everything yourself.
The changed doc wording probably needs to be checked.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
shutdown_at_recovery_target-v3.patchtext/x-diff; name=shutdown_at_recovery_target-v3.patchDownload
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 0f1ff34..fe42394 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -289,12 +289,39 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</term>
<listitem>
<para>
- Specifies whether recovery should pause when the recovery target
- is reached. The default is true.
- This is intended to allow queries to be executed against the
- database to check if this recovery target is the most desirable
- point for recovery. The paused state can be resumed by using
- <function>pg_xlog_replay_resume()</> (See
+ Alias for action_at_recovery_target, <literal>true</> is same as
+ action_at_recovery_target = <literal>pause</> and <literal>false</>
+ is same as action_at_recovery_target = <literal>promote</>.
+ </para>
+ <para>
+ This setting has no effect if <xref linkend="guc-hot-standby"> is not
+ enabled, or if no recovery target is set.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+
+ <varlistentry id="action-at-recovery-target"
+ xreflabel="action_at_recovery_target">
+ <term><varname>action_at_recovery_target</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>action_at_recovery_target</> recovery parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies what action should PostgreSQL do once the recovery target is
+ reached. The default is <literal>pause</>, which means recovery will
+ be paused. <literal>promote</> means recovery process will finish and
+ the server will start to accept connections.
+ Finally <literal>shutdown</> will stop the PostgreSQL instance at
+ recovery target.
+ </para>
+ The intended use of <literal>pause</> setting is to allow queries to be
+ executed against the database to check if this recovery target is the
+ most desirable point for recovery. The paused state can be resumed by
+ using <function>pg_xlog_replay_resume()</> (See
<xref linkend="functions-recovery-control-table">), which then
causes recovery to end. If this recovery target is not the
desired stopping point, then shutdown the server, change the
@@ -302,6 +329,20 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
continue recovery.
</para>
<para>
+ The <literal>shutdown</> setting is useful to have instance ready at
+ exact replay point desired.
+ The instance will still be able to replay more WAL records (and in fact
+ will have to replay WAL records since last checkpoint upon next time it is
+ started).
+ </para>
+ <para>
+ Note that because <filename>recovery.conf</> will not be renamed when
+ <varname>action_at_recovery_target</> is set to <literal>shutdown</>,
+ any subsequent start will end with immediate shutdown unless the
+ configuration is changed or the <filename>recovery.conf</> is removed
+ manually.
+ </para>
+ <para>
This setting has no effect if <xref linkend="guc-hot-standby"> is not
enabled, or if no recovery target is set.
</para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3c9aeae..974e6c1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -227,7 +227,7 @@ static char *recoveryEndCommand = NULL;
static char *archiveCleanupCommand = NULL;
static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
static bool recoveryTargetInclusive = true;
-static bool recoveryPauseAtTarget = true;
+static RecoveryTargetAction actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
static TransactionId recoveryTargetXid;
static TimestampTz recoveryTargetTime;
static char *recoveryTargetName;
@@ -5068,6 +5068,9 @@ readRecoveryCommandFile(void)
ConfigVariable *item,
*head = NULL,
*tail = NULL;
+ bool recoveryPauseAtTargetSet = false;
+ bool actionAtRecoveryTargetSet = false;
+
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
if (fd == NULL)
@@ -5113,13 +5116,43 @@ readRecoveryCommandFile(void)
}
else if (strcmp(item->name, "pause_at_recovery_target") == 0)
{
+ bool recoveryPauseAtTarget;
+
if (!parse_bool(item->value, &recoveryPauseAtTarget))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires a Boolean value", "pause_at_recovery_target")));
+
ereport(DEBUG2,
(errmsg_internal("pause_at_recovery_target = '%s'",
item->value)));
+
+ actionAtRecoveryTarget = recoveryPauseAtTarget ?
+ RECOVERY_TARGET_ACTION_PAUSE :
+ RECOVERY_TARGET_ACTION_PROMOTE;
+
+ recoveryPauseAtTargetSet = true;
+ }
+ else if (strcmp(item->name, "action_at_recovery_target") == 0)
+ {
+ if (strcmp(item->value, "pause") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
+ else if (strcmp(item->value, "promote") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PROMOTE;
+ else if (strcmp(item->value, "shutdown") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for recovery parameter \"%s\"",
+ "action_at_recovery_target"),
+ errhint("The allowed values are \"pause\", \"promote\" and \"shutdown\".")));
+
+ ereport(DEBUG2,
+ (errmsg_internal("action_at_recovery_target = '%s'",
+ item->value)));
+
+ actionAtRecoveryTargetSet = true;
}
else if (strcmp(item->name, "recovery_target_timeline") == 0)
{
@@ -5284,6 +5317,18 @@ readRecoveryCommandFile(void)
RECOVERY_COMMAND_FILE)));
}
+ /*
+ * Check for mutually exclusive parameters
+ */
+ if (recoveryPauseAtTargetSet && actionAtRecoveryTargetSet)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot set both \"%s\" and \"%s\" recovery parameters",
+ "pause_at_recovery_target",
+ "action_at_recovery_target"),
+ errhint("The \"pause_at_recovery_target\" is deprecated.")));
+
+
/* Enable fetching from archive recovery area */
ArchiveRecoveryRequested = true;
@@ -6832,7 +6877,8 @@ StartupXLOG(void)
* end of main redo apply loop
*/
- if (recoveryPauseAtTarget && reachedStopPoint)
+ if (actionAtRecoveryTarget == RECOVERY_TARGET_ACTION_PAUSE &&
+ reachedStopPoint)
{
SetRecoveryPause(true);
recoveryPausesHere();
@@ -6853,6 +6899,25 @@ StartupXLOG(void)
ereport(LOG,
(errmsg("last completed transaction was at log time %s",
timestamptz_to_str(xtime))));
+
+ /*
+ * Shutdown here when requested. We need to exit this early because
+ * we want to be able to continue the WAL replay when started
+ * the next time.
+ */
+ if (actionAtRecoveryTarget == RECOVERY_TARGET_ACTION_SHUTDOWN &&
+ reachedStopPoint)
+ {
+ ereport(LOG, (errmsg("shutting down at recovery target")));
+ /*
+ * Note that we exit with status 2 instead of 0 here to force
+ * postmaster shutdown the whole instance. This produces one
+ * unnecessary log line, but we don't have better way to
+ * shutdown postmaster from within single backend.
+ */
+ proc_exit(2);
+ }
+
InRedo = false;
}
else
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 0ae110f..7e48138 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -179,6 +179,16 @@ typedef enum
RECOVERY_TARGET_IMMEDIATE
} RecoveryTargetType;
+/*
+ * Recovery target action.
+ */
+typedef enum
+{
+ RECOVERY_TARGET_ACTION_PAUSE,
+ RECOVERY_TARGET_ACTION_PROMOTE,
+ RECOVERY_TARGET_ACTION_SHUTDOWN,
+} RecoveryTargetAction;
+
extern XLogRecPtr XactLastRecEnd;
extern bool reachedConsistency;
On 18 November 2014 22:05, Petr Jelinek <petr@2ndquadrant.com> wrote:
OK, promote works for me as well, I attached patch that changes continue to
promote so you don't have to find and replace everything yourself. The
changed doc wording probably needs to be checked.
I've reworded docs a little.
Which led me to think about shutdown more.
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE. I can raise that
separately if anyone objects.
Also, for the Shutdown itself, why are we not using
kill(PostmasterPid, SIGINT)?
That gives a clean, fast shutdown rather than what looks like a crash.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 19, 2014 at 8:13 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE.
To me, that seems like a definite improvement.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 19/11/14 14:13, Simon Riggs wrote:
On 18 November 2014 22:05, Petr Jelinek <petr@2ndquadrant.com> wrote:
OK, promote works for me as well, I attached patch that changes continue to
promote so you don't have to find and replace everything yourself. The
changed doc wording probably needs to be checked.I've reworded docs a little.
Which led me to think about shutdown more.
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE. I can raise that
separately if anyone objects.
Ok that seems reasonable, I can write updated patch which does that.
Also, for the Shutdown itself, why are we not using
kill(PostmasterPid, SIGINT)?That gives a clean, fast shutdown rather than what looks like a crash.
My first (unsubmitted) version did that, there was some issue with
latches when doing that, but I think that's no longer problem as the
point at which the shutdown happens was moved away from the problematic
part of code. Other than that, it's just child killing postmaster feels
bit weird, but I don't have strong objection to it.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 19 November 2014 13:13, Simon Riggs <simon@2ndquadrant.com> wrote:
I've reworded docs a little.
Done
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE.
Done
Also, for the Shutdown itself, why are we not using
kill(PostmasterPid, SIGINT)?
Done
Other plan is to throw a FATAL message.
That gives a clean, fast shutdown rather than what looks like a crash.
I've also changed the location of where we do
RECOVERY_TARGET_ACTION_SHUTDOWN, so its in the same place as where we
pause.
I've also moved the check to see if we should throw FATAL because
aren't yet consistent to *before* we do any actionOnRecoveryTarget
stuff. It seems essential that we know that earlier rather than later.
Thoughts?
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
shutdown_at_recovery_target-v4.patchapplication/octet-stream; name=shutdown_at_recovery_target-v4.patchDownload
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 0f1ff34..a145a3f 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -289,12 +289,39 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</term>
<listitem>
<para>
- Specifies whether recovery should pause when the recovery target
- is reached. The default is true.
- This is intended to allow queries to be executed against the
- database to check if this recovery target is the most desirable
- point for recovery. The paused state can be resumed by using
- <function>pg_xlog_replay_resume()</> (See
+ Alias for action_at_recovery_target, <literal>true</> is same as
+ action_at_recovery_target = <literal>pause</> and <literal>false</>
+ is same as action_at_recovery_target = <literal>promote</>.
+ </para>
+ <para>
+ This setting has no effect if <xref linkend="guc-hot-standby"> is not
+ enabled, or if no recovery target is set.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+
+ <varlistentry id="action-at-recovery-target"
+ xreflabel="action_at_recovery_target">
+ <term><varname>action_at_recovery_target</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>action_at_recovery_target</> recovery parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies what action the server should take once the recovery target is
+ reached. The default is <literal>pause</>, which means recovery will
+ be paused. <literal>promote</> means recovery process will finish and
+ the server will start to accept connections.
+ Finally <literal>shutdown</> will stop the server after reaching the
+ recovery target.
+ </para>
+ The intended use of <literal>pause</> setting is to allow queries to be
+ executed against the database to check if this recovery target is the
+ most desirable point for recovery. The paused state can be resumed by
+ using <function>pg_xlog_replay_resume()</> (See
<xref linkend="functions-recovery-control-table">), which then
causes recovery to end. If this recovery target is not the
desired stopping point, then shutdown the server, change the
@@ -302,8 +329,23 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
continue recovery.
</para>
<para>
- This setting has no effect if <xref linkend="guc-hot-standby"> is not
- enabled, or if no recovery target is set.
+ The <literal>shutdown</> setting is useful to have instance ready at
+ exact replay point desired.
+ The instance will still be able to replay more WAL records (and in fact
+ will have to replay WAL records since last checkpoint next time it is
+ started).
+ </para>
+ <para>
+ Note that because <filename>recovery.conf</> will not be renamed when
+ <varname>action_at_recovery_target</> is set to <literal>shutdown</>,
+ any subsequent start will end with immediate shutdown unless the
+ configuration is changed or the <filename>recovery.conf</> is removed
+ manually.
+ </para>
+ <para>
+ This setting has no effect if no recovery target is set.
+ If <xref linkend="guc-hot-standby"> is not enabled, a setting of
+ <literal>pause</> will act the same as <literal>shutdown</>.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6053127..29b7fc5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -228,7 +228,7 @@ static char *recoveryEndCommand = NULL;
static char *archiveCleanupCommand = NULL;
static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
static bool recoveryTargetInclusive = true;
-static bool recoveryPauseAtTarget = true;
+static RecoveryTargetAction actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
static TransactionId recoveryTargetXid;
static TimestampTz recoveryTargetTime;
static char *recoveryTargetName;
@@ -4643,6 +4643,9 @@ readRecoveryCommandFile(void)
ConfigVariable *item,
*head = NULL,
*tail = NULL;
+ bool recoveryPauseAtTargetSet = false;
+ bool actionAtRecoveryTargetSet = false;
+
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
if (fd == NULL)
@@ -4688,13 +4691,43 @@ readRecoveryCommandFile(void)
}
else if (strcmp(item->name, "pause_at_recovery_target") == 0)
{
+ bool recoveryPauseAtTarget;
+
if (!parse_bool(item->value, &recoveryPauseAtTarget))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires a Boolean value", "pause_at_recovery_target")));
+
ereport(DEBUG2,
(errmsg_internal("pause_at_recovery_target = '%s'",
item->value)));
+
+ actionAtRecoveryTarget = recoveryPauseAtTarget ?
+ RECOVERY_TARGET_ACTION_PAUSE :
+ RECOVERY_TARGET_ACTION_PROMOTE;
+
+ recoveryPauseAtTargetSet = true;
+ }
+ else if (strcmp(item->name, "action_at_recovery_target") == 0)
+ {
+ if (strcmp(item->value, "pause") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
+ else if (strcmp(item->value, "promote") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PROMOTE;
+ else if (strcmp(item->value, "shutdown") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for recovery parameter \"%s\"",
+ "action_at_recovery_target"),
+ errhint("The allowed values are \"pause\", \"promote\" and \"shutdown\".")));
+
+ ereport(DEBUG2,
+ (errmsg_internal("action_at_recovery_target = '%s'",
+ item->value)));
+
+ actionAtRecoveryTargetSet = true;
}
else if (strcmp(item->name, "recovery_target_timeline") == 0)
{
@@ -4859,6 +4892,25 @@ readRecoveryCommandFile(void)
RECOVERY_COMMAND_FILE)));
}
+ /*
+ * Check for mutually exclusive parameters
+ */
+ if (recoveryPauseAtTargetSet && actionAtRecoveryTargetSet)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot set both \"%s\" and \"%s\" recovery parameters",
+ "pause_at_recovery_target",
+ "action_at_recovery_target"),
+ errhint("The \"pause_at_recovery_target\" is deprecated.")));
+
+
+ /*
+ * Override any inconsistent requests
+ */
+ if (actionAtRecoveryTarget == RECOVERY_TARGET_ACTION_PAUSE &&
+ standbyState == STANDBY_DISABLED)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+
/* Enable fetching from archive recovery area */
ArchiveRecoveryRequested = true;
@@ -6408,10 +6460,41 @@ StartupXLOG(void)
* end of main redo apply loop
*/
- if (recoveryPauseAtTarget && reachedStopPoint)
+ if (reachedStopPoint)
{
- SetRecoveryPause(true);
- recoveryPausesHere();
+ if (!reachedConsistency)
+ ereport(FATAL,
+ (errmsg("requested recovery stop point is before consistent recovery point")));
+
+ /*
+ * This is the last point where we can restart recovery with a
+ * new recovery target, if we shutdown and begin again. After
+ * this, Resource Managers may choose to do permanent corrective
+ * actions at end of recovery.
+ */
+ switch (actionAtRecoveryTarget)
+ {
+ case RECOVERY_TARGET_ACTION_SHUTDOWN:
+ ereport(LOG, (errmsg("shutting down at recovery target")));
+ /*
+ * Tell postmaster to shutdown, which then calls us
+ * back to exit cleanly.
+ */
+ if (kill(PostmasterPid, SIGINT))
+ ereport(FATAL,
+ (errmsg("failed to send signal to postmaster: %m")));
+
+ /* drop into pause while waiting for clean shutdown */
+
+ case RECOVERY_TARGET_ACTION_PAUSE:
+ SetRecoveryPause(true);
+ recoveryPausesHere();
+
+ /* drop into promote */
+
+ case RECOVERY_TARGET_ACTION_PROMOTE:
+ break;
+ }
}
/* Allow resource managers to do any required cleanup. */
@@ -6429,6 +6512,7 @@ StartupXLOG(void)
ereport(LOG,
(errmsg("last completed transaction was at log time %s",
timestamptz_to_str(xtime))));
+
InRedo = false;
}
else
@@ -6489,13 +6573,6 @@ StartupXLOG(void)
(EndOfLog < minRecoveryPoint ||
!XLogRecPtrIsInvalid(ControlFile->backupStartPoint)))
{
- if (reachedStopPoint)
- {
- /* stopped because of stop request */
- ereport(FATAL,
- (errmsg("requested recovery stop point is before consistent recovery point")));
- }
-
/*
* Ran off end of WAL before reaching end-of-backup WAL record, or
* minRecoveryPoint. That's usually a bad sign, indicating that you
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index 19b2ef8..04866e6 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -204,6 +204,16 @@ typedef struct xl_end_of_recovery
} xl_end_of_recovery;
/*
+ * Recovery target action.
+ */
+typedef enum
+{
+ RECOVERY_TARGET_ACTION_PAUSE,
+ RECOVERY_TARGET_ACTION_PROMOTE,
+ RECOVERY_TARGET_ACTION_SHUTDOWN,
+} RecoveryTargetAction;
+
+/*
* Method table for resource managers.
*
* This struct must be kept in sync with the PG_RMGR definition in
On 2014-11-19 15:47:05 +0000, Simon Riggs wrote:
Also, for the Shutdown itself, why are we not using
kill(PostmasterPid, SIGINT)?Done
I don't think that's ok. The postmaster is the one that should be in
control, not some subprocess.
I fail to see the win in simplicity over using exit (like we already do
for the normal end of recovery!) is. The issue with the log line seems
perfectly easily to avoid by just checking the exit code in
postmaster.c.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 19 November 2014 15:57, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-11-19 15:47:05 +0000, Simon Riggs wrote:
Also, for the Shutdown itself, why are we not using
kill(PostmasterPid, SIGINT)?Done
I don't think that's ok. The postmaster is the one that should be in
control, not some subprocess.I fail to see the win in simplicity over using exit (like we already do
for the normal end of recovery!) is. The issue with the log line seems
perfectly easily to avoid by just checking the exit code in
postmaster.c.
We need to be able to tell the difference between a crashed Startup
process and this usage.
As long as we can tell, I don't mind how we do it.
Suggestions please.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 19/11/14 17:04, Simon Riggs wrote:
On 19 November 2014 15:57, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-11-19 15:47:05 +0000, Simon Riggs wrote:
Also, for the Shutdown itself, why are we not using
kill(PostmasterPid, SIGINT)?Done
I don't think that's ok. The postmaster is the one that should be in
control, not some subprocess.I fail to see the win in simplicity over using exit (like we already do
for the normal end of recovery!) is. The issue with the log line seems
perfectly easily to avoid by just checking the exit code in
postmaster.c.We need to be able to tell the difference between a crashed Startup
process and this usage.As long as we can tell, I don't mind how we do it.
Suggestions please.
Different exit code?
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-11-19 16:04:49 +0000, Simon Riggs wrote:
On 19 November 2014 15:57, Andres Freund <andres@2ndquadrant.com> wrote:
On 2014-11-19 15:47:05 +0000, Simon Riggs wrote:
Also, for the Shutdown itself, why are we not using
kill(PostmasterPid, SIGINT)?Done
I don't think that's ok. The postmaster is the one that should be in
control, not some subprocess.
Just as an example why I think this is wrong: Some user could just
trigger replication to resume and we'd be in some awkward state.
I fail to see the win in simplicity over using exit (like we already do
for the normal end of recovery!) is. The issue with the log line seems
perfectly easily to avoid by just checking the exit code in
postmaster.c.We need to be able to tell the difference between a crashed Startup
process and this usage.
Exit code, as suggested above.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 19/11/14 16:47, Simon Riggs wrote:
On 19 November 2014 13:13, Simon Riggs <simon@2ndquadrant.com> wrote:
Also, for the Shutdown itself, why are we not using
kill(PostmasterPid, SIGINT)?Done
Other plan is to throw a FATAL message.
That gives a clean, fast shutdown rather than what looks like a crash.
I've also changed the location of where we do
RECOVERY_TARGET_ACTION_SHUTDOWN, so its in the same place as where we
pause.
Another problem with how you did these two changes is that if you just
pause when you send kill then during clean shutdown the recovery will be
resumed and will finish renaming the recovery.conf to recovery.done,
bumping timeline etc, and we don't want that since that prevents
resuming recovery in the future.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 19, 2014 at 10:49 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Nov 19, 2014 at 8:13 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE.To me, that seems like a definite improvement.
But changing the default will force us to set
action_at_recovery_target to 'promote'
when we want to just recover the database to the specified point. This
extra step is
not necessary so far, but required in 9.5, which would surprise the
users and may
cause some troubles like "Oh, in 9.5, PITR failed and the server shut
down unexpectedly
even though I just ran the PITR procedures that I used to use so
far....". Of course
probably I can live with the change of the default if it's really worthwhile and
we warn the users about that, though.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 19 November 2014 16:11, Petr Jelinek <petr@2ndquadrant.com> wrote:
We need to be able to tell the difference between a crashed Startup
process and this usage.As long as we can tell, I don't mind how we do it.
Suggestions please.
Different exit code?
Try this one.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
shutdown_at_recovery_target-v5.patchapplication/octet-stream; name=shutdown_at_recovery_target-v5.patchDownload
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 0f1ff34..a145a3f 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -289,12 +289,39 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</term>
<listitem>
<para>
- Specifies whether recovery should pause when the recovery target
- is reached. The default is true.
- This is intended to allow queries to be executed against the
- database to check if this recovery target is the most desirable
- point for recovery. The paused state can be resumed by using
- <function>pg_xlog_replay_resume()</> (See
+ Alias for action_at_recovery_target, <literal>true</> is same as
+ action_at_recovery_target = <literal>pause</> and <literal>false</>
+ is same as action_at_recovery_target = <literal>promote</>.
+ </para>
+ <para>
+ This setting has no effect if <xref linkend="guc-hot-standby"> is not
+ enabled, or if no recovery target is set.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+
+ <varlistentry id="action-at-recovery-target"
+ xreflabel="action_at_recovery_target">
+ <term><varname>action_at_recovery_target</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>action_at_recovery_target</> recovery parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies what action the server should take once the recovery target is
+ reached. The default is <literal>pause</>, which means recovery will
+ be paused. <literal>promote</> means recovery process will finish and
+ the server will start to accept connections.
+ Finally <literal>shutdown</> will stop the server after reaching the
+ recovery target.
+ </para>
+ The intended use of <literal>pause</> setting is to allow queries to be
+ executed against the database to check if this recovery target is the
+ most desirable point for recovery. The paused state can be resumed by
+ using <function>pg_xlog_replay_resume()</> (See
<xref linkend="functions-recovery-control-table">), which then
causes recovery to end. If this recovery target is not the
desired stopping point, then shutdown the server, change the
@@ -302,8 +329,23 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
continue recovery.
</para>
<para>
- This setting has no effect if <xref linkend="guc-hot-standby"> is not
- enabled, or if no recovery target is set.
+ The <literal>shutdown</> setting is useful to have instance ready at
+ exact replay point desired.
+ The instance will still be able to replay more WAL records (and in fact
+ will have to replay WAL records since last checkpoint next time it is
+ started).
+ </para>
+ <para>
+ Note that because <filename>recovery.conf</> will not be renamed when
+ <varname>action_at_recovery_target</> is set to <literal>shutdown</>,
+ any subsequent start will end with immediate shutdown unless the
+ configuration is changed or the <filename>recovery.conf</> is removed
+ manually.
+ </para>
+ <para>
+ This setting has no effect if no recovery target is set.
+ If <xref linkend="guc-hot-standby"> is not enabled, a setting of
+ <literal>pause</> will act the same as <literal>shutdown</>.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6053127..27138c2 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -228,7 +228,7 @@ static char *recoveryEndCommand = NULL;
static char *archiveCleanupCommand = NULL;
static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
static bool recoveryTargetInclusive = true;
-static bool recoveryPauseAtTarget = true;
+static RecoveryTargetAction actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
static TransactionId recoveryTargetXid;
static TimestampTz recoveryTargetTime;
static char *recoveryTargetName;
@@ -4643,6 +4643,9 @@ readRecoveryCommandFile(void)
ConfigVariable *item,
*head = NULL,
*tail = NULL;
+ bool recoveryPauseAtTargetSet = false;
+ bool actionAtRecoveryTargetSet = false;
+
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
if (fd == NULL)
@@ -4688,13 +4691,43 @@ readRecoveryCommandFile(void)
}
else if (strcmp(item->name, "pause_at_recovery_target") == 0)
{
+ bool recoveryPauseAtTarget;
+
if (!parse_bool(item->value, &recoveryPauseAtTarget))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires a Boolean value", "pause_at_recovery_target")));
+
ereport(DEBUG2,
(errmsg_internal("pause_at_recovery_target = '%s'",
item->value)));
+
+ actionAtRecoveryTarget = recoveryPauseAtTarget ?
+ RECOVERY_TARGET_ACTION_PAUSE :
+ RECOVERY_TARGET_ACTION_PROMOTE;
+
+ recoveryPauseAtTargetSet = true;
+ }
+ else if (strcmp(item->name, "action_at_recovery_target") == 0)
+ {
+ if (strcmp(item->value, "pause") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
+ else if (strcmp(item->value, "promote") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PROMOTE;
+ else if (strcmp(item->value, "shutdown") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for recovery parameter \"%s\"",
+ "action_at_recovery_target"),
+ errhint("The allowed values are \"pause\", \"promote\" and \"shutdown\".")));
+
+ ereport(DEBUG2,
+ (errmsg_internal("action_at_recovery_target = '%s'",
+ item->value)));
+
+ actionAtRecoveryTargetSet = true;
}
else if (strcmp(item->name, "recovery_target_timeline") == 0)
{
@@ -4859,6 +4892,25 @@ readRecoveryCommandFile(void)
RECOVERY_COMMAND_FILE)));
}
+ /*
+ * Check for mutually exclusive parameters
+ */
+ if (recoveryPauseAtTargetSet && actionAtRecoveryTargetSet)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot set both \"%s\" and \"%s\" recovery parameters",
+ "pause_at_recovery_target",
+ "action_at_recovery_target"),
+ errhint("The \"pause_at_recovery_target\" is deprecated.")));
+
+
+ /*
+ * Override any inconsistent requests
+ */
+ if (actionAtRecoveryTarget == RECOVERY_TARGET_ACTION_PAUSE &&
+ standbyState == STANDBY_DISABLED)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+
/* Enable fetching from archive recovery area */
ArchiveRecoveryRequested = true;
@@ -6408,10 +6460,37 @@ StartupXLOG(void)
* end of main redo apply loop
*/
- if (recoveryPauseAtTarget && reachedStopPoint)
+ if (reachedStopPoint)
{
- SetRecoveryPause(true);
- recoveryPausesHere();
+ if (!reachedConsistency)
+ ereport(FATAL,
+ (errmsg("requested recovery stop point is before consistent recovery point")));
+
+ /*
+ * This is the last point where we can restart recovery with a
+ * new recovery target, if we shutdown and begin again. After
+ * this, Resource Managers may choose to do permanent corrective
+ * actions at end of recovery.
+ */
+ switch (actionAtRecoveryTarget)
+ {
+ case RECOVERY_TARGET_ACTION_SHUTDOWN:
+ /*
+ * exit with special return code to request shutdown
+ * of postmaster. Log messages issued from
+ * postmaster.
+ */
+ proc_exit(2);
+
+ case RECOVERY_TARGET_ACTION_PAUSE:
+ SetRecoveryPause(true);
+ recoveryPausesHere();
+
+ /* drop into promote */
+
+ case RECOVERY_TARGET_ACTION_PROMOTE:
+ break;
+ }
}
/* Allow resource managers to do any required cleanup. */
@@ -6429,6 +6508,7 @@ StartupXLOG(void)
ereport(LOG,
(errmsg("last completed transaction was at log time %s",
timestamptz_to_str(xtime))));
+
InRedo = false;
}
else
@@ -6489,13 +6569,6 @@ StartupXLOG(void)
(EndOfLog < minRecoveryPoint ||
!XLogRecPtrIsInvalid(ControlFile->backupStartPoint)))
{
- if (reachedStopPoint)
- {
- /* stopped because of stop request */
- ereport(FATAL,
- (errmsg("requested recovery stop point is before consistent recovery point")));
- }
-
/*
* Ran off end of WAL before reaching end-of-backup WAL record, or
* minRecoveryPoint. That's usually a bad sign, indicating that you
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 6220a8e..fbb2a49 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -509,6 +509,7 @@ static void ShmemBackendArrayRemove(Backend *bn);
/* Macros to check exit status of a child process */
#define EXIT_STATUS_0(st) ((st) == 0)
#define EXIT_STATUS_1(st) (WIFEXITED(st) && WEXITSTATUS(st) == 1)
+#define EXIT_STATUS_2(st) (WIFEXITED(st) && WEXITSTATUS(st) == 2)
#ifndef WIN32
/*
@@ -2555,6 +2556,15 @@ reaper(SIGNAL_ARGS)
continue;
}
+ if (EXIT_STATUS_2(exitstatus))
+ {
+ ereport(LOG,
+ (errmsg("shutdown at recovery target")));
+ pmState = PM_WAIT_BACKENDS;
+ /* PostmasterStateMachine logic does the rest */
+ continue;
+ }
+
/*
* Unexpected exit of startup process (including FATAL exit)
* during PM_STARTUP is treated as catastrophic. There are no
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index 19b2ef8..04866e6 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -204,6 +204,16 @@ typedef struct xl_end_of_recovery
} xl_end_of_recovery;
/*
+ * Recovery target action.
+ */
+typedef enum
+{
+ RECOVERY_TARGET_ACTION_PAUSE,
+ RECOVERY_TARGET_ACTION_PROMOTE,
+ RECOVERY_TARGET_ACTION_SHUTDOWN,
+} RecoveryTargetAction;
+
+/*
* Method table for resource managers.
*
* This struct must be kept in sync with the PG_RMGR definition in
On 19 November 2014 16:41, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Nov 19, 2014 at 10:49 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Nov 19, 2014 at 8:13 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE.To me, that seems like a definite improvement.
But changing the default will force us to set
action_at_recovery_target to 'promote'
when we want to just recover the database to the specified point.
If you explicitly request pause and then can't pause, ISTM the action
we actually perform shouldn't be the exact opposite of what was
requested.
So if the user explicitly requests pause and we aren't in HS then they
should get Shutdown, which is closest action.
If the user doesn't request anything at all then we can default to
Promote, just like we do now.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 19/11/14 19:51, Simon Riggs wrote:
On 19 November 2014 16:11, Petr Jelinek <petr@2ndquadrant.com> wrote:
We need to be able to tell the difference between a crashed Startup
process and this usage.As long as we can tell, I don't mind how we do it.
Suggestions please.
Different exit code?
Try this one.
Ok this seems ok, I did couple of fixes - used exit code 3 as 2 is used
in some places - given the "if (pid == StartupPID)" it would probably
never conflict in practice, but better be safe than sorry in this case IMHO.
And you forgot to actually set the postmaster into one of the Shutdown
states so I added that.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
shutdown_at_recovery_target-v6.patchtext/x-diff; name=shutdown_at_recovery_target-v6.patchDownload
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 0f1ff34..a145a3f 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -289,12 +289,39 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</term>
<listitem>
<para>
- Specifies whether recovery should pause when the recovery target
- is reached. The default is true.
- This is intended to allow queries to be executed against the
- database to check if this recovery target is the most desirable
- point for recovery. The paused state can be resumed by using
- <function>pg_xlog_replay_resume()</> (See
+ Alias for action_at_recovery_target, <literal>true</> is same as
+ action_at_recovery_target = <literal>pause</> and <literal>false</>
+ is same as action_at_recovery_target = <literal>promote</>.
+ </para>
+ <para>
+ This setting has no effect if <xref linkend="guc-hot-standby"> is not
+ enabled, or if no recovery target is set.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+
+ <varlistentry id="action-at-recovery-target"
+ xreflabel="action_at_recovery_target">
+ <term><varname>action_at_recovery_target</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>action_at_recovery_target</> recovery parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies what action the server should take once the recovery target is
+ reached. The default is <literal>pause</>, which means recovery will
+ be paused. <literal>promote</> means recovery process will finish and
+ the server will start to accept connections.
+ Finally <literal>shutdown</> will stop the server after reaching the
+ recovery target.
+ </para>
+ The intended use of <literal>pause</> setting is to allow queries to be
+ executed against the database to check if this recovery target is the
+ most desirable point for recovery. The paused state can be resumed by
+ using <function>pg_xlog_replay_resume()</> (See
<xref linkend="functions-recovery-control-table">), which then
causes recovery to end. If this recovery target is not the
desired stopping point, then shutdown the server, change the
@@ -302,8 +329,23 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
continue recovery.
</para>
<para>
- This setting has no effect if <xref linkend="guc-hot-standby"> is not
- enabled, or if no recovery target is set.
+ The <literal>shutdown</> setting is useful to have instance ready at
+ exact replay point desired.
+ The instance will still be able to replay more WAL records (and in fact
+ will have to replay WAL records since last checkpoint next time it is
+ started).
+ </para>
+ <para>
+ Note that because <filename>recovery.conf</> will not be renamed when
+ <varname>action_at_recovery_target</> is set to <literal>shutdown</>,
+ any subsequent start will end with immediate shutdown unless the
+ configuration is changed or the <filename>recovery.conf</> is removed
+ manually.
+ </para>
+ <para>
+ This setting has no effect if no recovery target is set.
+ If <xref linkend="guc-hot-standby"> is not enabled, a setting of
+ <literal>pause</> will act the same as <literal>shutdown</>.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 99f702c..6747ea6 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -228,7 +228,7 @@ static char *recoveryEndCommand = NULL;
static char *archiveCleanupCommand = NULL;
static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
static bool recoveryTargetInclusive = true;
-static bool recoveryPauseAtTarget = true;
+static RecoveryTargetAction actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
static TransactionId recoveryTargetXid;
static TimestampTz recoveryTargetTime;
static char *recoveryTargetName;
@@ -4643,6 +4643,9 @@ readRecoveryCommandFile(void)
ConfigVariable *item,
*head = NULL,
*tail = NULL;
+ bool recoveryPauseAtTargetSet = false;
+ bool actionAtRecoveryTargetSet = false;
+
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
if (fd == NULL)
@@ -4688,13 +4691,43 @@ readRecoveryCommandFile(void)
}
else if (strcmp(item->name, "pause_at_recovery_target") == 0)
{
+ bool recoveryPauseAtTarget;
+
if (!parse_bool(item->value, &recoveryPauseAtTarget))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires a Boolean value", "pause_at_recovery_target")));
+
ereport(DEBUG2,
(errmsg_internal("pause_at_recovery_target = '%s'",
item->value)));
+
+ actionAtRecoveryTarget = recoveryPauseAtTarget ?
+ RECOVERY_TARGET_ACTION_PAUSE :
+ RECOVERY_TARGET_ACTION_PROMOTE;
+
+ recoveryPauseAtTargetSet = true;
+ }
+ else if (strcmp(item->name, "action_at_recovery_target") == 0)
+ {
+ if (strcmp(item->value, "pause") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
+ else if (strcmp(item->value, "promote") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PROMOTE;
+ else if (strcmp(item->value, "shutdown") == 0)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for recovery parameter \"%s\"",
+ "action_at_recovery_target"),
+ errhint("The allowed values are \"pause\", \"promote\" and \"shutdown\".")));
+
+ ereport(DEBUG2,
+ (errmsg_internal("action_at_recovery_target = '%s'",
+ item->value)));
+
+ actionAtRecoveryTargetSet = true;
}
else if (strcmp(item->name, "recovery_target_timeline") == 0)
{
@@ -4859,6 +4892,25 @@ readRecoveryCommandFile(void)
RECOVERY_COMMAND_FILE)));
}
+ /*
+ * Check for mutually exclusive parameters
+ */
+ if (recoveryPauseAtTargetSet && actionAtRecoveryTargetSet)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot set both \"%s\" and \"%s\" recovery parameters",
+ "pause_at_recovery_target",
+ "action_at_recovery_target"),
+ errhint("The \"pause_at_recovery_target\" is deprecated.")));
+
+
+ /*
+ * Override any inconsistent requests
+ */
+ if (actionAtRecoveryTarget == RECOVERY_TARGET_ACTION_PAUSE &&
+ standbyState == STANDBY_DISABLED)
+ actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+
/* Enable fetching from archive recovery area */
ArchiveRecoveryRequested = true;
@@ -6408,10 +6460,37 @@ StartupXLOG(void)
* end of main redo apply loop
*/
- if (recoveryPauseAtTarget && reachedStopPoint)
+ if (reachedStopPoint)
{
- SetRecoveryPause(true);
- recoveryPausesHere();
+ if (!reachedConsistency)
+ ereport(FATAL,
+ (errmsg("requested recovery stop point is before consistent recovery point")));
+
+ /*
+ * This is the last point where we can restart recovery with a
+ * new recovery target, if we shutdown and begin again. After
+ * this, Resource Managers may choose to do permanent corrective
+ * actions at end of recovery.
+ */
+ switch (actionAtRecoveryTarget)
+ {
+ case RECOVERY_TARGET_ACTION_SHUTDOWN:
+ /*
+ * exit with special return code to request shutdown
+ * of postmaster. Log messages issued from
+ * postmaster.
+ */
+ proc_exit(3);
+
+ case RECOVERY_TARGET_ACTION_PAUSE:
+ SetRecoveryPause(true);
+ recoveryPausesHere();
+
+ /* drop into promote */
+
+ case RECOVERY_TARGET_ACTION_PROMOTE:
+ break;
+ }
}
/* Allow resource managers to do any required cleanup. */
@@ -6429,6 +6508,7 @@ StartupXLOG(void)
ereport(LOG,
(errmsg("last completed transaction was at log time %s",
timestamptz_to_str(xtime))));
+
InRedo = false;
}
else
@@ -6479,13 +6559,6 @@ StartupXLOG(void)
(EndOfLog < minRecoveryPoint ||
!XLogRecPtrIsInvalid(ControlFile->backupStartPoint)))
{
- if (reachedStopPoint)
- {
- /* stopped because of stop request */
- ereport(FATAL,
- (errmsg("requested recovery stop point is before consistent recovery point")));
- }
-
/*
* Ran off end of WAL before reaching end-of-backup WAL record, or
* minRecoveryPoint. That's usually a bad sign, indicating that you
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 6220a8e..5106f52 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -509,6 +509,7 @@ static void ShmemBackendArrayRemove(Backend *bn);
/* Macros to check exit status of a child process */
#define EXIT_STATUS_0(st) ((st) == 0)
#define EXIT_STATUS_1(st) (WIFEXITED(st) && WEXITSTATUS(st) == 1)
+#define EXIT_STATUS_3(st) (WIFEXITED(st) && WEXITSTATUS(st) == 3)
#ifndef WIN32
/*
@@ -2555,6 +2556,17 @@ reaper(SIGNAL_ARGS)
continue;
}
+ if (EXIT_STATUS_3(exitstatus))
+ {
+ ereport(LOG,
+ (errmsg("shutdown at recovery target")));
+ Shutdown = SmartShutdown;
+ TerminateChildren(SIGTERM);
+ pmState = PM_WAIT_BACKENDS;
+ /* PostmasterStateMachine logic does the rest */
+ continue;
+ }
+
/*
* Unexpected exit of startup process (including FATAL exit)
* during PM_STARTUP is treated as catastrophic. There are no
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index 19b2ef8..04866e6 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -204,6 +204,16 @@ typedef struct xl_end_of_recovery
} xl_end_of_recovery;
/*
+ * Recovery target action.
+ */
+typedef enum
+{
+ RECOVERY_TARGET_ACTION_PAUSE,
+ RECOVERY_TARGET_ACTION_PROMOTE,
+ RECOVERY_TARGET_ACTION_SHUTDOWN,
+} RecoveryTargetAction;
+
+/*
* Method table for resource managers.
*
* This struct must be kept in sync with the PG_RMGR definition in
On 19 November 2014 22:47, Petr Jelinek <petr@2ndquadrant.com> wrote:
On 19/11/14 19:51, Simon Riggs wrote:
On 19 November 2014 16:11, Petr Jelinek <petr@2ndquadrant.com> wrote:
We need to be able to tell the difference between a crashed Startup
process and this usage.As long as we can tell, I don't mind how we do it.
...
Ok this seems ok, I did couple of fixes - used exit code 3 as 2 is used in
some places - given the "if (pid == StartupPID)" it would probably never
conflict in practice, but better be safe than sorry in this case IMHO.
And you forgot to actually set the postmaster into one of the Shutdown
states so I added that.
Like it.
Patch looks good now. Will commit shortly.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 20, 2014 at 5:35 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 19 November 2014 16:41, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Nov 19, 2014 at 10:49 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Nov 19, 2014 at 8:13 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE.To me, that seems like a definite improvement.
But changing the default will force us to set
action_at_recovery_target to 'promote'
when we want to just recover the database to the specified point.If you explicitly request pause and then can't pause, ISTM the action
we actually perform shouldn't be the exact opposite of what was
requested.So if the user explicitly requests pause and we aren't in HS then they
should get Shutdown, which is closest action.If the user doesn't request anything at all then we can default to
Promote, just like we do now.
Yes, this is what I was trying to suggest. +1 to do that.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 25 November 2014 at 14:06, Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Nov 20, 2014 at 5:35 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 19 November 2014 16:41, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Nov 19, 2014 at 10:49 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Nov 19, 2014 at 8:13 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE.To me, that seems like a definite improvement.
But changing the default will force us to set
action_at_recovery_target to 'promote'
when we want to just recover the database to the specified point.If you explicitly request pause and then can't pause, ISTM the action
we actually perform shouldn't be the exact opposite of what was
requested.So if the user explicitly requests pause and we aren't in HS then they
should get Shutdown, which is closest action.If the user doesn't request anything at all then we can default to
Promote, just like we do now.Yes, this is what I was trying to suggest. +1 to do that.
Implemented.
Patch committed.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 25/11/14 21:15, Simon Riggs wrote:
On 25 November 2014 at 14:06, Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Nov 20, 2014 at 5:35 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 19 November 2014 16:41, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Nov 19, 2014 at 10:49 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Nov 19, 2014 at 8:13 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE.To me, that seems like a definite improvement.
But changing the default will force us to set
action_at_recovery_target to 'promote'
when we want to just recover the database to the specified point.If you explicitly request pause and then can't pause, ISTM the action
we actually perform shouldn't be the exact opposite of what was
requested.So if the user explicitly requests pause and we aren't in HS then they
should get Shutdown, which is closest action.If the user doesn't request anything at all then we can default to
Promote, just like we do now.Yes, this is what I was trying to suggest. +1 to do that.
Implemented.
Patch committed.
Thanks!
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Petr Jelinek 2014-11-25 <5474EFEA.2040000@2ndquadrant.com>
Patch committed.
Thanks!
I'm a bit late to the party, but wouldn't
recovery_target_action = ...
have been a better name for this? It'd be in line with the other
recovery_target_* parameters, and also a bit shorter than the imho
somewhat ugly "action_at_recovery_target".
Christoph
--
cb@df7cb.de | http://www.df7cb.de/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 26, 2014 at 7:10 PM, Christoph Berg <cb@df7cb.de> wrote:
Re: Petr Jelinek 2014-11-25 <5474EFEA.2040000@2ndquadrant.com>
Patch committed.
Thanks!
I'm a bit late to the party, but wouldn't
recovery_target_action = ...
have been a better name for this? It'd be in line with the other
recovery_target_* parameters, and also a bit shorter than the imho
somewhat ugly "action_at_recovery_target".
Looks like a good idea to me. Why not as well mark
pause_at_recovery_target as deprecated in the docs and remove it in,
let's say, 2 releases?
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 26, 2014 at 5:15 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 25 November 2014 at 14:06, Fujii Masao <masao.fujii@gmail.com> wrote:
On Thu, Nov 20, 2014 at 5:35 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 19 November 2014 16:41, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Nov 19, 2014 at 10:49 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Nov 19, 2014 at 8:13 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
If we ask for PAUSE and we're not in HotStandby it just ignores it,
which means it changes into PROMOTE. My feeling is that we should
change that into a SHUTDOWN, not a PROMOTE.To me, that seems like a definite improvement.
But changing the default will force us to set
action_at_recovery_target to 'promote'
when we want to just recover the database to the specified point.If you explicitly request pause and then can't pause, ISTM the action
we actually perform shouldn't be the exact opposite of what was
requested.So if the user explicitly requests pause and we aren't in HS then they
should get Shutdown, which is closest action.If the user doesn't request anything at all then we can default to
Promote, just like we do now.Yes, this is what I was trying to suggest. +1 to do that.
Implemented.
Patch committed.
Isn't it better to add the sample setting of this parameter into
recovery.conf.sample?
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Christoph Berg <cb@df7cb.de> writes:
Re: Petr Jelinek 2014-11-25 <5474EFEA.2040000@2ndquadrant.com>
Patch committed.
Before I go and rebase that recovery.conf -> GUC patch on top of
this... is it final?
Thanks!
I'm a bit late to the party, but wouldn't
recovery_target_action = ...
have been a better name for this? It'd be in line with the other
recovery_target_* parameters, and also a bit shorter than the imho
somewhat ugly "action_at_recovery_target".
FWIW, I too think that "recovery_target_action" is a better name.
--
Alex
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 28/11/14 17:46, Alex Shulgin wrote:
Christoph Berg <cb@df7cb.de> writes:
Re: Petr Jelinek 2014-11-25 <5474EFEA.2040000@2ndquadrant.com>
Patch committed.
Before I go and rebase that recovery.conf -> GUC patch on top of
this... is it final?
I think so, perhaps sans the name mentioned below.
Thanks!
I'm a bit late to the party, but wouldn't
recovery_target_action = ...
have been a better name for this? It'd be in line with the other
recovery_target_* parameters, and also a bit shorter than the imho
somewhat ugly "action_at_recovery_target".FWIW, I too think that "recovery_target_action" is a better name.
I agree.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Nov 28, 2014 at 11:59 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
I'm a bit late to the party, but wouldn't
recovery_target_action = ...
have been a better name for this? It'd be in line with the other
recovery_target_* parameters, and also a bit shorter than the imho
somewhat ugly "action_at_recovery_target".FWIW, I too think that "recovery_target_action" is a better name.
I agree.
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02/12/14 18:59, Robert Haas wrote:
On Fri, Nov 28, 2014 at 11:59 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
I'm a bit late to the party, but wouldn't
recovery_target_action = ...
have been a better name for this? It'd be in line with the other
recovery_target_* parameters, and also a bit shorter than the imho
somewhat ugly "action_at_recovery_target".FWIW, I too think that "recovery_target_action" is a better name.
I agree.
+1.
Here is patch which renames action_at_recovery_target to
recovery_target_action everywhere.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
recovery_target_action.patchtext/x-diff; name=recovery_target_action.patchDownload
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index b4959ac..06d66d2 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -289,9 +289,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</term>
<listitem>
<para>
- Alias for action_at_recovery_target, <literal>true</> is same as
- action_at_recovery_target = <literal>pause</> and <literal>false</>
- is same as action_at_recovery_target = <literal>promote</>.
+ Alias for recovery_target_action, <literal>true</> is same as
+ recovery_target_action = <literal>pause</> and <literal>false</>
+ is same as recovery_target_action = <literal>promote</>.
</para>
<para>
This setting has no effect if <xref linkend="guc-hot-standby"> is not
@@ -300,11 +300,11 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</listitem>
</varlistentry>
- <varlistentry id="action-at-recovery-target"
- xreflabel="action_at_recovery_target">
- <term><varname>action_at_recovery_target</varname> (<type>enum</type>)
+ <varlistentry id="recovery-target-action"
+ xreflabel="recovery_target_action">
+ <term><varname>recovery_target_action</varname> (<type>enum</type>)
<indexterm>
- <primary><varname>action_at_recovery_target</> recovery parameter</primary>
+ <primary><varname>recovery_target_action</> recovery parameter</primary>
</indexterm>
</term>
<listitem>
@@ -336,7 +336,7 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</para>
<para>
Note that because <filename>recovery.conf</> will not be renamed when
- <varname>action_at_recovery_target</> is set to <literal>shutdown</>,
+ <varname>recovery_target_action</> is set to <literal>shutdown</>,
any subsequent start will end with immediate shutdown unless the
configuration is changed or the <filename>recovery.conf</> is removed
manually.
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index da28de9..e8e5325 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -229,7 +229,7 @@ static char *recoveryEndCommand = NULL;
static char *archiveCleanupCommand = NULL;
static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
static bool recoveryTargetInclusive = true;
-static RecoveryTargetAction actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
+static RecoveryTargetAction recoveryTargetAction = RECOVERY_TARGET_ACTION_PAUSE;
static TransactionId recoveryTargetXid;
static TimestampTz recoveryTargetTime;
static char *recoveryTargetName;
@@ -4654,7 +4654,7 @@ readRecoveryCommandFile(void)
*head = NULL,
*tail = NULL;
bool recoveryPauseAtTargetSet = false;
- bool actionAtRecoveryTargetSet = false;
+ bool recoveryTargetActionSet = false;
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
@@ -4712,32 +4712,32 @@ readRecoveryCommandFile(void)
(errmsg_internal("pause_at_recovery_target = '%s'",
item->value)));
- actionAtRecoveryTarget = recoveryPauseAtTarget ?
- RECOVERY_TARGET_ACTION_PAUSE :
- RECOVERY_TARGET_ACTION_PROMOTE;
+ recoveryTargetAction = recoveryPauseAtTarget ?
+ RECOVERY_TARGET_ACTION_PAUSE :
+ RECOVERY_TARGET_ACTION_PROMOTE;
recoveryPauseAtTargetSet = true;
}
- else if (strcmp(item->name, "action_at_recovery_target") == 0)
+ else if (strcmp(item->name, "recovery_target_action") == 0)
{
if (strcmp(item->value, "pause") == 0)
- actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
+ recoveryTargetAction = RECOVERY_TARGET_ACTION_PAUSE;
else if (strcmp(item->value, "promote") == 0)
- actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PROMOTE;
+ recoveryTargetAction = RECOVERY_TARGET_ACTION_PROMOTE;
else if (strcmp(item->value, "shutdown") == 0)
- actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ recoveryTargetAction = RECOVERY_TARGET_ACTION_SHUTDOWN;
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid value for recovery parameter \"%s\"",
- "action_at_recovery_target"),
+ "recovery_target_action"),
errhint("The allowed values are \"pause\", \"promote\" and \"shutdown\".")));
ereport(DEBUG2,
- (errmsg_internal("action_at_recovery_target = '%s'",
+ (errmsg_internal("recovery_target_action = '%s'",
item->value)));
- actionAtRecoveryTargetSet = true;
+ recoveryTargetActionSet = true;
}
else if (strcmp(item->name, "recovery_target_timeline") == 0)
{
@@ -4905,12 +4905,12 @@ readRecoveryCommandFile(void)
/*
* Check for mutually exclusive parameters
*/
- if (recoveryPauseAtTargetSet && actionAtRecoveryTargetSet)
+ if (recoveryPauseAtTargetSet && recoveryTargetActionSet)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cannot set both \"%s\" and \"%s\" recovery parameters",
"pause_at_recovery_target",
- "action_at_recovery_target"),
+ "recovery_target_action"),
errhint("The \"pause_at_recovery_target\" is deprecated.")));
@@ -4919,10 +4919,10 @@ readRecoveryCommandFile(void)
* of behaviour in 9.5; prior to this we simply ignored a request
* to pause if hot_standby = off, which was surprising behaviour.
*/
- if (actionAtRecoveryTarget == RECOVERY_TARGET_ACTION_PAUSE &&
- actionAtRecoveryTargetSet &&
+ if (recoveryTargetAction == RECOVERY_TARGET_ACTION_PAUSE &&
+ recoveryTargetActionSet &&
standbyState == STANDBY_DISABLED)
- actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ recoveryTargetAction = RECOVERY_TARGET_ACTION_SHUTDOWN;
/* Enable fetching from archive recovery area */
ArchiveRecoveryRequested = true;
@@ -6495,7 +6495,7 @@ StartupXLOG(void)
* this, Resource Managers may choose to do permanent corrective
* actions at end of recovery.
*/
- switch (actionAtRecoveryTarget)
+ switch (recoveryTargetAction)
{
case RECOVERY_TARGET_ACTION_SHUTDOWN:
/*
On 4 December 2014 at 22:13, Petr Jelinek <petr@2ndquadrant.com> wrote:
On 02/12/14 18:59, Robert Haas wrote:
On Fri, Nov 28, 2014 at 11:59 AM, Petr Jelinek <petr@2ndquadrant.com>
wrote:I'm a bit late to the party, but wouldn't
recovery_target_action = ...
have been a better name for this? It'd be in line with the other
recovery_target_* parameters, and also a bit shorter than the imho
somewhat ugly "action_at_recovery_target".FWIW, I too think that "recovery_target_action" is a better name.
I agree.
+1.
Here is patch which renames action_at_recovery_target to
recovery_target_action everywhere.
Thanks; I'll fix it up on Monday.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Dec 4, 2014 at 10:13 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
On 02/12/14 18:59, Robert Haas wrote:
On Fri, Nov 28, 2014 at 11:59 AM, Petr Jelinek <petr@2ndquadrant.com>
wrote:I'm a bit late to the party, but wouldn't
recovery_target_action = ...
have been a better name for this? It'd be in line with the other
recovery_target_* parameters, and also a bit shorter than the imho
somewhat ugly "action_at_recovery_target".FWIW, I too think that "recovery_target_action" is a better name.
I agree.
+1.
Here is patch which renames action_at_recovery_target to
recovery_target_action everywhere.
Thanks, Looks good to me.
A couple of things that would be good to document as well in
recovery-config.sgml:
- the fact that pause_at_recovery_target is deprecated.
- the fact that both parameters cannot be used at the same time.
Let's not surprise the users with behaviors they may expect or guess and
document this stuff precisely..
This would make docs consistent with this block of code in xlog.c:
if (recoveryPauseAtTargetSet && actionAtRecoveryTargetSet)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cannot set both \"%s\" and \"%s\"
recovery parameters",
"pause_at_recovery_target",
"action_at_recovery_target"),
errhint("The \"pause_at_recovery_target\"
is deprecated.")));
Regards,
--
Michael
On Thu, Dec 4, 2014 at 10:45 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Thu, Dec 4, 2014 at 10:13 PM, Petr Jelinek <petr@2ndquadrant.com>
wrote:On 02/12/14 18:59, Robert Haas wrote:
On Fri, Nov 28, 2014 at 11:59 AM, Petr Jelinek <petr@2ndquadrant.com>
wrote:I'm a bit late to the party, but wouldn't
recovery_target_action = ...
have been a better name for this? It'd be in line with the other
recovery_target_* parameters, and also a bit shorter than the imho
somewhat ugly "action_at_recovery_target".FWIW, I too think that "recovery_target_action" is a better name.
I agree.
+1.
Here is patch which renames action_at_recovery_target to
recovery_target_action everywhere.Thanks, Looks good to me.
A couple of things that would be good to document as well in
recovery-config.sgml:
- the fact that pause_at_recovery_target is deprecated.
- the fact that both parameters cannot be used at the same time.
Let's not surprise the users with behaviors they may expect or guess and
document this stuff precisely..
Btw, you are missing as well the addition of this parameter in
recovery.conf.sample (mentioned by Fujii-san upthread). It would be nice to
have a description paragraph as well similarly to what is written for
pause_at_recovery_target.
--
Michael
On Thu, Dec 4, 2014 at 8:45 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
Here is patch which renames action_at_recovery_target to
recovery_target_action everywhere.Thanks, Looks good to me.
A couple of things that would be good to document as well in
recovery-config.sgml:
- the fact that pause_at_recovery_target is deprecated.
Why not just remove it altogether? I don't think the
backward-compatibility break is enough to get excited about, or to
justify the annoyance of carrying an extra parameter that does (part
of) the same thing.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Dec 6, 2014 at 12:49 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Dec 4, 2014 at 8:45 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:Here is patch which renames action_at_recovery_target to
recovery_target_action everywhere.Thanks, Looks good to me.
A couple of things that would be good to document as well in
recovery-config.sgml:
- the fact that pause_at_recovery_target is deprecated.Why not just remove it altogether? I don't think the
backward-compatibility break is enough to get excited about, or to
justify the annoyance of carrying an extra parameter that does (part
of) the same thing.
At least we won't forget removing in the future something that has been
marked as deprecated for years. So +1 here for a simple removal, and a
mention in the future release notes.
--
Michael
On 05/12/14 16:49, Robert Haas wrote:
On Thu, Dec 4, 2014 at 8:45 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:Here is patch which renames action_at_recovery_target to
recovery_target_action everywhere.Thanks, Looks good to me.
A couple of things that would be good to document as well in
recovery-config.sgml:
- the fact that pause_at_recovery_target is deprecated.Why not just remove it altogether? I don't think the
backward-compatibility break is enough to get excited about, or to
justify the annoyance of carrying an extra parameter that does (part
of) the same thing.
Ok this patch does that, along with the rename to recovery_target_action
and addition to the recovery.conf.sample.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
recovery_target_action-v2.patchtext/x-diff; name=recovery_target_action-v2.patchDownload
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index b4959ac..519a0ce 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -280,31 +280,11 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</listitem>
</varlistentry>
- <varlistentry id="pause-at-recovery-target"
- xreflabel="pause_at_recovery_target">
- <term><varname>pause_at_recovery_target</varname> (<type>boolean</type>)
+ <varlistentry id="recovery-target-action"
+ xreflabel="recovery_target_action">
+ <term><varname>recovery_target_action</varname> (<type>enum</type>)
<indexterm>
- <primary><varname>pause_at_recovery_target</> recovery parameter</primary>
- </indexterm>
- </term>
- <listitem>
- <para>
- Alias for action_at_recovery_target, <literal>true</> is same as
- action_at_recovery_target = <literal>pause</> and <literal>false</>
- is same as action_at_recovery_target = <literal>promote</>.
- </para>
- <para>
- This setting has no effect if <xref linkend="guc-hot-standby"> is not
- enabled, or if no recovery target is set.
- </para>
- </listitem>
- </varlistentry>
-
- <varlistentry id="action-at-recovery-target"
- xreflabel="action_at_recovery_target">
- <term><varname>action_at_recovery_target</varname> (<type>enum</type>)
- <indexterm>
- <primary><varname>action_at_recovery_target</> recovery parameter</primary>
+ <primary><varname>recovery_target_action</> recovery parameter</primary>
</indexterm>
</term>
<listitem>
@@ -336,7 +316,7 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</para>
<para>
Note that because <filename>recovery.conf</> will not be renamed when
- <varname>action_at_recovery_target</> is set to <literal>shutdown</>,
+ <varname>recovery_target_action</> is set to <literal>shutdown</>,
any subsequent start will end with immediate shutdown unless the
configuration is changed or the <filename>recovery.conf</> is removed
manually.
diff --git a/doc/src/sgml/release-9.1.sgml b/doc/src/sgml/release-9.1.sgml
index 79a8b07..4fcc0b3 100644
--- a/doc/src/sgml/release-9.1.sgml
+++ b/doc/src/sgml/release-9.1.sgml
@@ -6301,8 +6301,7 @@
<listitem>
<para>
- Add <filename>recovery.conf</> setting <link
- linkend="pause-at-recovery-target"><varname>pause_at_recovery_target</></link>
+ Add <filename>recovery.conf</> setting <varname>pause_at_recovery_target</>
to pause recovery at target (Simon Riggs)
</para>
diff --git a/src/backend/access/transam/recovery.conf.sample b/src/backend/access/transam/recovery.conf.sample
index 7657df3..42da62b 100644
--- a/src/backend/access/transam/recovery.conf.sample
+++ b/src/backend/access/transam/recovery.conf.sample
@@ -94,12 +94,13 @@
#recovery_target_timeline = 'latest'
#
#
-# If pause_at_recovery_target is enabled, recovery will pause when
-# the recovery target is reached. The pause state will continue until
-# pg_xlog_replay_resume() is called. This setting has no effect if
-# hot standby is not enabled, or if no recovery target is set.
+# The recovery_target_action option can be set to either promote, paused
+# or shutdown and decides the behaviour of the server once the recovery_target
+# is reached. Promote will promote the server to standalone one. Pause will
+# pause the recovery, which can be then resumed by pg_xlog_replay_resume().
+# And shutdown will stop the server.
#
-#pause_at_recovery_target = true
+#recovery_target_action = 'pause'
#
#---------------------------------------------------------------------------
# STANDBY SERVER PARAMETERS
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index da28de9..469a83b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -229,7 +229,7 @@ static char *recoveryEndCommand = NULL;
static char *archiveCleanupCommand = NULL;
static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
static bool recoveryTargetInclusive = true;
-static RecoveryTargetAction actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
+static RecoveryTargetAction recoveryTargetAction = RECOVERY_TARGET_ACTION_PAUSE;
static TransactionId recoveryTargetXid;
static TimestampTz recoveryTargetTime;
static char *recoveryTargetName;
@@ -4653,8 +4653,7 @@ readRecoveryCommandFile(void)
ConfigVariable *item,
*head = NULL,
*tail = NULL;
- bool recoveryPauseAtTargetSet = false;
- bool actionAtRecoveryTargetSet = false;
+ bool recoveryTargetActionSet = false;
fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
@@ -4699,45 +4698,26 @@ readRecoveryCommandFile(void)
(errmsg_internal("archive_cleanup_command = '%s'",
archiveCleanupCommand)));
}
- else if (strcmp(item->name, "pause_at_recovery_target") == 0)
- {
- bool recoveryPauseAtTarget;
-
- if (!parse_bool(item->value, &recoveryPauseAtTarget))
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("parameter \"%s\" requires a Boolean value", "pause_at_recovery_target")));
-
- ereport(DEBUG2,
- (errmsg_internal("pause_at_recovery_target = '%s'",
- item->value)));
-
- actionAtRecoveryTarget = recoveryPauseAtTarget ?
- RECOVERY_TARGET_ACTION_PAUSE :
- RECOVERY_TARGET_ACTION_PROMOTE;
-
- recoveryPauseAtTargetSet = true;
- }
- else if (strcmp(item->name, "action_at_recovery_target") == 0)
+ else if (strcmp(item->name, "recovery_target_action") == 0)
{
if (strcmp(item->value, "pause") == 0)
- actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PAUSE;
+ recoveryTargetAction = RECOVERY_TARGET_ACTION_PAUSE;
else if (strcmp(item->value, "promote") == 0)
- actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_PROMOTE;
+ recoveryTargetAction = RECOVERY_TARGET_ACTION_PROMOTE;
else if (strcmp(item->value, "shutdown") == 0)
- actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ recoveryTargetAction = RECOVERY_TARGET_ACTION_SHUTDOWN;
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid value for recovery parameter \"%s\"",
- "action_at_recovery_target"),
+ "recovery_target_action"),
errhint("The allowed values are \"pause\", \"promote\" and \"shutdown\".")));
ereport(DEBUG2,
- (errmsg_internal("action_at_recovery_target = '%s'",
+ (errmsg_internal("recovery_target_action = '%s'",
item->value)));
- actionAtRecoveryTargetSet = true;
+ recoveryTargetActionSet = true;
}
else if (strcmp(item->name, "recovery_target_timeline") == 0)
{
@@ -4902,27 +4882,16 @@ readRecoveryCommandFile(void)
RECOVERY_COMMAND_FILE)));
}
- /*
- * Check for mutually exclusive parameters
- */
- if (recoveryPauseAtTargetSet && actionAtRecoveryTargetSet)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot set both \"%s\" and \"%s\" recovery parameters",
- "pause_at_recovery_target",
- "action_at_recovery_target"),
- errhint("The \"pause_at_recovery_target\" is deprecated.")));
-
/*
- * Override any inconsistent requests. Not that this is a change
+ * Override any inconsistent requests. Note that this is a change
* of behaviour in 9.5; prior to this we simply ignored a request
* to pause if hot_standby = off, which was surprising behaviour.
*/
- if (actionAtRecoveryTarget == RECOVERY_TARGET_ACTION_PAUSE &&
- actionAtRecoveryTargetSet &&
+ if (recoveryTargetAction == RECOVERY_TARGET_ACTION_PAUSE &&
+ recoveryTargetActionSet &&
standbyState == STANDBY_DISABLED)
- actionAtRecoveryTarget = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ recoveryTargetAction = RECOVERY_TARGET_ACTION_SHUTDOWN;
/* Enable fetching from archive recovery area */
ArchiveRecoveryRequested = true;
@@ -6495,7 +6464,7 @@ StartupXLOG(void)
* this, Resource Managers may choose to do permanent corrective
* actions at end of recovery.
*/
- switch (actionAtRecoveryTarget)
+ switch (recoveryTargetAction)
{
case RECOVERY_TARGET_ACTION_SHUTDOWN:
/*
On Mon, Dec 8, 2014 at 9:59 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
Ok this patch does that, along with the rename to recovery_target_action and
addition to the recovery.conf.sample.
This needs a rebase as at least da71632 and b8e33a8 are conflicting.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 08/12/14 02:03, Michael Paquier wrote:
On Mon, Dec 8, 2014 at 9:59 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
Ok this patch does that, along with the rename to recovery_target_action and
addition to the recovery.conf.sample.This needs a rebase as at least da71632 and b8e33a8 are conflicting.
Simon actually already committed something similar, so no need.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 08/12/14 02:06, Petr Jelinek wrote:
On 08/12/14 02:03, Michael Paquier wrote:
On Mon, Dec 8, 2014 at 9:59 AM, Petr Jelinek <petr@2ndquadrant.com>
wrote:Ok this patch does that, along with the rename to
recovery_target_action and
addition to the recovery.conf.sample.This needs a rebase as at least da71632 and b8e33a8 are conflicting.
Simon actually already committed something similar, so no need.
...except for the removal of pause_at_recovery_target it seems, so I
attached just that
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
remove_pause_at_recovery_target.patchtext/x-diff; name=remove_pause_at_recovery_target.patchDownload
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 31473cd..07b4856 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -280,26 +280,6 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</listitem>
</varlistentry>
- <varlistentry id="pause-at-recovery-target"
- xreflabel="pause_at_recovery_target">
- <term><varname>pause_at_recovery_target</varname> (<type>boolean</type>)
- <indexterm>
- <primary><varname>pause_at_recovery_target</> recovery parameter</primary>
- </indexterm>
- </term>
- <listitem>
- <para>
- Alias for recovery_target_action, <literal>true</> is same as
- recovery_target_action = <literal>pause</> and <literal>false</>
- is same as recovery_target_action = <literal>promote</>.
- </para>
- <para>
- This setting has no effect if <xref linkend="guc-hot-standby"> is not
- enabled, or if no recovery target is set.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry id="action-at-recovery-target"
xreflabel="recovery_target_action">
<term><varname>recovery_target_action</varname> (<type>enum</type>)
diff --git a/doc/src/sgml/release-9.1.sgml b/doc/src/sgml/release-9.1.sgml
index 79a8b07..5cbfc4a 100644
--- a/doc/src/sgml/release-9.1.sgml
+++ b/doc/src/sgml/release-9.1.sgml
@@ -6301,8 +6301,8 @@
<listitem>
<para>
- Add <filename>recovery.conf</> setting <link
- linkend="pause-at-recovery-target"><varname>pause_at_recovery_target</></link>
+ Add <filename>recovery.conf</> setting
+ <varname>pause_at_recovery_target</>
to pause recovery at target (Simon Riggs)
</para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 0f09add..6370aed 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4653,7 +4653,6 @@ readRecoveryCommandFile(void)
ConfigVariable *item,
*head = NULL,
*tail = NULL;
- bool recoveryPauseAtTargetSet = false;
bool recoveryTargetActionSet = false;
@@ -4699,25 +4698,6 @@ readRecoveryCommandFile(void)
(errmsg_internal("archive_cleanup_command = '%s'",
archiveCleanupCommand)));
}
- else if (strcmp(item->name, "pause_at_recovery_target") == 0)
- {
- bool recoveryPauseAtTarget;
-
- if (!parse_bool(item->value, &recoveryPauseAtTarget))
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("parameter \"%s\" requires a Boolean value", "pause_at_recovery_target")));
-
- ereport(DEBUG2,
- (errmsg_internal("pause_at_recovery_target = '%s'",
- item->value)));
-
- recoveryTargetAction = recoveryPauseAtTarget ?
- RECOVERY_TARGET_ACTION_PAUSE :
- RECOVERY_TARGET_ACTION_PROMOTE;
-
- recoveryPauseAtTargetSet = true;
- }
else if (strcmp(item->name, "recovery_target_action") == 0)
{
if (strcmp(item->value, "pause") == 0)
@@ -4902,17 +4882,6 @@ readRecoveryCommandFile(void)
RECOVERY_COMMAND_FILE)));
}
- /*
- * Check for mutually exclusive parameters
- */
- if (recoveryPauseAtTargetSet && recoveryTargetActionSet)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot set both \"%s\" and \"%s\" recovery parameters",
- "pause_at_recovery_target",
- "recovery_target_action"),
- errhint("The \"pause_at_recovery_target\" is deprecated.")));
-
/*
* Override any inconsistent requests. Not that this is a change
On Mon, Dec 8, 2014 at 10:18 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
On 08/12/14 02:06, Petr Jelinek wrote:
Simon actually already committed something similar, so no need.
...except for the removal of pause_at_recovery_target it seems, so I
attached just that
Thanks! Removal of this parameter is getting at least two votes, and
nobody expressed against it, so IMO we should remove it now instead or
later, or else I'm sure we would simply forget it. My2c.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-12-08 02:18:11 +0100, Petr Jelinek wrote:
On 08/12/14 02:06, Petr Jelinek wrote:
On 08/12/14 02:03, Michael Paquier wrote:
On Mon, Dec 8, 2014 at 9:59 AM, Petr Jelinek <petr@2ndquadrant.com>
wrote:Ok this patch does that, along with the rename to
recovery_target_action and
addition to the recovery.conf.sample.This needs a rebase as at least da71632 and b8e33a8 are conflicting.
Simon actually already committed something similar, so no need.
...except for the removal of pause_at_recovery_target it seems, so I
attached just that
I intend to push this one unless somebody protests soon.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
On 2014-12-08 02:18:11 +0100, Petr Jelinek wrote:
...except for the removal of pause_at_recovery_target it seems, so I
attached just that
Pushed.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers