Function to promote standby servers
Providing SQL access for administrative tasks seems to be a
good thing, see ALTER SYSTEM and pg_reload_conf().
In that vein, I propose a function pg_promote() to promote
physical standby servers.
If there are no fundamental objections, I'll add it to the
next commitfest.
Yours,
Laurenz Albe
Attachments:
0001-Add-pg_promote-to-promote-standby-servers.patchtext/x-patch; charset=UTF-8; name=0001-Add-pg_promote-to-promote-standby-servers.patchDownload
From dd18d6eb38168db4d3d8c99a74d06b39e719092e Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Thu, 20 Sep 2018 07:52:28 +0200
Subject: [PATCH] Add pg_promote() to promote standby servers
---
doc/src/sgml/func.sgml | 20 +++++++++++
doc/src/sgml/high-availability.sgml | 2 +-
doc/src/sgml/recovery-config.sgml | 3 +-
src/backend/access/transam/xlogfuncs.c | 48 ++++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 4 +++
5 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4331bebc96..7beeaeacde 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18596,6 +18596,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<indexterm>
<primary>pg_terminate_backend</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_promote</primary>
+ </indexterm>
<indexterm>
<primary>signal</primary>
@@ -18655,6 +18658,16 @@ SELECT set_config('log_statement_stats', 'off', false);
however only superusers can terminate superuser backends.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_promote()</function></literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>Promote a physical standby server. This function can only be
+ called by superusers and will only have an effect when run on
+ a standby server.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -18692,6 +18705,13 @@ SELECT set_config('log_statement_stats', 'off', false);
subprocess.
</para>
+ <para>
+ <function>pg_promote()</function> sends a signal to the server that causes it
+ to leave standby mode. Since sending signals is by nature asynchronous,
+ successful execution of the function does not guarantee that the server has
+ already been promoted.
+ </para>
+
</sect2>
<sect2 id="functions-admin-backup">
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index 8cb77f85ec..6014817d9e 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1472,7 +1472,7 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)'
<para>
To trigger failover of a log-shipping standby server,
- run <command>pg_ctl promote</command> or create a trigger
+ run <command>pg_ctl promote</command>, call <function>pg_promote()</function>, or create a trigger
file with the file name and path specified by the <varname>trigger_file</varname>
setting in <filename>recovery.conf</filename>. If you're planning to use
<command>pg_ctl promote</command> to fail over, <varname>trigger_file</varname> is
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 92825fdf19..d06cd0b08e 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -439,7 +439,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<para>
Specifies a trigger file whose presence ends recovery in the
standby. Even if this value is not set, you can still promote
- the standby using <command>pg_ctl promote</command>.
+ the standby using <command>pg_ctl promote</command> or calling
+ <function>pg_promote()</function>.
This setting has no effect if <varname>standby_mode</varname> is <literal>off</literal>.
</para>
</listitem>
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 9731742978..9c55ef619b 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -35,6 +35,7 @@
#include "storage/fd.h"
#include "storage/ipc.h"
+#include <unistd.h>
/*
* Store label file and tablespace map during non-exclusive backups.
@@ -697,3 +698,50 @@ pg_backup_start_time(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(xtime);
}
+
+/*
+ * Promote a standby server.
+ *
+ * A result of "true" means that promotion has been initiated.
+ */
+Datum
+pg_promote(PG_FUNCTION_ARGS)
+{
+ FILE *promote_file;
+
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to promote standby servers")));
+
+ if (!RecoveryInProgress() || !StandbyMode)
+ PG_RETURN_BOOL(false);
+
+ /* create the promote signal file */
+ promote_file = AllocateFile("promote", "w");
+ if (!promote_file)
+ {
+ ereport(WARNING,
+ (errmsg("could not create promote file: %m")));
+ PG_RETURN_BOOL(false);
+ }
+
+ if (FreeFile(promote_file))
+ {
+ /* probably unreachable, but it is better to be safe */
+ ereport(WARNING,
+ (errmsg("could not write promote file: %m")));
+ PG_RETURN_BOOL(false);
+ }
+
+ /* signal the postmaster */
+ if (kill(PostmasterPid, SIGUSR1) != 0)
+ {
+ ereport(WARNING,
+ (errmsg("failed to send signal to postmaster: %m")));
+ (void) unlink("promote");
+ PG_RETURN_BOOL(false);
+ }
+
+ PG_RETURN_BOOL(true);
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 860571440a..a26e5216da 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5997,6 +5997,10 @@
proname => 'pg_backup_start_time', provolatile => 's',
prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_backup_start_time' },
+{ oid => '3436', descr => 'promote standby server',
+ proname => 'pg_promote', provolatile => 'v',
+ prorettype => 'bool', proargtypes => '',
+ prosrc => 'pg_promote' },
{ oid => '2848', descr => 'switch to new wal file',
proname => 'pg_switch_wal', provolatile => 'v', prorettype => 'pg_lsn',
proargtypes => '', prosrc => 'pg_switch_wal' },
--
2.17.1
On Thu, Sep 20, 2018 at 07:59:02AM +0200, Laurenz Albe wrote:
Providing SQL access for administrative tasks seems to be a
good thing, see ALTER SYSTEM and pg_reload_conf().In that vein, I propose a function pg_promote() to promote
physical standby servers.
No fundamental issues from me regarding the concept of being able to
trigger a promotion remotely, so +1. Do we want this capability as well
for fallback_promote? My gut tells me no, and that we ought to drop
this option at some point in the future, still that's worth mentioning.
You may want to move PROMOTE_SIGNAL_FILE in a position where xlogfuncs.c
could use it.
--
Michael
Michael Paquier wrote:
In that vein, I propose a function pg_promote() to promote
physical standby servers.No fundamental issues from me regarding the concept of being able to
trigger a promotion remotely, so +1. Do we want this capability as well
for fallback_promote? My gut tells me no, and that we ought to drop
this option at some point in the future, still that's worth mentioning.You may want to move PROMOTE_SIGNAL_FILE in a position where xlogfuncs.c
could use it.
Good, idea; updated patch attached.
Yours,
Laurenz Albe
Attachments:
0001-Add-pg_promote-to-promote-standby-servers-V2.patchtext/x-patch; charset=UTF-8; name=0001-Add-pg_promote-to-promote-standby-servers-V2.patchDownload
From 63ba6c6f8088bea138e924b4180a08086d339eb5 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Thu, 4 Oct 2018 13:24:03 +0200
Subject: [PATCH] Add pg_promote() to promote standby servers
---
doc/src/sgml/func.sgml | 20 +++++++++++
doc/src/sgml/high-availability.sgml | 2 +-
doc/src/sgml/recovery-config.sgml | 3 +-
src/backend/access/transam/xlog.c | 2 --
src/backend/access/transam/xlogfuncs.c | 48 ++++++++++++++++++++++++++
src/include/access/xlog.h | 4 +++
src/include/catalog/pg_proc.dat | 4 +++
7 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4331bebc96..7beeaeacde 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18596,6 +18596,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<indexterm>
<primary>pg_terminate_backend</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_promote</primary>
+ </indexterm>
<indexterm>
<primary>signal</primary>
@@ -18655,6 +18658,16 @@ SELECT set_config('log_statement_stats', 'off', false);
however only superusers can terminate superuser backends.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_promote()</function></literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>Promote a physical standby server. This function can only be
+ called by superusers and will only have an effect when run on
+ a standby server.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -18692,6 +18705,13 @@ SELECT set_config('log_statement_stats', 'off', false);
subprocess.
</para>
+ <para>
+ <function>pg_promote()</function> sends a signal to the server that causes it
+ to leave standby mode. Since sending signals is by nature asynchronous,
+ successful execution of the function does not guarantee that the server has
+ already been promoted.
+ </para>
+
</sect2>
<sect2 id="functions-admin-backup">
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index 8cb77f85ec..6014817d9e 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1472,7 +1472,7 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)'
<para>
To trigger failover of a log-shipping standby server,
- run <command>pg_ctl promote</command> or create a trigger
+ run <command>pg_ctl promote</command>, call <function>pg_promote()</function>, or create a trigger
file with the file name and path specified by the <varname>trigger_file</varname>
setting in <filename>recovery.conf</filename>. If you're planning to use
<command>pg_ctl promote</command> to fail over, <varname>trigger_file</varname> is
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 92825fdf19..d06cd0b08e 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -439,7 +439,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<para>
Specifies a trigger file whose presence ends recovery in the
standby. Even if this value is not set, you can still promote
- the standby using <command>pg_ctl promote</command>.
+ the standby using <command>pg_ctl promote</command> or calling
+ <function>pg_promote()</function>.
This setting has no effect if <varname>standby_mode</varname> is <literal>off</literal>.
</para>
</listitem>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3025d0badb..bb422e9a13 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -81,8 +81,6 @@ extern uint32 bootstrap_data_checksum_version;
/* File path names (all relative to $PGDATA) */
#define RECOVERY_COMMAND_FILE "recovery.conf"
#define RECOVERY_COMMAND_DONE "recovery.done"
-#define PROMOTE_SIGNAL_FILE "promote"
-#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
/* User-settable parameters */
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 9731742978..ab37b03dee 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -35,6 +35,7 @@
#include "storage/fd.h"
#include "storage/ipc.h"
+#include <unistd.h>
/*
* Store label file and tablespace map during non-exclusive backups.
@@ -697,3 +698,50 @@ pg_backup_start_time(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(xtime);
}
+
+/*
+ * Promote a standby server.
+ *
+ * A result of "true" means that promotion has been initiated.
+ */
+Datum
+pg_promote(PG_FUNCTION_ARGS)
+{
+ FILE *promote_file;
+
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to promote standby servers")));
+
+ if (!RecoveryInProgress() || !StandbyMode)
+ PG_RETURN_BOOL(false);
+
+ /* create the promote signal file */
+ promote_file = AllocateFile(PROMOTE_SIGNAL_FILE, "w");
+ if (!promote_file)
+ {
+ ereport(WARNING,
+ (errmsg("could not create file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ if (FreeFile(promote_file))
+ {
+ /* probably unreachable, but it is better to be safe */
+ ereport(WARNING,
+ (errmsg("could not write to file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ /* signal the postmaster */
+ if (kill(PostmasterPid, SIGUSR1) != 0)
+ {
+ ereport(WARNING,
+ (errmsg("failed to send signal to postmaster: %m")));
+ (void) unlink(PROMOTE_SIGNAL_FILE);
+ PG_RETURN_BOOL(false);
+ }
+
+ PG_RETURN_BOOL(true);
+}
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 421ba6d775..a5a3c59007 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -192,6 +192,10 @@ extern bool XLOG_DEBUG;
#define XLOG_INCLUDE_ORIGIN 0x01 /* include the replication origin */
#define XLOG_MARK_UNIMPORTANT 0x02 /* record not important for durability */
+/* files to signal promotion to primary */
+#define PROMOTE_SIGNAL_FILE "promote"
+#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
+
/* Checkpoint statistics */
typedef struct CheckpointStatsData
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 860571440a..a26e5216da 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5997,6 +5997,10 @@
proname => 'pg_backup_start_time', provolatile => 's',
prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_backup_start_time' },
+{ oid => '3436', descr => 'promote standby server',
+ proname => 'pg_promote', provolatile => 'v',
+ prorettype => 'bool', proargtypes => '',
+ prosrc => 'pg_promote' },
{ oid => '2848', descr => 'switch to new wal file',
proname => 'pg_switch_wal', provolatile => 'v', prorettype => 'pg_lsn',
proargtypes => '', prosrc => 'pg_switch_wal' },
--
2.17.1
On Thu, Oct 4, 2018 at 8:26 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
Michael Paquier wrote:
In that vein, I propose a function pg_promote() to promote
physical standby servers.
+1
No fundamental issues from me regarding the concept of being able to
trigger a promotion remotely, so +1. Do we want this capability as well
for fallback_promote? My gut tells me no, and that we ought to drop
this option at some point in the future, still that's worth mentioning.You may want to move PROMOTE_SIGNAL_FILE in a position where xlogfuncs.c
could use it.Good, idea; updated patch attached.
Maybe the patch needs regression tests for the new function. And I'd
suggest to make the function name more clear by changing to
pg_promote_server(), pg_promote_standby() and so on.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Masahiko Sawada wrote:
Maybe the patch needs regression tests for the new function. And I'd
suggest to make the function name more clear by changing to
pg_promote_server(), pg_promote_standby() and so on.
Thanks for the review.
The attached patch has regression tests - I though it would be good
to change some of the existing tests that run standby promotion to
use the SQL function instead of pg_ctl.
I have left the name though -- as far as I can tell, "promote" has
no other meaning in PostgreSQL than standby promotion, and I believe
it is only good to be more verbose if that avoids confusion.
Yours,
Laurenz Albe
Attachments:
0001-Add-pg_promote-to-promote-standby-servers-V3.patchtext/x-patch; charset=UTF-8; name=0001-Add-pg_promote-to-promote-standby-servers-V3.patchDownload
From a5de6f9893e049bf97810e41530907e237f909d7 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Mon, 8 Oct 2018 17:59:37 +0200
Subject: [PATCH] Add pg_promote() to promote standby servers
---
doc/src/sgml/func.sgml | 20 +++++++++
doc/src/sgml/high-availability.sgml | 2 +-
doc/src/sgml/recovery-config.sgml | 3 +-
src/backend/access/transam/xlog.c | 2 -
src/backend/access/transam/xlogfuncs.c | 48 ++++++++++++++++++++++
src/include/access/xlog.h | 4 ++
src/include/catalog/pg_proc.dat | 4 ++
src/test/perl/PostgresNode.pm | 22 ++++++++++
src/test/recovery/t/004_timeline_switch.pl | 2 +-
src/test/recovery/t/009_twophase.pl | 2 +-
10 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9a7f683658..ae8a9b4ccb 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18731,6 +18731,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<indexterm>
<primary>pg_terminate_backend</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_promote</primary>
+ </indexterm>
<indexterm>
<primary>signal</primary>
@@ -18790,6 +18793,16 @@ SELECT set_config('log_statement_stats', 'off', false);
however only superusers can terminate superuser backends.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_promote()</function></literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>Promote a physical standby server. This function can only be
+ called by superusers and will only have an effect when run on
+ a standby server.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -18827,6 +18840,13 @@ SELECT set_config('log_statement_stats', 'off', false);
subprocess.
</para>
+ <para>
+ <function>pg_promote()</function> sends a signal to the server that causes it
+ to leave standby mode. Since sending signals is by nature asynchronous,
+ successful execution of the function does not guarantee that the server has
+ already been promoted.
+ </para>
+
</sect2>
<sect2 id="functions-admin-backup">
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index 6f57362df7..8ccd1ffcd1 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1472,7 +1472,7 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)'
<para>
To trigger failover of a log-shipping standby server,
- run <command>pg_ctl promote</command> or create a trigger
+ run <command>pg_ctl promote</command>, call <function>pg_promote()</function>, or create a trigger
file with the file name and path specified by the <varname>trigger_file</varname>
setting in <filename>recovery.conf</filename>. If you're planning to use
<command>pg_ctl promote</command> to fail over, <varname>trigger_file</varname> is
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 92825fdf19..d06cd0b08e 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -439,7 +439,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<para>
Specifies a trigger file whose presence ends recovery in the
standby. Even if this value is not set, you can still promote
- the standby using <command>pg_ctl promote</command>.
+ the standby using <command>pg_ctl promote</command> or calling
+ <function>pg_promote()</function>.
This setting has no effect if <varname>standby_mode</varname> is <literal>off</literal>.
</para>
</listitem>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7375a78ffc..3a1f49e83a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -81,8 +81,6 @@ extern uint32 bootstrap_data_checksum_version;
/* File path names (all relative to $PGDATA) */
#define RECOVERY_COMMAND_FILE "recovery.conf"
#define RECOVERY_COMMAND_DONE "recovery.done"
-#define PROMOTE_SIGNAL_FILE "promote"
-#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
/* User-settable parameters */
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 9731742978..b448d0b515 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -35,6 +35,7 @@
#include "storage/fd.h"
#include "storage/ipc.h"
+#include <unistd.h>
/*
* Store label file and tablespace map during non-exclusive backups.
@@ -697,3 +698,50 @@ pg_backup_start_time(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(xtime);
}
+
+/*
+ * Promote a standby server.
+ *
+ * A result of "true" means that promotion has been initiated.
+ */
+Datum
+pg_promote(PG_FUNCTION_ARGS)
+{
+ FILE *promote_file;
+
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be superuser to promote standby servers")));
+
+ if (!RecoveryInProgress())
+ PG_RETURN_BOOL(false);
+
+ /* create the promote signal file */
+ promote_file = AllocateFile(PROMOTE_SIGNAL_FILE, "w");
+ if (!promote_file)
+ {
+ ereport(WARNING,
+ (errmsg("could not create file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ if (FreeFile(promote_file))
+ {
+ /* probably unreachable, but it is better to be safe */
+ ereport(WARNING,
+ (errmsg("could not write to file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ /* signal the postmaster */
+ if (kill(PostmasterPid, SIGUSR1) != 0)
+ {
+ ereport(WARNING,
+ (errmsg("failed to send signal to postmaster: %m")));
+ (void) unlink(PROMOTE_SIGNAL_FILE);
+ PG_RETURN_BOOL(false);
+ }
+
+ PG_RETURN_BOOL(true);
+}
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 421ba6d775..a5a3c59007 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -192,6 +192,10 @@ extern bool XLOG_DEBUG;
#define XLOG_INCLUDE_ORIGIN 0x01 /* include the replication origin */
#define XLOG_MARK_UNIMPORTANT 0x02 /* record not important for durability */
+/* files to signal promotion to primary */
+#define PROMOTE_SIGNAL_FILE "promote"
+#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
+
/* Checkpoint statistics */
typedef struct CheckpointStatsData
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 8e4145f42b..99d494806a 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5997,6 +5997,10 @@
proname => 'pg_backup_start_time', provolatile => 's',
prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_backup_start_time' },
+{ oid => '3436', descr => 'promote standby server',
+ proname => 'pg_promote', provolatile => 'v',
+ prorettype => 'bool', proargtypes => '',
+ prosrc => 'pg_promote' },
{ oid => '2848', descr => 'switch to new wal file',
proname => 'pg_switch_wal', provolatile => 'v', prorettype => 'pg_lsn',
proargtypes => '', prosrc => 'pg_switch_wal' },
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index ae3d8ee10c..4eeff00c92 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -806,6 +806,28 @@ sub promote
=pod
+=item $node->promote_sql()
+
+Wrapper for "SELECT pg_promote()"
+
+=cut
+
+sub promote_sql
+{
+ my ($self) = @_;
+ my $name = $self->name;
+ print "### Promoting node \"$name\"\n";
+ $self->safe_psql('postgres', "SELECT pg_promote()");
+ # pg_promote() is asynchronous, hence we must wait until promotion is complete
+ while ($self->safe_psql('postgres', "SELECT pg_is_in_recovery()") != 'f')
+ {
+ sleep 1;
+ }
+ return;
+}
+
+=pod
+
=item $node->logrotate()
Wrapper for pg_ctl logrotate
diff --git a/src/test/recovery/t/004_timeline_switch.pl b/src/test/recovery/t/004_timeline_switch.pl
index 34ee335129..107ba2b316 100644
--- a/src/test/recovery/t/004_timeline_switch.pl
+++ b/src/test/recovery/t/004_timeline_switch.pl
@@ -39,7 +39,7 @@ $node_master->wait_for_catchup($node_standby_1, 'replay',
# Stop and remove master, and promote standby 1, switching it to a new timeline
$node_master->teardown_node;
-$node_standby_1->promote;
+$node_standby_1->promote_sql;
# Switch standby 2 to replay from standby 1
rmtree($node_standby_2->data_dir . '/recovery.conf');
diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_twophase.pl
index 9ea3bd65fc..197d18dd18 100644
--- a/src/test/recovery/t/009_twophase.pl
+++ b/src/test/recovery/t/009_twophase.pl
@@ -214,7 +214,7 @@ $cur_master->psql(
INSERT INTO t_009_tbl VALUES (22, 'issued to ${cur_master_name}');
PREPARE TRANSACTION 'xact_009_10';");
$cur_master->teardown_node;
-$cur_standby->promote;
+$cur_standby->promote_sql;
# change roles
note "Now paris is master and london is standby";
--
2.17.1
On Mon, Oct 08, 2018 at 07:36:50PM +0200, Laurenz Albe wrote:
The attached patch has regression tests - I though it would be good
to change some of the existing tests that run standby promotion to
use the SQL function instead of pg_ctl.I have left the name though -- as far as I can tell, "promote" has
no other meaning in PostgreSQL than standby promotion, and I believe
it is only good to be more verbose if that avoids confusion.
I am fine with pg_promote.
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 7375a78ffc..3a1f49e83a 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -81,8 +81,6 @@ extern uint32 bootstrap_data_checksum_version; /* File path names (all relative to $PGDATA) */ #define RECOVERY_COMMAND_FILE "recovery.conf" #define RECOVERY_COMMAND_DONE "recovery.done" -#define PROMOTE_SIGNAL_FILE "promote" -#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
Perhaps we could just move the whole set to xlog.h.
+Datum +pg_promote(PG_FUNCTION_ARGS) +{ + FILE *promote_file; + + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to promote standby servers")));
Let's remove this restriction at code level, and instead use
function-level ACLs so as it is possible to allow non-superusers to
trigger a promotion as well, say a dedicated role. We could add a
system role for this purpose, but I am not sure that it is worth the
addition yet.
+ while ($self->safe_psql('postgres', "SELECT pg_is_in_recovery()") != 'f') + { + sleep 1; + } + return;
This could go on infinitely, locking down buildfarm machines silently if
a commit is not careful. What I would suggest to do instead is to not
touch PostgresNode.pm at all, and to just modify one test to trigger
promotion with the SQL function. Then from the test, you should add a
comment that triggerring promotion from SQL is wanted instead of the
internal routine, and then please add a call to poll_query_until() in
the same way as what 6deb52b2 has removed.
As of now, this function returns true if promotion has been triggered,
and false if not. However it seems to me that we should have something
more consistent with "pg_ctl promote", so there are actually three
possible states:
1) Node is in recovery, with promotion triggered. pg_promote() returns
true in case of success in creating the trigger file.
2) Node is in recovery, with promotion triggered. pg_promote() returns
false in case of failure in creating the trigger file.
3) Node is not in recovery, where I think a call to pg_promote should
trigger an error. This is not handled in the patch.
An other thing which has value is to implement a "wait" mode for this
feature, or actually a nowait mode. You could simply do what pg_ctl
does with its logic in get_control_dbstate() by looking at the control
file state. I think that waiting for the promotion to happen should be
the default.
At the end this patch needs a bit more work.
--
Michael
Michael Paquier wrote:
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 7375a78ffc..3a1f49e83a 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -81,8 +81,6 @@ extern uint32 bootstrap_data_checksum_version; /* File path names (all relative to $PGDATA) */ #define RECOVERY_COMMAND_FILE "recovery.conf" #define RECOVERY_COMMAND_DONE "recovery.done" -#define PROMOTE_SIGNAL_FILE "promote" -#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"Perhaps we could just move the whole set to xlog.h.
Done.
+Datum +pg_promote(PG_FUNCTION_ARGS) +{ + FILE *promote_file; + + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to promote standby servers")));Let's remove this restriction at code level, and instead use
function-level ACLs so as it is possible to allow non-superusers to
trigger a promotion as well, say a dedicated role. We could add a
system role for this purpose, but I am not sure that it is worth the
addition yet.
Agreed, done.
+ while ($self->safe_psql('postgres', "SELECT pg_is_in_recovery()") != 'f') + { + sleep 1; + } + return;This could go on infinitely, locking down buildfarm machines silently if
a commit is not careful. What I would suggest to do instead is to not
touch PostgresNode.pm at all, and to just modify one test to trigger
promotion with the SQL function. Then from the test, you should add a
comment that triggerring promotion from SQL is wanted instead of the
internal routine, and then please add a call to poll_query_until() in
the same way as what 6deb52b2 has removed.
I have modified recovery/t/004_timeline_switch.pl and recovery/t/009_twophase.pl
accordingly: one calls the function with "wait => true", the other
uses "wait => false" and waits as you suggested.
As of now, this function returns true if promotion has been triggered,
and false if not. However it seems to me that we should have something
more consistent with "pg_ctl promote", so there are actually three
possible states:
1) Node is in recovery, with promotion triggered. pg_promote() returns
true in case of success in creating the trigger file.
2) Node is in recovery, with promotion triggered. pg_promote() returns
false in case of failure in creating the trigger file.
3) Node is not in recovery, where I think a call to pg_promote should
trigger an error. This is not handled in the patch.
So far I had returned "false" in the last case, but I am fine with
throwing an error in that case. Done.
An other thing which has value is to implement a "wait" mode for this
feature, or actually a nowait mode. You could simply do what pg_ctl
does with its logic in get_control_dbstate() by looking at the control
file state. I think that waiting for the promotion to happen should be
the default.
I have implemented that.
At the end this patch needs a bit more work.
Yes, it did. Thanks for the thorough review!
Yours,
Laurenz Albe
Attachments:
0001-Add-pg_promote-to-promote-standby-servers-V4.patchtext/x-patch; charset=UTF-8; name=0001-Add-pg_promote-to-promote-standby-servers-V4.patchDownload
From 8e45dc7dfbb76eb1625323e3cd8f161779973528 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Mon, 15 Oct 2018 16:07:56 +0200
Subject: [PATCH] Add pg_promote() to promote standby servers
---
doc/src/sgml/func.sgml | 21 +++++++
doc/src/sgml/high-availability.sgml | 2 +-
doc/src/sgml/recovery-config.sgml | 3 +-
src/backend/access/transam/xlog.c | 6 --
src/backend/access/transam/xlogfuncs.c | 66 ++++++++++++++++++++++
src/backend/catalog/system_views.sql | 7 +++
src/include/access/xlog.h | 6 ++
src/include/catalog/pg_proc.dat | 4 ++
src/test/recovery/t/004_timeline_switch.pl | 6 +-
src/test/recovery/t/009_twophase.pl | 6 +-
10 files changed, 116 insertions(+), 11 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9a7f683658..5ecf7f5b9d 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18731,6 +18731,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<indexterm>
<primary>pg_terminate_backend</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_promote</primary>
+ </indexterm>
<indexterm>
<primary>signal</primary>
@@ -18790,6 +18793,16 @@ SELECT set_config('log_statement_stats', 'off', false);
however only superusers can terminate superuser backends.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_promote(<parameter>wait</parameter> <type>boolean</type> DEFAULT true)</function></literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>Promote a physical standby server. This function is restricted to
+ superusers by default, but other users can be granted EXECUTE to run
+ the function.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -18827,6 +18840,14 @@ SELECT set_config('log_statement_stats', 'off', false);
subprocess.
</para>
+ <para>
+ <function>pg_promote</function> can only be called on standby servers.
+ If the argument <parameter>wait</parameter> is <literal>true</literal>,
+ the function waits until promotion is complete or 60 seconds have passed,
+ otherwise the function returns immediately after sending the promotion
+ signal to the postmaster.
+ </para>
+
</sect2>
<sect2 id="functions-admin-backup">
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index 6f57362df7..8ccd1ffcd1 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1472,7 +1472,7 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)'
<para>
To trigger failover of a log-shipping standby server,
- run <command>pg_ctl promote</command> or create a trigger
+ run <command>pg_ctl promote</command>, call <function>pg_promote()</function>, or create a trigger
file with the file name and path specified by the <varname>trigger_file</varname>
setting in <filename>recovery.conf</filename>. If you're planning to use
<command>pg_ctl promote</command> to fail over, <varname>trigger_file</varname> is
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 92825fdf19..d06cd0b08e 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -439,7 +439,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<para>
Specifies a trigger file whose presence ends recovery in the
standby. Even if this value is not set, you can still promote
- the standby using <command>pg_ctl promote</command>.
+ the standby using <command>pg_ctl promote</command> or calling
+ <function>pg_promote()</function>.
This setting has no effect if <varname>standby_mode</varname> is <literal>off</literal>.
</para>
</listitem>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7375a78ffc..62fc418893 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -78,12 +78,6 @@
extern uint32 bootstrap_data_checksum_version;
-/* File path names (all relative to $PGDATA) */
-#define RECOVERY_COMMAND_FILE "recovery.conf"
-#define RECOVERY_COMMAND_DONE "recovery.done"
-#define PROMOTE_SIGNAL_FILE "promote"
-#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
-
/* User-settable parameters */
int max_wal_size_mb = 1024; /* 1 GB */
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 9731742978..7cd5db001e 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -35,6 +35,7 @@
#include "storage/fd.h"
#include "storage/ipc.h"
+#include <unistd.h>
/*
* Store label file and tablespace map during non-exclusive backups.
@@ -697,3 +698,68 @@ pg_backup_start_time(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(xtime);
}
+
+#define WAIT_SECONDS 60
+#define WAITS_PER_SECOND 10
+
+/*
+ * Promote a standby server.
+ *
+ * A result of "true" means that promotion has been initiated.
+ */
+Datum
+pg_promote(PG_FUNCTION_ARGS)
+{
+ bool wait = PG_GETARG_BOOL(0);
+ FILE *promote_file;
+ int i;
+
+ if (!RecoveryInProgress())
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("recovery is not in progress"),
+ errhint("Only a server that is in recovery can be promoted.")));
+
+ /* create the promote signal file */
+ promote_file = AllocateFile(PROMOTE_SIGNAL_FILE, "w");
+ if (!promote_file)
+ {
+ ereport(WARNING,
+ (errmsg("could not create file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ if (FreeFile(promote_file))
+ {
+ /* probably unreachable, but it is better to be safe */
+ ereport(WARNING,
+ (errmsg("could not write to file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ /* signal the postmaster */
+ if (kill(PostmasterPid, SIGUSR1) != 0)
+ {
+ ereport(WARNING,
+ (errmsg("failed to send signal to postmaster: %m")));
+ (void) unlink(PROMOTE_SIGNAL_FILE);
+ PG_RETURN_BOOL(false);
+ }
+
+ /* return immediately if waiting was not requested */
+ if (wait)
+ PG_RETURN_BOOL(true);
+
+ /* wait for up to a minute for promotion */
+ for (i = 0; i < WAITS_PER_SECOND * WAIT_SECONDS; ++i)
+ {
+ if (!RecoveryInProgress())
+ PG_RETURN_BOOL(true);
+
+ pg_usleep(1000000L / WAITS_PER_SECOND);
+ }
+
+ ereport(WARNING,
+ (errmsg("server did not promote within %d seconds", WAIT_SECONDS)));
+ PG_RETURN_BOOL(false);
+}
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7251552419..9c9cf84c10 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1119,6 +1119,13 @@ LANGUAGE INTERNAL
STRICT IMMUTABLE PARALLEL SAFE
AS 'jsonb_insert';
+CREATE OR REPLACE FUNCTION
+ pg_promote(wait boolean DEFAULT true)
+RETURNS boolean
+LANGUAGE INTERNAL
+STRICT VOLATILE
+AS 'pg_promote';
+
--
-- The default permissions for functions mean that anyone can execute them.
-- A number of functions shouldn't be executable by just anyone, but rather
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 421ba6d775..e01d12eb7c 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -319,10 +319,16 @@ extern void do_pg_abort_backup(void);
extern SessionBackupState get_backup_status(void);
/* File path names (all relative to $PGDATA) */
+#define RECOVERY_COMMAND_FILE "recovery.conf"
+#define RECOVERY_COMMAND_DONE "recovery.done"
#define BACKUP_LABEL_FILE "backup_label"
#define BACKUP_LABEL_OLD "backup_label.old"
#define TABLESPACE_MAP "tablespace_map"
#define TABLESPACE_MAP_OLD "tablespace_map.old"
+/* files to signal promotion to primary */
+#define PROMOTE_SIGNAL_FILE "promote"
+#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
+
#endif /* XLOG_H */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 8e4145f42b..1598f930a4 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5997,6 +5997,10 @@
proname => 'pg_backup_start_time', provolatile => 's',
prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_backup_start_time' },
+{ oid => '3436', descr => 'promote standby server',
+ proname => 'pg_promote', provolatile => 'v',
+ prorettype => 'bool', proargtypes => 'bool', proargnames => '{wait}',
+ prosrc => 'pg_promote' },
{ oid => '2848', descr => 'switch to new wal file',
proname => 'pg_switch_wal', provolatile => 'v', prorettype => 'pg_lsn',
proargtypes => '', prosrc => 'pg_switch_wal' },
diff --git a/src/test/recovery/t/004_timeline_switch.pl b/src/test/recovery/t/004_timeline_switch.pl
index 34ee335129..fd440ca9dd 100644
--- a/src/test/recovery/t/004_timeline_switch.pl
+++ b/src/test/recovery/t/004_timeline_switch.pl
@@ -37,9 +37,11 @@ $node_master->safe_psql('postgres',
$node_master->wait_for_catchup($node_standby_1, 'replay',
$node_master->lsn('write'));
-# Stop and remove master, and promote standby 1, switching it to a new timeline
+# Stop and remove master
$node_master->teardown_node;
-$node_standby_1->promote;
+
+# promote standby 1 using "pg_promote", switching it to a new timeline
+$node_standby_1->safe_psql('postgres', "SELECT pg_promote()");
# Switch standby 2 to replay from standby 1
rmtree($node_standby_2->data_dir . '/recovery.conf');
diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_twophase.pl
index 9ea3bd65fc..a6044179bc 100644
--- a/src/test/recovery/t/009_twophase.pl
+++ b/src/test/recovery/t/009_twophase.pl
@@ -214,7 +214,11 @@ $cur_master->psql(
INSERT INTO t_009_tbl VALUES (22, 'issued to ${cur_master_name}');
PREPARE TRANSACTION 'xact_009_10';");
$cur_master->teardown_node;
-$cur_standby->promote;
+
+# promote standby using "pg_promote" and wait until it is promoted
+$cur_standby->safe_psql('postgres', 'SELECT pg_promote(FALSE)');
+$cur_standby->poll_query_until('postgres', "SELECT NOT pg_is_in_recovery()")
+ or die "standby never exited recovery";
# change roles
note "Now paris is master and london is standby";
--
2.17.2
Just curious to know, is promotion via function only allowed for
hot-standby or works for any standby?
On Mon, Oct 15, 2018 at 7:16 AM Laurenz Albe <laurenz.albe@cybertec.at>
wrote:
Show quoted text
Michael Paquier wrote:
diff --git a/src/backend/access/transam/xlog.cb/src/backend/access/transam/xlog.c
index 7375a78ffc..3a1f49e83a 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -81,8 +81,6 @@ extern uint32 bootstrap_data_checksum_version; /* File path names (all relative to $PGDATA) */ #define RECOVERY_COMMAND_FILE "recovery.conf" #define RECOVERY_COMMAND_DONE "recovery.done" -#define PROMOTE_SIGNAL_FILE "promote" -#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"Perhaps we could just move the whole set to xlog.h.
Done.
+Datum +pg_promote(PG_FUNCTION_ARGS) +{ + FILE *promote_file; + + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to promotestandby servers")));
Let's remove this restriction at code level, and instead use
function-level ACLs so as it is possible to allow non-superusers to
trigger a promotion as well, say a dedicated role. We could add a
system role for this purpose, but I am not sure that it is worth the
addition yet.Agreed, done.
+ while ($self->safe_psql('postgres', "SELECT pg_is_in_recovery()")
!= 'f')
+ { + sleep 1; + } + return;This could go on infinitely, locking down buildfarm machines silently if
a commit is not careful. What I would suggest to do instead is to not
touch PostgresNode.pm at all, and to just modify one test to trigger
promotion with the SQL function. Then from the test, you should add a
comment that triggerring promotion from SQL is wanted instead of the
internal routine, and then please add a call to poll_query_until() in
the same way as what 6deb52b2 has removed.I have modified recovery/t/004_timeline_switch.pl and recovery/t/
009_twophase.pl
accordingly: one calls the function with "wait => true", the other
uses "wait => false" and waits as you suggested.As of now, this function returns true if promotion has been triggered,
and false if not. However it seems to me that we should have something
more consistent with "pg_ctl promote", so there are actually three
possible states:
1) Node is in recovery, with promotion triggered. pg_promote() returns
true in case of success in creating the trigger file.
2) Node is in recovery, with promotion triggered. pg_promote() returns
false in case of failure in creating the trigger file.
3) Node is not in recovery, where I think a call to pg_promote should
trigger an error. This is not handled in the patch.So far I had returned "false" in the last case, but I am fine with
throwing an error in that case. Done.An other thing which has value is to implement a "wait" mode for this
feature, or actually a nowait mode. You could simply do what pg_ctl
does with its logic in get_control_dbstate() by looking at the control
file state. I think that waiting for the promotion to happen should be
the default.I have implemented that.
At the end this patch needs a bit more work.
Yes, it did. Thanks for the thorough review!
Yours,
Laurenz Albe
Ashwin Agrawal wrote:
Just curious to know, is promotion via function only allowed for
hot-standby or works for any standby?
Only for hot standby - how do you want to execute a function on a standby
server that doesn't allow connections?
Yours,
Laurenz Albe
On Tue, Oct 16, 2018 at 09:49:20AM +0200, Laurenz Albe wrote:
Only for hot standby - how do you want to execute a function on a standby
server that doesn't allow connections?
In most deployments hot standby will be enabled, and wal_level uses the
same value for archive and hot_standby for some time now, so that does
not sound like a huge issue to me.
--
Michael
On Mon, Oct 15, 2018 at 04:16:02PM +0200, Laurenz Albe wrote:
Michael Paquier wrote:
Let's remove this restriction at code level, and instead use
function-level ACLs so as it is possible to allow non-superusers to
trigger a promotion as well, say a dedicated role. We could add a
system role for this purpose, but I am not sure that it is worth the
addition yet.Agreed, done.
As of now, this function returns true if promotion has been triggered,
and false if not. However it seems to me that we should have something
more consistent with "pg_ctl promote", so there are actually three
possible states:
1) Node is in recovery, with promotion triggered. pg_promote() returns
true in case of success in creating the trigger file.
2) Node is in recovery, with promotion triggered. pg_promote() returns
false in case of failure in creating the trigger file.
3) Node is not in recovery, where I think a call to pg_promote should
trigger an error. This is not handled in the patch.So far I had returned "false" in the last case, but I am fine with
throwing an error in that case. Done.
Thanks, that looks correct to me. I think that consistency with pg_ctl
is quite important, as this is a feature present for many releases now.
At the end this patch needs a bit more work.
Yes, it did. Thanks for the thorough review!
+ /* wait for up to a minute for promotion */
+ for (i = 0; i < WAITS_PER_SECOND * WAIT_SECONDS; ++i)
+ {
+ if (!RecoveryInProgress())
+ PG_RETURN_BOOL(true);
+
+ pg_usleep(1000000L / WAITS_PER_SECOND);
+ }
I would recommend to avoid pg_usleep and instead use a WaitLatch() or
similar to generate a wait event. The wait can then also be seen in
pg_stat_activity, which is useful for monitoring purposes. Using
RecoveryInProgress is indeed doable, and that's more simple than what I
thought first.
Something I missed to mention in the previous review: the timeout should
be manually enforceable, with a default at 60s.
Is the function marked as strict? Per the code it should be, I am not
able to test now though.
You are missing REVOKE EXECUTE ON FUNCTION pg_promote() in
system_views.sql, or any users could trigger a promotion, no?
--
Michael
Michael Paquier wrote:
+ /* wait for up to a minute for promotion */ + for (i = 0; i < WAITS_PER_SECOND * WAIT_SECONDS; ++i) + { + if (!RecoveryInProgress()) + PG_RETURN_BOOL(true); + + pg_usleep(1000000L / WAITS_PER_SECOND); + } I would recommend to avoid pg_usleep and instead use a WaitLatch() or similar to generate a wait event. The wait can then also be seen in pg_stat_activity, which is useful for monitoring purposes. Using RecoveryInProgress is indeed doable, and that's more simple than what I thought first.
Agreed, done.
I have introduced a new wait event, because I couldn't find one that fit.
Something I missed to mention in the previous review: the timeout should
be manually enforceable, with a default at 60s.
Ok, added as a new parameter "wait_seconds".
Is the function marked as strict? Per the code it should be, I am not
able to test now though.
Yes, it is.
You are missing REVOKE EXECUTE ON FUNCTION pg_promote() in
system_views.sql, or any users could trigger a promotion, no?
You are right *blush*.
Fixed.
Yours,
Laurenz Albe
Attachments:
0001-Add-pg_promote-to-promote-standby-servers-V5.patchtext/x-patch; charset=UTF-8; name=0001-Add-pg_promote-to-promote-standby-servers-V5.patchDownload
From 08951fea7c526450d9a632ef0e6e246dd9dba307 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Fri, 19 Oct 2018 13:24:29 +0200
Subject: [PATCH] Add pg_promote() to promote standby servers
---
doc/src/sgml/func.sgml | 21 ++++++
doc/src/sgml/high-availability.sgml | 2 +-
doc/src/sgml/recovery-config.sgml | 3 +-
src/backend/access/transam/xlog.c | 6 --
src/backend/access/transam/xlogfuncs.c | 82 ++++++++++++++++++++++
src/backend/catalog/system_views.sql | 8 +++
src/backend/postmaster/pgstat.c | 3 +
src/include/access/xlog.h | 6 ++
src/include/catalog/pg_proc.dat | 4 ++
src/include/pgstat.h | 3 +-
src/test/recovery/t/004_timeline_switch.pl | 6 +-
src/test/recovery/t/009_twophase.pl | 6 +-
12 files changed, 138 insertions(+), 12 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 5193df3366..88121cdc66 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18731,6 +18731,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<indexterm>
<primary>pg_terminate_backend</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_promote</primary>
+ </indexterm>
<indexterm>
<primary>signal</primary>
@@ -18790,6 +18793,16 @@ SELECT set_config('log_statement_stats', 'off', false);
however only superusers can terminate superuser backends.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_promote(<parameter>wait</parameter> <type>boolean</type> DEFAULT true, <parameter>wait_seconds</parameter> <type>integer</type> DEFAULT 60)</function></literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>Promote a physical standby server. This function is restricted to
+ superusers by default, but other users can be granted EXECUTE to run
+ the function.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -18827,6 +18840,14 @@ SELECT set_config('log_statement_stats', 'off', false);
subprocess.
</para>
+ <para>
+ <function>pg_promote</function> can only be called on standby servers.
+ If the argument <parameter>wait</parameter> is <literal>true</literal>,
+ the function waits until promotion is complete or <parameter>wait_seconds</parameter>
+ seconds have passed, otherwise the function returns immediately after sending
+ the promotion signal to the postmaster.
+ </para>
+
</sect2>
<sect2 id="functions-admin-backup">
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index ebcb3daaed..f8e036965c 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1472,7 +1472,7 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)'
<para>
To trigger failover of a log-shipping standby server,
- run <command>pg_ctl promote</command> or create a trigger
+ run <command>pg_ctl promote</command>, call <function>pg_promote()</function>, or create a trigger
file with the file name and path specified by the <varname>trigger_file</varname>
setting in <filename>recovery.conf</filename>. If you're planning to use
<command>pg_ctl promote</command> to fail over, <varname>trigger_file</varname> is
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 92825fdf19..d06cd0b08e 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -439,7 +439,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<para>
Specifies a trigger file whose presence ends recovery in the
standby. Even if this value is not set, you can still promote
- the standby using <command>pg_ctl promote</command>.
+ the standby using <command>pg_ctl promote</command> or calling
+ <function>pg_promote()</function>.
This setting has no effect if <varname>standby_mode</varname> is <literal>off</literal>.
</para>
</listitem>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7375a78ffc..62fc418893 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -78,12 +78,6 @@
extern uint32 bootstrap_data_checksum_version;
-/* File path names (all relative to $PGDATA) */
-#define RECOVERY_COMMAND_FILE "recovery.conf"
-#define RECOVERY_COMMAND_DONE "recovery.done"
-#define PROMOTE_SIGNAL_FILE "promote"
-#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
-
/* User-settable parameters */
int max_wal_size_mb = 1024; /* 1 GB */
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 9731742978..35f817786b 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -23,6 +23,7 @@
#include "catalog/pg_type.h"
#include "funcapi.h"
#include "miscadmin.h"
+#include "pgstat.h"
#include "replication/walreceiver.h"
#include "storage/smgr.h"
#include "utils/builtins.h"
@@ -35,6 +36,7 @@
#include "storage/fd.h"
#include "storage/ipc.h"
+#include <unistd.h>
/*
* Store label file and tablespace map during non-exclusive backups.
@@ -697,3 +699,83 @@ pg_backup_start_time(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(xtime);
}
+
+/*
+ * Promote a standby server.
+ *
+ * A result of "true" means that promotion has been initiated.
+ */
+Datum
+pg_promote(PG_FUNCTION_ARGS)
+{
+ bool wait = PG_GETARG_BOOL(0);
+ int wait_seconds = PG_GETARG_INT32(1);
+ FILE *promote_file;
+ int i;
+
+ if (!RecoveryInProgress())
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("recovery is not in progress"),
+ errhint("Only a server that is in recovery can be promoted.")));
+
+ if (wait_seconds < 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("\"wait_seconds\" cannot be negative")));
+
+ /* create the promote signal file */
+ promote_file = AllocateFile(PROMOTE_SIGNAL_FILE, "w");
+ if (!promote_file)
+ {
+ ereport(WARNING,
+ (errmsg("could not create file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ if (FreeFile(promote_file))
+ {
+ /* probably unreachable, but it is better to be safe */
+ ereport(WARNING,
+ (errmsg("could not write to file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ /* signal the postmaster */
+ if (kill(PostmasterPid, SIGUSR1) != 0)
+ {
+ ereport(WARNING,
+ (errmsg("failed to send signal to postmaster: %m")));
+ (void) unlink(PROMOTE_SIGNAL_FILE);
+ PG_RETURN_BOOL(false);
+ }
+
+ /* return immediately if waiting was not requested */
+ if (!wait || wait_seconds == 0)
+ PG_RETURN_BOOL(true);
+
+ /* wait for up to a minute for promotion */
+#define WAITS_PER_SECOND 10
+ for (i = 0; i < WAITS_PER_SECOND * wait_seconds; ++i)
+ {
+ int rc;
+
+ if (!RecoveryInProgress())
+ PG_RETURN_BOOL(true);
+
+ CHECK_FOR_INTERRUPTS();
+
+ rc = WaitLatch(MyLatch,
+ WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
+ 1000L / WAITS_PER_SECOND,
+ WAIT_EVENT_PROMOTE);
+ ResetLatch(MyLatch);
+
+ if (!(rc & WL_TIMEOUT))
+ PG_RETURN_BOOL(false);
+ }
+
+ ereport(WARNING,
+ (errmsg("server did not promote within %d seconds", wait_seconds)));
+ PG_RETURN_BOOL(false);
+}
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index a03b005f73..2857d80984 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1119,6 +1119,13 @@ LANGUAGE INTERNAL
STRICT IMMUTABLE PARALLEL SAFE
AS 'jsonb_insert';
+CREATE OR REPLACE FUNCTION
+ pg_promote(wait boolean DEFAULT true, wait_seconds integer DEFAULT 60)
+RETURNS boolean
+LANGUAGE INTERNAL
+STRICT VOLATILE
+AS 'pg_promote';
+
--
-- The default permissions for functions mean that anyone can execute them.
-- A number of functions shouldn't be executable by just anyone, but rather
@@ -1138,6 +1145,7 @@ REVOKE EXECUTE ON FUNCTION pg_rotate_logfile() FROM public;
REVOKE EXECUTE ON FUNCTION pg_reload_conf() FROM public;
REVOKE EXECUTE ON FUNCTION pg_current_logfile() FROM public;
REVOKE EXECUTE ON FUNCTION pg_current_logfile(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_promote(boolean, integer) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_reset() FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_reset_shared(text) FROM public;
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 8a5b2b3b42..bec84c8b55 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3668,6 +3668,9 @@ pgstat_get_wait_ipc(WaitEventIPC w)
case WAIT_EVENT_PROCARRAY_GROUP_UPDATE:
event_name = "ProcArrayGroupUpdate";
break;
+ case WAIT_EVENT_PROMOTE:
+ event_name = "Promote";
+ break;
case WAIT_EVENT_CLOG_GROUP_UPDATE:
event_name = "ClogGroupUpdate";
break;
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 421ba6d775..e01d12eb7c 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -319,10 +319,16 @@ extern void do_pg_abort_backup(void);
extern SessionBackupState get_backup_status(void);
/* File path names (all relative to $PGDATA) */
+#define RECOVERY_COMMAND_FILE "recovery.conf"
+#define RECOVERY_COMMAND_DONE "recovery.done"
#define BACKUP_LABEL_FILE "backup_label"
#define BACKUP_LABEL_OLD "backup_label.old"
#define TABLESPACE_MAP "tablespace_map"
#define TABLESPACE_MAP_OLD "tablespace_map.old"
+/* files to signal promotion to primary */
+#define PROMOTE_SIGNAL_FILE "promote"
+#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
+
#endif /* XLOG_H */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index cff58ed2d8..25f1303a83 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5824,6 +5824,10 @@
proname => 'pg_backup_start_time', provolatile => 's',
prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_backup_start_time' },
+{ oid => '3436', descr => 'promote standby server',
+ proname => 'pg_promote', provolatile => 'v',
+ prorettype => 'bool', proargtypes => 'bool int4', proargnames => '{wait}',
+ prosrc => 'pg_promote' },
{ oid => '2848', descr => 'switch to new wal file',
proname => 'pg_switch_wal', provolatile => 'v', prorettype => 'pg_lsn',
proargtypes => '', prosrc => 'pg_switch_wal' },
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index d59c24ae23..d8d73fec21 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -832,7 +832,8 @@ typedef enum
WAIT_EVENT_REPLICATION_ORIGIN_DROP,
WAIT_EVENT_REPLICATION_SLOT_DROP,
WAIT_EVENT_SAFE_SNAPSHOT,
- WAIT_EVENT_SYNC_REP
+ WAIT_EVENT_SYNC_REP,
+ WAIT_EVENT_PROMOTE
} WaitEventIPC;
/* ----------
diff --git a/src/test/recovery/t/004_timeline_switch.pl b/src/test/recovery/t/004_timeline_switch.pl
index 34ee335129..fd440ca9dd 100644
--- a/src/test/recovery/t/004_timeline_switch.pl
+++ b/src/test/recovery/t/004_timeline_switch.pl
@@ -37,9 +37,11 @@ $node_master->safe_psql('postgres',
$node_master->wait_for_catchup($node_standby_1, 'replay',
$node_master->lsn('write'));
-# Stop and remove master, and promote standby 1, switching it to a new timeline
+# Stop and remove master
$node_master->teardown_node;
-$node_standby_1->promote;
+
+# promote standby 1 using "pg_promote", switching it to a new timeline
+$node_standby_1->safe_psql('postgres', "SELECT pg_promote()");
# Switch standby 2 to replay from standby 1
rmtree($node_standby_2->data_dir . '/recovery.conf');
diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_twophase.pl
index 9ea3bd65fc..a6044179bc 100644
--- a/src/test/recovery/t/009_twophase.pl
+++ b/src/test/recovery/t/009_twophase.pl
@@ -214,7 +214,11 @@ $cur_master->psql(
INSERT INTO t_009_tbl VALUES (22, 'issued to ${cur_master_name}');
PREPARE TRANSACTION 'xact_009_10';");
$cur_master->teardown_node;
-$cur_standby->promote;
+
+# promote standby using "pg_promote" and wait until it is promoted
+$cur_standby->safe_psql('postgres', 'SELECT pg_promote(FALSE)');
+$cur_standby->poll_query_until('postgres', "SELECT NOT pg_is_in_recovery()")
+ or die "standby never exited recovery";
# change roles
note "Now paris is master and london is standby";
--
2.17.2
I wrote:
Fixed.
Here is another version, with a fix in pg_proc.dat, an improved comment
and "wait_seconds" exercised in the regression test.
Yours,
Laurenz Albe
Attachments:
0001-Add-pg_promote-to-promote-standby-servers-V6.patchtext/x-patch; charset=UTF-8; name=0001-Add-pg_promote-to-promote-standby-servers-V6.patchDownload
From a2a7f9fd1b23ad102d11992b22158dab8b5451d5 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Sat, 20 Oct 2018 06:21:00 +0200
Subject: [PATCH] Add pg_promote() to promote standby servers
---
doc/src/sgml/func.sgml | 21 ++++++
doc/src/sgml/high-availability.sgml | 2 +-
doc/src/sgml/recovery-config.sgml | 3 +-
src/backend/access/transam/xlog.c | 6 --
src/backend/access/transam/xlogfuncs.c | 83 ++++++++++++++++++++++
src/backend/catalog/system_views.sql | 8 +++
src/backend/postmaster/pgstat.c | 3 +
src/include/access/xlog.h | 6 ++
src/include/catalog/pg_proc.dat | 4 ++
src/include/pgstat.h | 3 +-
src/test/recovery/t/004_timeline_switch.pl | 6 +-
src/test/recovery/t/009_twophase.pl | 6 +-
12 files changed, 139 insertions(+), 12 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 5193df3366..88121cdc66 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18731,6 +18731,9 @@ SELECT set_config('log_statement_stats', 'off', false);
<indexterm>
<primary>pg_terminate_backend</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_promote</primary>
+ </indexterm>
<indexterm>
<primary>signal</primary>
@@ -18790,6 +18793,16 @@ SELECT set_config('log_statement_stats', 'off', false);
however only superusers can terminate superuser backends.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_promote(<parameter>wait</parameter> <type>boolean</type> DEFAULT true, <parameter>wait_seconds</parameter> <type>integer</type> DEFAULT 60)</function></literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>Promote a physical standby server. This function is restricted to
+ superusers by default, but other users can be granted EXECUTE to run
+ the function.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
@@ -18827,6 +18840,14 @@ SELECT set_config('log_statement_stats', 'off', false);
subprocess.
</para>
+ <para>
+ <function>pg_promote</function> can only be called on standby servers.
+ If the argument <parameter>wait</parameter> is <literal>true</literal>,
+ the function waits until promotion is complete or <parameter>wait_seconds</parameter>
+ seconds have passed, otherwise the function returns immediately after sending
+ the promotion signal to the postmaster.
+ </para>
+
</sect2>
<sect2 id="functions-admin-backup">
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index ebcb3daaed..f8e036965c 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1472,7 +1472,7 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)'
<para>
To trigger failover of a log-shipping standby server,
- run <command>pg_ctl promote</command> or create a trigger
+ run <command>pg_ctl promote</command>, call <function>pg_promote()</function>, or create a trigger
file with the file name and path specified by the <varname>trigger_file</varname>
setting in <filename>recovery.conf</filename>. If you're planning to use
<command>pg_ctl promote</command> to fail over, <varname>trigger_file</varname> is
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 92825fdf19..d06cd0b08e 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -439,7 +439,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<para>
Specifies a trigger file whose presence ends recovery in the
standby. Even if this value is not set, you can still promote
- the standby using <command>pg_ctl promote</command>.
+ the standby using <command>pg_ctl promote</command> or calling
+ <function>pg_promote()</function>.
This setting has no effect if <varname>standby_mode</varname> is <literal>off</literal>.
</para>
</listitem>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7375a78ffc..62fc418893 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -78,12 +78,6 @@
extern uint32 bootstrap_data_checksum_version;
-/* File path names (all relative to $PGDATA) */
-#define RECOVERY_COMMAND_FILE "recovery.conf"
-#define RECOVERY_COMMAND_DONE "recovery.done"
-#define PROMOTE_SIGNAL_FILE "promote"
-#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
-
/* User-settable parameters */
int max_wal_size_mb = 1024; /* 1 GB */
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 9731742978..e97d1b63bc 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -23,6 +23,7 @@
#include "catalog/pg_type.h"
#include "funcapi.h"
#include "miscadmin.h"
+#include "pgstat.h"
#include "replication/walreceiver.h"
#include "storage/smgr.h"
#include "utils/builtins.h"
@@ -35,6 +36,7 @@
#include "storage/fd.h"
#include "storage/ipc.h"
+#include <unistd.h>
/*
* Store label file and tablespace map during non-exclusive backups.
@@ -697,3 +699,84 @@ pg_backup_start_time(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(xtime);
}
+
+/*
+ * Promote a standby server.
+ *
+ * A result of "true" means that promotion has been completed
+ * (or initiated if "wait" is false).
+ */
+Datum
+pg_promote(PG_FUNCTION_ARGS)
+{
+ bool wait = PG_GETARG_BOOL(0);
+ int wait_seconds = PG_GETARG_INT32(1);
+ FILE *promote_file;
+ int i;
+
+ if (!RecoveryInProgress())
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("recovery is not in progress"),
+ errhint("Only a server that is in recovery can be promoted.")));
+
+ if (wait_seconds < 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("\"wait_seconds\" cannot be negative")));
+
+ /* create the promote signal file */
+ promote_file = AllocateFile(PROMOTE_SIGNAL_FILE, "w");
+ if (!promote_file)
+ {
+ ereport(WARNING,
+ (errmsg("could not create file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ if (FreeFile(promote_file))
+ {
+ /* probably unreachable, but it is better to be safe */
+ ereport(WARNING,
+ (errmsg("could not write to file \"%s\": %m", PROMOTE_SIGNAL_FILE)));
+ PG_RETURN_BOOL(false);
+ }
+
+ /* signal the postmaster */
+ if (kill(PostmasterPid, SIGUSR1) != 0)
+ {
+ ereport(WARNING,
+ (errmsg("failed to send signal to postmaster: %m")));
+ (void) unlink(PROMOTE_SIGNAL_FILE);
+ PG_RETURN_BOOL(false);
+ }
+
+ /* return immediately if waiting was not requested */
+ if (!wait || wait_seconds == 0)
+ PG_RETURN_BOOL(true);
+
+ /* wait for up to a minute for promotion */
+#define WAITS_PER_SECOND 10
+ for (i = 0; i < WAITS_PER_SECOND * wait_seconds; ++i)
+ {
+ int rc;
+
+ if (!RecoveryInProgress())
+ PG_RETURN_BOOL(true);
+
+ CHECK_FOR_INTERRUPTS();
+
+ rc = WaitLatch(MyLatch,
+ WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
+ 1000L / WAITS_PER_SECOND,
+ WAIT_EVENT_PROMOTE);
+ ResetLatch(MyLatch);
+
+ if (!(rc & WL_TIMEOUT))
+ PG_RETURN_BOOL(false);
+ }
+
+ ereport(WARNING,
+ (errmsg("server did not promote within %d seconds", wait_seconds)));
+ PG_RETURN_BOOL(false);
+}
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index a03b005f73..2857d80984 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1119,6 +1119,13 @@ LANGUAGE INTERNAL
STRICT IMMUTABLE PARALLEL SAFE
AS 'jsonb_insert';
+CREATE OR REPLACE FUNCTION
+ pg_promote(wait boolean DEFAULT true, wait_seconds integer DEFAULT 60)
+RETURNS boolean
+LANGUAGE INTERNAL
+STRICT VOLATILE
+AS 'pg_promote';
+
--
-- The default permissions for functions mean that anyone can execute them.
-- A number of functions shouldn't be executable by just anyone, but rather
@@ -1138,6 +1145,7 @@ REVOKE EXECUTE ON FUNCTION pg_rotate_logfile() FROM public;
REVOKE EXECUTE ON FUNCTION pg_reload_conf() FROM public;
REVOKE EXECUTE ON FUNCTION pg_current_logfile() FROM public;
REVOKE EXECUTE ON FUNCTION pg_current_logfile(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_promote(boolean, integer) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_reset() FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_reset_shared(text) FROM public;
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 8a5b2b3b42..bec84c8b55 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3668,6 +3668,9 @@ pgstat_get_wait_ipc(WaitEventIPC w)
case WAIT_EVENT_PROCARRAY_GROUP_UPDATE:
event_name = "ProcArrayGroupUpdate";
break;
+ case WAIT_EVENT_PROMOTE:
+ event_name = "Promote";
+ break;
case WAIT_EVENT_CLOG_GROUP_UPDATE:
event_name = "ClogGroupUpdate";
break;
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 421ba6d775..e01d12eb7c 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -319,10 +319,16 @@ extern void do_pg_abort_backup(void);
extern SessionBackupState get_backup_status(void);
/* File path names (all relative to $PGDATA) */
+#define RECOVERY_COMMAND_FILE "recovery.conf"
+#define RECOVERY_COMMAND_DONE "recovery.done"
#define BACKUP_LABEL_FILE "backup_label"
#define BACKUP_LABEL_OLD "backup_label.old"
#define TABLESPACE_MAP "tablespace_map"
#define TABLESPACE_MAP_OLD "tablespace_map.old"
+/* files to signal promotion to primary */
+#define PROMOTE_SIGNAL_FILE "promote"
+#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
+
#endif /* XLOG_H */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index cff58ed2d8..4d7fe1b383 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5824,6 +5824,10 @@
proname => 'pg_backup_start_time', provolatile => 's',
prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_backup_start_time' },
+{ oid => '3436', descr => 'promote standby server',
+ proname => 'pg_promote', provolatile => 'v',
+ prorettype => 'bool', proargtypes => 'bool int4', proargnames => '{wait,wait_seconds}',
+ prosrc => 'pg_promote' },
{ oid => '2848', descr => 'switch to new wal file',
proname => 'pg_switch_wal', provolatile => 'v', prorettype => 'pg_lsn',
proargtypes => '', prosrc => 'pg_switch_wal' },
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index d59c24ae23..d8d73fec21 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -832,7 +832,8 @@ typedef enum
WAIT_EVENT_REPLICATION_ORIGIN_DROP,
WAIT_EVENT_REPLICATION_SLOT_DROP,
WAIT_EVENT_SAFE_SNAPSHOT,
- WAIT_EVENT_SYNC_REP
+ WAIT_EVENT_SYNC_REP,
+ WAIT_EVENT_PROMOTE
} WaitEventIPC;
/* ----------
diff --git a/src/test/recovery/t/004_timeline_switch.pl b/src/test/recovery/t/004_timeline_switch.pl
index 34ee335129..1775b9eb16 100644
--- a/src/test/recovery/t/004_timeline_switch.pl
+++ b/src/test/recovery/t/004_timeline_switch.pl
@@ -37,9 +37,11 @@ $node_master->safe_psql('postgres',
$node_master->wait_for_catchup($node_standby_1, 'replay',
$node_master->lsn('write'));
-# Stop and remove master, and promote standby 1, switching it to a new timeline
+# Stop and remove master
$node_master->teardown_node;
-$node_standby_1->promote;
+
+# promote standby 1 using "pg_promote", switching it to a new timeline
+$node_standby_1->safe_psql('postgres', "SELECT pg_promote(wait_seconds => 10)");
# Switch standby 2 to replay from standby 1
rmtree($node_standby_2->data_dir . '/recovery.conf');
diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_twophase.pl
index 9ea3bd65fc..a6044179bc 100644
--- a/src/test/recovery/t/009_twophase.pl
+++ b/src/test/recovery/t/009_twophase.pl
@@ -214,7 +214,11 @@ $cur_master->psql(
INSERT INTO t_009_tbl VALUES (22, 'issued to ${cur_master_name}');
PREPARE TRANSACTION 'xact_009_10';");
$cur_master->teardown_node;
-$cur_standby->promote;
+
+# promote standby using "pg_promote" and wait until it is promoted
+$cur_standby->safe_psql('postgres', 'SELECT pg_promote(FALSE)');
+$cur_standby->poll_query_until('postgres', "SELECT NOT pg_is_in_recovery()")
+ or die "standby never exited recovery";
# change roles
note "Now paris is master and london is standby";
--
2.17.2
On Sat, Oct 20, 2018 at 06:24:28AM +0200, Laurenz Albe wrote:
Here is another version, with a fix in pg_proc.dat, an improved comment
and "wait_seconds" exercised in the regression test.
Thanks for the new version. This looks pretty good to me. I'll see if
I can review it once and then commit.
- WAIT_EVENT_SYNC_REP + WAIT_EVENT_SYNC_REP, + WAIT_EVENT_PROMOTE } WaitEventIPC;
Those are kept in alphabetical order. Individual wait events are also
documented with a description.
--
Michael
On Sat, Oct 20, 2018 at 01:48:56PM +0900, Michael Paquier wrote:
On Sat, Oct 20, 2018 at 06:24:28AM +0200, Laurenz Albe wrote:
Here is another version, with a fix in pg_proc.dat, an improved comment
and "wait_seconds" exercised in the regression test.Thanks for the new version. This looks pretty good to me. I'll see if
I can review it once and then commit.- WAIT_EVENT_SYNC_REP + WAIT_EVENT_SYNC_REP, + WAIT_EVENT_PROMOTE } WaitEventIPC;Those are kept in alphabetical order. Individual wait events are also
documented with a description.
Regarding the documentation, wouldn't it be more adapted to list the new
function under the section "Recovery Control Functions"? Not only does
the new function signal the postmaster, but it also creates the
promotion trigger file.
Anyway, I have been looking in depth at the patch, and I finish with the
attached. Here are some notes:
- pg_ctl returns an error if it cannot create the promote trigger file
and if it doesn't close it. pg_promote should do the same.
- WL_TIMEOUT could have been reached, leading to no further retries
(remove for example the part related to the trigger file creation and
try to trigger the promotion, the wait time is incorrect).
- Documentation has been reworked.
- The new wait event is documented.
- a WARNING is returned if the signal to the postmaster is not sent,
which is something I think makes most sense as we do the same for other
signals.
- position including unistd.h was not correct in xlogfuncs.c.
- Timeouts for the tests are made a tad longer. Some buildfarm machines
don't like a promotion wait of 10s.
- a catalog version bump is included, just a personal reminder..
- Indentatio has been run.
Laurenz, what do you think?
--
Michael
Attachments:
promote-func.patchtext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 5193df3366..96d45419e5 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19202,6 +19202,9 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
<indexterm>
<primary>pg_is_wal_replay_paused</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_promote</primary>
+ </indexterm>
<indexterm>
<primary>pg_wal_replay_pause</primary>
</indexterm>
@@ -19232,6 +19235,22 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
<entry>True if recovery is paused.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_promote(<parameter>wait</parameter> <type>boolean</type> DEFAULT true, <parameter>wait_seconds</parameter> <type>integer</type> DEFAULT 60)</function></literal>
+ </entry>
+ <entry><type>boolean</type></entry>
+ <entry>
+ Promotes a physical standby server. Returns <literal>true</literal>
+ if promotion is successful and <literal>false</literal> otherwise.
+ With <parameter>wait</parameter> set to <literal>true</literal>, the
+ default, the function waits until promotion is completed or
+ <parameter>wait_seconds</parameter> seconds have passed, otherwise the
+ function returns immediately after sending the promotion signal to the
+ postmaster. This function is restricted to superusers by default, but
+ other users can be granted EXECUTE to run the function.
+ </entry>
+ </row>
<row>
<entry>
<literal><function>pg_wal_replay_pause()</function></literal>
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index ebcb3daaed..faf8e71854 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1471,14 +1471,17 @@ synchronous_standby_names = 'ANY 2 (s1, s2, s3)'
</para>
<para>
- To trigger failover of a log-shipping standby server,
- run <command>pg_ctl promote</command> or create a trigger
- file with the file name and path specified by the <varname>trigger_file</varname>
- setting in <filename>recovery.conf</filename>. If you're planning to use
- <command>pg_ctl promote</command> to fail over, <varname>trigger_file</varname> is
- not required. If you're setting up the reporting servers that are
- only used to offload read-only queries from the primary, not for high
- availability purposes, you don't need to promote it.
+ To trigger failover of a log-shipping standby server, run
+ <command>pg_ctl promote</command>, call <function>pg_promote</function>,
+ or create a trigger file with the file name and path specified by the
+ <varname>trigger_file</varname> setting in
+ <filename>recovery.conf</filename>. If you're planning to use
+ <command>pg_ctl promote</command> or to call
+ <function>pg_promote</function> to fail over,
+ <varname>trigger_file</varname> is not required. If you're
+ setting up the reporting servers that are only used to offload read-only
+ queries from the primary, not for high availability purposes, you don't
+ need to promote it.
</para>
</sect1>
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 0484cfa77a..073f19542a 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1268,7 +1268,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<entry>Waiting in an extension.</entry>
</row>
<row>
- <entry morerows="33"><literal>IPC</literal></entry>
+ <entry morerows="34"><literal>IPC</literal></entry>
<entry><literal>BgWorkerShutdown</literal></entry>
<entry>Waiting for background worker to shut down.</entry>
</row>
@@ -1384,6 +1384,10 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<entry><literal>ProcArrayGroupUpdate</literal></entry>
<entry>Waiting for group leader to clear transaction id at transaction end.</entry>
</row>
+ <row>
+ <entry><literal>Promote</literal></entry>
+ <entry>Waiting for standby promotion.</entry>
+ </row>
<row>
<entry><literal>ClogGroupUpdate</literal></entry>
<entry>Waiting for group leader to update transaction status at transaction end.</entry>
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 92825fdf19..a2bdffda94 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -439,7 +439,8 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<para>
Specifies a trigger file whose presence ends recovery in the
standby. Even if this value is not set, you can still promote
- the standby using <command>pg_ctl promote</command>.
+ the standby using <command>pg_ctl promote</command> or calling
+ <function>pg_promote</function>.
This setting has no effect if <varname>standby_mode</varname> is <literal>off</literal>.
</para>
</listitem>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7375a78ffc..62fc418893 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -78,12 +78,6 @@
extern uint32 bootstrap_data_checksum_version;
-/* File path names (all relative to $PGDATA) */
-#define RECOVERY_COMMAND_FILE "recovery.conf"
-#define RECOVERY_COMMAND_DONE "recovery.done"
-#define PROMOTE_SIGNAL_FILE "promote"
-#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
-
/* User-settable parameters */
int max_wal_size_mb = 1024; /* 1 GB */
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 9731742978..6279b17caa 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -16,6 +16,8 @@
*/
#include "postgres.h"
+#include <unistd.h>
+
#include "access/htup_details.h"
#include "access/xlog.h"
#include "access/xlog_internal.h"
@@ -23,6 +25,7 @@
#include "catalog/pg_type.h"
#include "funcapi.h"
#include "miscadmin.h"
+#include "pgstat.h"
#include "replication/walreceiver.h"
#include "storage/smgr.h"
#include "utils/builtins.h"
@@ -697,3 +700,77 @@ pg_backup_start_time(PG_FUNCTION_ARGS)
PG_RETURN_DATUM(xtime);
}
+
+/*
+ * Promotes a standby server.
+ *
+ * A result of "true" means that promotion has been completed if "wait is
+ * "true", or initiated if "wait" is false.
+ */
+Datum
+pg_promote(PG_FUNCTION_ARGS)
+{
+ bool wait = PG_GETARG_BOOL(0);
+ int wait_seconds = PG_GETARG_INT32(1);
+ FILE *promote_file;
+ int i;
+
+ if (!RecoveryInProgress())
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("recovery is not in progress"),
+ errhint("Recovery control functions can only be executed during recovery.")));
+
+ if (wait_seconds <= 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("\"wait_seconds\" cannot be negative or equal zero")));
+
+ /* create the promote signal file */
+ promote_file = AllocateFile(PROMOTE_SIGNAL_FILE, "w");
+ if (!promote_file)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not create file \"%s\": %m",
+ PROMOTE_SIGNAL_FILE)));
+
+ if (FreeFile(promote_file))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not write file \"%s\": %m",
+ PROMOTE_SIGNAL_FILE)));
+
+ /* signal the postmaster */
+ if (kill(PostmasterPid, SIGUSR1) != 0)
+ {
+ ereport(WARNING,
+ (errmsg("failed to send signal to postmaster: %m")));
+ (void) unlink(PROMOTE_SIGNAL_FILE);
+ PG_RETURN_BOOL(false);
+ }
+
+ /* return immediately if waiting was not requested */
+ if (!wait)
+ PG_RETURN_BOOL(true);
+
+ /* wait for the amount of time wanted until promotion */
+#define WAITS_PER_SECOND 10
+ for (i = 0; i < WAITS_PER_SECOND * wait_seconds; ++i)
+ {
+ ResetLatch(MyLatch);
+
+ if (!RecoveryInProgress())
+ PG_RETURN_BOOL(true);
+
+ CHECK_FOR_INTERRUPTS();
+
+ WaitLatch(MyLatch,
+ WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
+ 1000L / WAITS_PER_SECOND,
+ WAIT_EVENT_PROMOTE);
+ }
+
+ ereport(WARNING,
+ (errmsg("server did not promote within %d seconds", wait_seconds)));
+ PG_RETURN_BOOL(false);
+}
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index a03b005f73..2857d80984 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1119,6 +1119,13 @@ LANGUAGE INTERNAL
STRICT IMMUTABLE PARALLEL SAFE
AS 'jsonb_insert';
+CREATE OR REPLACE FUNCTION
+ pg_promote(wait boolean DEFAULT true, wait_seconds integer DEFAULT 60)
+RETURNS boolean
+LANGUAGE INTERNAL
+STRICT VOLATILE
+AS 'pg_promote';
+
--
-- The default permissions for functions mean that anyone can execute them.
-- A number of functions shouldn't be executable by just anyone, but rather
@@ -1138,6 +1145,7 @@ REVOKE EXECUTE ON FUNCTION pg_rotate_logfile() FROM public;
REVOKE EXECUTE ON FUNCTION pg_reload_conf() FROM public;
REVOKE EXECUTE ON FUNCTION pg_current_logfile() FROM public;
REVOKE EXECUTE ON FUNCTION pg_current_logfile(text) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_promote(boolean, integer) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_reset() FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_reset_shared(text) FROM public;
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 8de603d193..d02d6a647d 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3660,6 +3660,9 @@ pgstat_get_wait_ipc(WaitEventIPC w)
case WAIT_EVENT_PROCARRAY_GROUP_UPDATE:
event_name = "ProcArrayGroupUpdate";
break;
+ case WAIT_EVENT_PROMOTE:
+ event_name = "Promote";
+ break;
case WAIT_EVENT_CLOG_GROUP_UPDATE:
event_name = "ClogGroupUpdate";
break;
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 421ba6d775..e01d12eb7c 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -319,10 +319,16 @@ extern void do_pg_abort_backup(void);
extern SessionBackupState get_backup_status(void);
/* File path names (all relative to $PGDATA) */
+#define RECOVERY_COMMAND_FILE "recovery.conf"
+#define RECOVERY_COMMAND_DONE "recovery.done"
#define BACKUP_LABEL_FILE "backup_label"
#define BACKUP_LABEL_OLD "backup_label.old"
#define TABLESPACE_MAP "tablespace_map"
#define TABLESPACE_MAP_OLD "tablespace_map.old"
+/* files to signal promotion to primary */
+#define PROMOTE_SIGNAL_FILE "promote"
+#define FALLBACK_PROMOTE_SIGNAL_FILE "fallback_promote"
+
#endif /* XLOG_H */
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index e04eabf683..7c2d2aa075 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 201810111
+#define CATALOG_VERSION_NO 201810221
#endif
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index cff58ed2d8..4d7fe1b383 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5824,6 +5824,10 @@
proname => 'pg_backup_start_time', provolatile => 's',
prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_backup_start_time' },
+{ oid => '3436', descr => 'promote standby server',
+ proname => 'pg_promote', provolatile => 'v',
+ prorettype => 'bool', proargtypes => 'bool int4', proargnames => '{wait,wait_seconds}',
+ prosrc => 'pg_promote' },
{ oid => '2848', descr => 'switch to new wal file',
proname => 'pg_switch_wal', provolatile => 'v', prorettype => 'pg_lsn',
proargtypes => '', prosrc => 'pg_switch_wal' },
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index d59c24ae23..e0ec3f9086 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -828,6 +828,7 @@ typedef enum
WAIT_EVENT_PARALLEL_BITMAP_SCAN,
WAIT_EVENT_PARALLEL_CREATE_INDEX_SCAN,
WAIT_EVENT_PROCARRAY_GROUP_UPDATE,
+ WAIT_EVENT_PROMOTE,
WAIT_EVENT_CLOG_GROUP_UPDATE,
WAIT_EVENT_REPLICATION_ORIGIN_DROP,
WAIT_EVENT_REPLICATION_SLOT_DROP,
diff --git a/src/test/recovery/t/004_timeline_switch.pl b/src/test/recovery/t/004_timeline_switch.pl
index 34ee335129..e774ec26f0 100644
--- a/src/test/recovery/t/004_timeline_switch.pl
+++ b/src/test/recovery/t/004_timeline_switch.pl
@@ -37,9 +37,12 @@ $node_master->safe_psql('postgres',
$node_master->wait_for_catchup($node_standby_1, 'replay',
$node_master->lsn('write'));
-# Stop and remove master, and promote standby 1, switching it to a new timeline
+# Stop and remove master
$node_master->teardown_node;
-$node_standby_1->promote;
+
+# promote standby 1 using "pg_promote", switching it to a new timeline
+$node_standby_1->safe_psql('postgres',
+ "SELECT pg_promote(wait_seconds => 300)");
# Switch standby 2 to replay from standby 1
rmtree($node_standby_2->data_dir . '/recovery.conf');
diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_twophase.pl
index 9ea3bd65fc..f5ebbd995e 100644
--- a/src/test/recovery/t/009_twophase.pl
+++ b/src/test/recovery/t/009_twophase.pl
@@ -214,7 +214,11 @@ $cur_master->psql(
INSERT INTO t_009_tbl VALUES (22, 'issued to ${cur_master_name}');
PREPARE TRANSACTION 'xact_009_10';");
$cur_master->teardown_node;
-$cur_standby->promote;
+
+# promote standby using "pg_promote" and wait until it is promoted
+$cur_standby->safe_psql('postgres', 'SELECT pg_promote(false)');
+$cur_standby->poll_query_until('postgres', "SELECT NOT pg_is_in_recovery()")
+ or die "standby never exited recovery";
# change roles
note "Now paris is master and london is standby";
On Mon, Oct 22, 2018 at 3:01 PM Michael Paquier <michael@paquier.xyz> wrote:
On Sat, Oct 20, 2018 at 01:48:56PM +0900, Michael Paquier wrote:
On Sat, Oct 20, 2018 at 06:24:28AM +0200, Laurenz Albe wrote:
Here is another version, with a fix in pg_proc.dat, an improved comment
and "wait_seconds" exercised in the regression test.Thanks for the new version. This looks pretty good to me. I'll see if
I can review it once and then commit.- WAIT_EVENT_SYNC_REP + WAIT_EVENT_SYNC_REP, + WAIT_EVENT_PROMOTE } WaitEventIPC;Those are kept in alphabetical order. Individual wait events are also
documented with a description.Regarding the documentation, wouldn't it be more adapted to list the new
function under the section "Recovery Control Functions"? Not only does
the new function signal the postmaster, but it also creates the
promotion trigger file.Anyway, I have been looking in depth at the patch, and I finish with the
attached. Here are some notes:
- pg_ctl returns an error if it cannot create the promote trigger file
and if it doesn't close it. pg_promote should do the same.
- WL_TIMEOUT could have been reached, leading to no further retries
(remove for example the part related to the trigger file creation and
try to trigger the promotion, the wait time is incorrect).
- Documentation has been reworked.
- The new wait event is documented.
- a WARNING is returned if the signal to the postmaster is not sent,
which is something I think makes most sense as we do the same for other
signals.
- position including unistd.h was not correct in xlogfuncs.c.
- Timeouts for the tests are made a tad longer. Some buildfarm machines
don't like a promotion wait of 10s.
- a catalog version bump is included, just a personal reminder..
- Indentatio has been run.
Thank you for workig on this. There is one review comment for the latest patch.
+ if (FreeFile(promote_file))
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not write file \"%s\": %m",
+ PROMOTE_SIGNAL_FILE)));
Maybe we should unlink PROMOTE_SIGNAL_FILE before erroring.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Masahiko Sawada wrote:
On Mon, Oct 22, 2018 at 3:01 PM Michael Paquier <michael@paquier.xyz> wrote:
Regarding the documentation, wouldn't it be more adapted to list the new
function under the section "Recovery Control Functions"? Not only does
the new function signal the postmaster, but it also creates the
promotion trigger file.
Hmm, yes, that's probably the first place where people would look.
I guess the implementation lead me to categorize it as a "signaling function",
and it also wouldn't be wrong there.
Anyway, I have been looking in depth at the patch, and I finish with the
attached. Here are some notes:
[...]
- WL_TIMEOUT could have been reached, leading to no further retries
(remove for example the part related to the trigger file creation and
try to trigger the promotion, the wait time is incorrect).
Ok.
- Documentation has been reworked.
- The new wait event is documented.
Thanks.
- position including unistd.h was not correct in xlogfuncs.c.
- Timeouts for the tests are made a tad longer. Some buildfarm machines
don't like a promotion wait of 10s.
- a catalog version bump is included, just a personal reminder..
- Indentatio has been run.
Thanks.
Thank you for workig on this. There is one review comment for the latest patch.
+ if (FreeFile(promote_file)) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not write file \"%s\": %m", + PROMOTE_SIGNAL_FILE)));Maybe we should unlink PROMOTE_SIGNAL_FILE before erroring.
Yes, that cannot hurt.
Yours,
Laurenz Albe
On Mon, Oct 22, 2018 at 11:45:30AM +0200, Laurenz Albe wrote:
Masahiko Sawada wrote:
Thank you for workig on this. There is one review comment for the latest patch.
+ if (FreeFile(promote_file)) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not write file \"%s\": %m", + PROMOTE_SIGNAL_FILE)));Maybe we should unlink PROMOTE_SIGNAL_FILE before erroring.
Yes, that cannot hurt.
If FreeFile() fails, unlink() would most likely fail for the same
reason. Please note that if unlink() happens before issuing the ERROR,
saving errno would be necessary. That's not a huge issue anyway, if a
failure happens, the operator would retry the operation. If there is a
crash, the file gets removed at the end of recovery. If there are no
objections, I'll look at this patch again by the end of this week in
order to get it committed.
--
Michael
Michael Paquier wrote:
If there are no
objections, I'll look at this patch again by the end of this week in
order to get it committed.
No objections from me; on the contrary, I would like to thank you for
your effort here. I appreciate that you probably spent more time
tutoring me than it would have taken you to write this yourself.
Yours,
Laurenz Albe
On Tue, Oct 23, 2018 at 09:42:16AM +0200, Laurenz Albe wrote:
No objections from me; on the contrary, I would like to thank you for
your effort here. I appreciate that you probably spent more time
tutoring me than it would have taken you to write this yourself.
You're welcome. Happy that it helped.
--
Michael
On Wed, Oct 24, 2018 at 08:50:43AM +0900, Michael Paquier wrote:
On Tue, Oct 23, 2018 at 09:42:16AM +0200, Laurenz Albe wrote:
No objections from me; on the contrary, I would like to thank you for
your effort here. I appreciate that you probably spent more time
tutoring me than it would have taken you to write this yourself.You're welcome. Happy that it helped.
And committed. I double-checked the code, and tweaked a bit the tests
so as the test using wait_mode = false is removed as it did not seem
worth the extra cycles. I also added a check on the return value of
pg_promote when using the wait mode. Another thing was that the
function needs to be parallel-restricted.
--
Michael
Hi
On 10/25/2018 09:47 AM, Michael Paquier wrote:
And committed. I double-checked the code, and tweaked a bit the tests
so as the test using wait_mode = false is removed as it did not seem
worth the extra cycles. I also added a check on the return value of
pg_promote when using the wait mode. Another thing was that the
function needs to be parallel-restricted.
Documentation for this [*] says "Returns true if promotion is successful and false otherwise",
which is not correct if "wait" is false, as it will always return TRUE.
[*] https://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-RECOVERY-CONTROL
Attached patch contains a suggested rewording to clarify this.
Regards
Ian Barwick
--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
doc-pg-promote.patchtext/x-patch; name=doc-pg-promote.patchDownload
commit 208247998484ee70cff5f16050344d53bcc6bb42
Author: Ian Barwick <ian@2ndquadrant.com>
Date: Fri Oct 26 11:09:21 2018 +0900
Clarify description of pg_promote function
Note that it always returns TRUE if the "wait" parameter is set to false.
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 96d4541..422af61 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19241,13 +19241,14 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</entry>
<entry><type>boolean</type></entry>
<entry>
- Promotes a physical standby server. Returns <literal>true</literal>
- if promotion is successful and <literal>false</literal> otherwise.
- With <parameter>wait</parameter> set to <literal>true</literal>, the
- default, the function waits until promotion is completed or
- <parameter>wait_seconds</parameter> seconds have passed, otherwise the
- function returns immediately after sending the promotion signal to the
- postmaster. This function is restricted to superusers by default, but
+ Promotes a physical standby server. With <parameter>wait</parameter> set
+ to <literal>true</literal> (the default), the function waits until promotion
+ is completed or <parameter>wait_seconds</parameter> seconds have passed,
+ and returns <literal>true</literal> if promotion is successful and
+ <literal>false</literal> otherwise. If <parameter>wait</parameter>
+ is set to <literal>false</literal>, the function returns <literal>true</literal>
+ immediately after sending the promotion signal to the postmaster.
+ This function is restricted to superusers by default, but
other users can be granted EXECUTE to run the function.
</entry>
</row>
Hi
On 10/25/2018 09:47 AM, Michael Paquier wrote:
And committed. I double-checked the code, and tweaked a bit the tests
so as the test using wait_mode = false is removed as it did not seem
worth the extra cycles. I also added a check on the return value of
pg_promote when using the wait mode. Another thing was that the
function needs to be parallel-restricted.
Documentation for this [*] says "Returns true if promotion is successful and false otherwise",
which is not correct if "wait" is false, as it will always return TRUE.
[*] https://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-RECOVERY-CONTROL
Attached patch contains a suggested rewording to clarify this.
Regards
Ian Barwick
--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
doc-pg-promote.patchtext/x-patch; name=doc-pg-promote.patchDownload
commit 208247998484ee70cff5f16050344d53bcc6bb42
Author: Ian Barwick <ian@2ndquadrant.com>
Date: Fri Oct 26 11:09:21 2018 +0900
Clarify description of pg_promote function
Note that it always returns TRUE if the "wait" parameter is set to false.
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 96d4541..422af61 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19241,13 +19241,14 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
</entry>
<entry><type>boolean</type></entry>
<entry>
- Promotes a physical standby server. Returns <literal>true</literal>
- if promotion is successful and <literal>false</literal> otherwise.
- With <parameter>wait</parameter> set to <literal>true</literal>, the
- default, the function waits until promotion is completed or
- <parameter>wait_seconds</parameter> seconds have passed, otherwise the
- function returns immediately after sending the promotion signal to the
- postmaster. This function is restricted to superusers by default, but
+ Promotes a physical standby server. With <parameter>wait</parameter> set
+ to <literal>true</literal> (the default), the function waits until promotion
+ is completed or <parameter>wait_seconds</parameter> seconds have passed,
+ and returns <literal>true</literal> if promotion is successful and
+ <literal>false</literal> otherwise. If <parameter>wait</parameter>
+ is set to <literal>false</literal>, the function returns <literal>true</literal>
+ immediately after sending the promotion signal to the postmaster.
+ This function is restricted to superusers by default, but
other users can be granted EXECUTE to run the function.
</entry>
</row>
On Fri, Oct 26, 2018 at 11:36:00AM +0900, Ian Barwick wrote:
Documentation for this [*] says "Returns true if promotion is
successful and false otherwise", which is not correct if "wait" is
false, as it will always return TRUE.
Yes, in the case where the promotion has been initiated.
+ <literal>false</literal> otherwise. If <parameter>wait</parameter> + is set to <literal>false</literal>, the function returns <literal>true</literal> + immediately after sending the promotion signal to the postmaster.
Or we could use "the function returns true immediately after initiating
the promotion by sending the promotion signal to the postmaster"? As a
native speaker which one feels more natural to you?
--
Michael
On Fri, Oct 26, 2018 at 01:51:24PM +0900, Michael Paquier wrote:
Or we could use "the function returns true immediately after initiating
the promotion by sending the promotion signal to the postmaster"? As a
native speaker which one feels more natural to you?
Sorry for the time it took. After pondering on it, I have committed the
improvements from your version.
--
Michael
On 11/19/2018 01:22 PM, Michael Paquier wrote:
On Fri, Oct 26, 2018 at 01:51:24PM +0900, Michael Paquier wrote:
Or we could use "the function returns true immediately after initiating
the promotion by sending the promotion signal to the postmaster"? As a
native speaker which one feels more natural to you?Sorry for the time it took. After pondering on it, I have committed the
improvements from your version.
Thanks, looks good (and apologies for the delay in responding from my
side).
Regards
Ian Barwick
--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On Wed, Nov 28, 2018 at 10:06:34AM +0900, Ian Barwick wrote:
Thanks, looks good (and apologies for the delay in responding from my
side).
Thanks for double-checking, Ian. I took my time as well ;)
(Hope to see you face-to-face in a couple of days around Akihabara.)
--
Michael