On 2/19/16 3:09 PM, Tom Lane wrote:
> I see no need for an additional mechanism. Just watch pg_control until
> you see DB_IN_PRODUCTION state there, then switch over to the same
> connection probing that "pg_ctl start -w" uses.
Here is a patch set around that idea.
The subsequent discussion mentioned that there might still be a window
between end of waiting and when read-write queries would be accepted. I
don't know how big that window would be in practice and would be
interested in some testing and feedback.
From cb5d4a63620636d4043d1a85acf7fcfdace73b1d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Sun, 28 Feb 2016 20:21:54 -0500
Subject: [PATCH 1/3] pg_ctl: Add tests for promote action
---
src/bin/pg_ctl/t/003_promote.pl | 62 +++++++++++++++++++++++++++++++++++++++++
src/test/perl/TestLib.pm | 11 ++++++++
2 files changed, 73 insertions(+)
create mode 100644 src/bin/pg_ctl/t/003_promote.pl
diff --git a/src/bin/pg_ctl/t/003_promote.pl b/src/bin/pg_ctl/t/003_promote.pl
new file mode 100644
index 0000000..64d72e0
--- /dev/null
+++ b/src/bin/pg_ctl/t/003_promote.pl
@@ -0,0 +1,62 @@
+use strict;
+use warnings;
+
+use PostgresNode;
+use TestLib;
+use Test::More tests => 13;
+
+my $tempdir = TestLib::tempdir;
+#my $tempdir_short = TestLib::tempdir_short;
+
+command_fails_like([ 'pg_ctl', 'promote', '-D', "$tempdir/nonexistent" ],
+ qr/directory .* does not exist/,
+ 'pg_ctl promote with nonexistent directory');
+
+my $node_primary = get_new_node('primary');
+$node_primary->init;
+$node_primary->append_conf(
+ "postgresql.conf", qq(
+wal_level = hot_standby
+max_wal_senders = 2
+wal_keep_segments = 20
+hot_standby = on
+)
+ );
+
+command_fails_like([ 'pg_ctl', 'promote', '-D', $node_primary->data_dir ],
+ qr/PID file .* does not exist/,
+ 'pg_ctl promote of not running instance fails');
+
+$node_primary->start;
+
+command_fails_like([ 'pg_ctl', 'promote', '-D', $node_primary->data_dir ],
+ qr/not in standby mode/,
+ 'pg_ctl promote of primary instance fails');
+
+my $node_standby = get_new_node('standby');
+$node_primary->backup('my_backup');
+$node_standby->init_from_backup($node_primary, 'my_backup');
+my $connstr_primary = $node_primary->connstr('postgres');
+
+$node_standby->append_conf(
+ "recovery.conf", qq(
+primary_conninfo='$connstr_primary'
+standby_mode=on
+recovery_target_timeline='latest'
+)
+ );
+
+$node_standby->start;
+
+$node_standby->command_like(['psql', '-X', '-A', '-t', '-c', 'SELECT pg_is_in_recovery()'],
+ qr/^t$/,
+ 'standby is in recovery');
+
+command_ok([ 'pg_ctl', 'promote', '-D', $node_standby->data_dir ],
+ 'pg_ctl promote of standby runs');
+
+sleep 3; # needs a moment to react
+
+$node_standby->command_like(['psql', '-X', '-A', '-t', '-c', 'SELECT pg_is_in_recovery()'],
+ qr/^f$/,
+ 'promoted standby is not in recovery');
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 3d11cbb..dd275cf 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -34,6 +34,7 @@ our @EXPORT = qw(
program_version_ok
program_options_handling_ok
command_like
+ command_fails_like
$windows_os
);
@@ -262,4 +263,14 @@ sub command_like
like($stdout, $expected_stdout, "$test_name: matches");
}
+sub command_fails_like
+{
+ my ($cmd, $expected_stderr, $test_name) = @_;
+ my ($stdout, $stderr);
+ print("# Running: " . join(" ", @{$cmd}) . "\n");
+ my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ ok(!$result, "@$cmd exit code not 0");
+ like($stderr, $expected_stderr, "$test_name: matches");
+}
+
1;
--
2.7.2
From 7c1b6e94f5e3beb9e558d2af7098940d4475fe11 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Sun, 28 Feb 2016 20:21:54 -0500
Subject: [PATCH 2/3] pg_ctl: Detect current standby state from pg_control
pg_ctl used to determine whether a server was in standby mode by looking
for a recovery.conf file. With this change, it instead looks into
pg_control, which is potentially more accurate. There are also
occasional discussions about removing recovery.conf, so this removes one
dependency.
---
src/bin/pg_ctl/pg_ctl.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 57 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index bae6c22..c38c479 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -19,6 +19,7 @@
#include "postgres_fe.h"
+#include "catalog/pg_control.h"
#include "libpq-fe.h"
#include "pqexpbuffer.h"
@@ -158,6 +159,8 @@ static bool postmaster_is_alive(pid_t pid);
static void unlimit_core_size(void);
#endif
+static DBState get_control_dbstate(void);
+
#ifdef WIN32
static void
@@ -1168,7 +1171,6 @@ do_promote(void)
{
FILE *prmfile;
pgpid_t pid;
- struct stat statbuf;
pid = get_pgpid(false);
@@ -1187,8 +1189,7 @@ do_promote(void)
exit(1);
}
- /* If recovery.conf doesn't exist, the server is not in standby mode */
- if (stat(recovery_file, &statbuf) != 0)
+ if (get_control_dbstate() != DB_IN_ARCHIVE_RECOVERY)
{
write_stderr(_("%s: cannot promote server; "
"server is not in standby mode\n"),
@@ -2115,6 +2116,59 @@ adjust_data_dir(void)
}
+static DBState
+get_control_dbstate(void)
+{
+ char *control_file_path;
+ ControlFileData control_file_data;
+
+ control_file_path = psprintf("%s/global/pg_control", pg_data);
+
+ for (;;)
+ {
+ int fd;
+ pg_crc32c crc;
+
+ if ((fd = open(control_file_path, O_RDONLY | PG_BINARY, 0)) == -1)
+ {
+ fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
+ progname, control_file_path, strerror(errno));
+ exit(1);
+ }
+
+ if (read(fd, &control_file_data, sizeof(control_file_data)) != sizeof(control_file_data))
+ {
+ fprintf(stderr, _("%s: could not read file \"%s\": %s\n"),
+ progname, control_file_path, strerror(errno));
+ exit(1);
+ }
+ close(fd);
+
+ INIT_CRC32C(crc);
+ COMP_CRC32C(crc,
+ &control_file_data,
+ offsetof(ControlFileData, crc));
+ FIN_CRC32C(crc);
+
+ if (EQ_CRC32C(crc, control_file_data.crc))
+ break;
+
+ if (wait_seconds > 0)
+ {
+ sleep(1);
+ wait_seconds--;
+ continue;
+ }
+
+ fprintf(stderr, _("%s: control file \"%s\" appears to be corrupt\n"),
+ progname, control_file_path);
+ exit(1);
+ }
+
+ return control_file_data.state;
+}
+
+
int
main(int argc, char **argv)
{
--
2.7.2
From fc1f402ed1c60470322133ab30b0034357163ff5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Sun, 28 Feb 2016 20:21:54 -0500
Subject: [PATCH 3/3] pg_ctl: Add wait option to promote action
When waiting is selected for the promote action, look into pg_control
until the state changes, then use the PQping-based waiting until the
server is reachable.
---
doc/src/sgml/ref/pg_ctl-ref.sgml | 29 ++++++++++++++++++++------
src/bin/pg_ctl/pg_ctl.c | 45 ++++++++++++++++++++++++++++------------
src/bin/pg_ctl/t/003_promote.pl | 28 ++++++++++++++++++++++++-
3 files changed, 82 insertions(+), 20 deletions(-)
diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
index 6ceb781..a00c355 100644
--- a/doc/src/sgml/ref/pg_ctl-ref.sgml
+++ b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -91,6 +91,8 @@
<cmdsynopsis>
<command>pg_ctl</command>
<arg choice="plain"><option>promote</option></arg>
+ <arg choice="opt"><option>-w</option></arg>
+ <arg choice="opt"><option>-t</option> <replaceable>seconds</replaceable></arg>
<arg choice="opt"><option>-s</option></arg>
<arg choice="opt"><option>-D</option> <replaceable>datadir</replaceable></arg>
</cmdsynopsis>
@@ -361,8 +363,8 @@ <title>Options</title>
<term><option>--timeout</option></term>
<listitem>
<para>
- The maximum number of seconds to wait when waiting for startup or
- shutdown to complete. Defaults to the value of the
+ The maximum number of seconds to wait when waiting for an operation
+ to complete (see option <option>-w</option>). Defaults to the value of the
<envar>PGCTLTIMEOUT</> environment variable or, if not set, to 60
seconds.
</para>
@@ -383,8 +385,23 @@ <title>Options</title>
<term><option>-w</option></term>
<listitem>
<para>
- Wait for the startup or shutdown to complete.
- Waiting is the default option for shutdowns, but not startups.
+ Wait for an operation to complete. This is supported for the
+ modes <literal>start</literal>, <literal>stop</literal>,
+ <literal>restart</literal>, <literal>promote</literal>,
+ and <literal>register</literal>.
+ </para>
+
+ <para>
+ Waiting is the default option for shutdowns, but not startups,
+ restarts, or promotions. This is mainly for historical reasons; the
+ waiting option is almost always preferable. If waiting is not
+ selected, the requested action is triggered, but there is no feedback
+ about its success. In that case, the server log file or an external
+ monitoring system would have to be used to check the progress and
+ success of the operation.
+ </para>
+
+ <para>
When waiting for startup, <command>pg_ctl</command> repeatedly
attempts to connect to the server.
When waiting for shutdown, <command>pg_ctl</command> waits for
@@ -400,8 +417,8 @@ <title>Options</title>
<term><option>-W</option></term>
<listitem>
<para>
- Do not wait for startup or shutdown to complete. This is the
- default for start and restart modes.
+ Do not wait for an operation to complete. This is the opposite of the
+ option <option>-w</option>.
</para>
</listitem>
</varlistentry>
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index c38c479..6c152b1 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1228,7 +1228,34 @@ do_promote(void)
exit(1);
}
- print_msg(_("server promoting\n"));
+ if (do_wait)
+ {
+ DBState state = DB_STARTUP;
+
+ print_msg(_("waiting for server to promote..."));
+ while (wait_seconds > 0)
+ {
+ state = get_control_dbstate();
+ if (state == DB_IN_PRODUCTION)
+ break;
+
+ print_msg(".");
+ pg_usleep(1000000); /* 1 sec */
+ wait_seconds--;
+ }
+ if (state == DB_IN_PRODUCTION)
+ {
+ print_msg(_(" done\n"));
+ print_msg(_("server promoted\n"));
+ }
+ else
+ {
+ print_msg(_(" stopped waiting\n"));
+ print_msg(_("server is still promoting\n"));
+ }
+ }
+ else
+ print_msg(_("server promoting\n"));
}
@@ -2429,18 +2456,10 @@ main(int argc, char **argv)
if (!wait_set)
{
- switch (ctl_command)
- {
- case RESTART_COMMAND:
- case START_COMMAND:
- do_wait = false;
- break;
- case STOP_COMMAND:
- do_wait = true;
- break;
- default:
- break;
- }
+ if (ctl_command == STOP_COMMAND)
+ do_wait = true;
+ else
+ do_wait = false;
}
if (ctl_command == RELOAD_COMMAND)
diff --git a/src/bin/pg_ctl/t/003_promote.pl b/src/bin/pg_ctl/t/003_promote.pl
index 64d72e0..5c9c31c 100644
--- a/src/bin/pg_ctl/t/003_promote.pl
+++ b/src/bin/pg_ctl/t/003_promote.pl
@@ -3,7 +3,7 @@
use PostgresNode;
use TestLib;
-use Test::More tests => 13;
+use Test::More tests => 20;
my $tempdir = TestLib::tempdir;
#my $tempdir_short = TestLib::tempdir_short;
@@ -60,3 +60,29 @@
$node_standby->command_like(['psql', '-X', '-A', '-t', '-c', 'SELECT pg_is_in_recovery()'],
qr/^f$/,
'promoted standby is not in recovery');
+
+$node_standby = get_new_node('standby2');
+$node_standby->init_from_backup($node_primary, 'my_backup');
+
+$node_standby->append_conf(
+ "recovery.conf", qq(
+primary_conninfo='$connstr_primary'
+standby_mode=on
+recovery_target_timeline='latest'
+)
+ );
+
+$node_standby->start;
+
+$node_standby->command_like(['psql', '-X', '-A', '-t', '-c', 'SELECT pg_is_in_recovery()'],
+ qr/^t$/,
+ 'standby is in recovery');
+
+command_ok([ 'pg_ctl', 'promote', '-w', '-D', $node_standby->data_dir ],
+ 'pg_ctl promote -w of standby runs');
+
+# no wait here
+
+$node_standby->command_like(['psql', '-X', '-A', '-t', '-c', 'SELECT pg_is_in_recovery()'],
+ qr/^f$/,
+ 'promoted standby is not in recovery');
--
2.7.2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
