pg_ctl promote wait

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

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 00f139a..590385c 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7371,12 +7371,6 @@ StartupXLOG(void)
 	 */
 	InRecovery = false;
 
-	LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
-	ControlFile->state = DB_IN_PRODUCTION;
-	ControlFile->time = (pg_time_t) time(NULL);
-	UpdateControlFile();
-	LWLockRelease(ControlFileLock);
-
 	/* start the archive_timeout timer running */
 	XLogCtl->lastSegSwitchTime = (pg_time_t) time(NULL);
 
@@ -7434,15 +7428,32 @@ StartupXLOG(void)
 	CompleteCommitTsInitialization();
 
 	/*
-	 * All done.  Allow backends to write WAL.  (Although the bool flag is
-	 * probably atomic in itself, we use the info_lck here to ensure that
-	 * there are no race conditions concerning visibility of other recent
-	 * updates to shared memory.)
+	 * All done with end-of-recovery actions.
+	 *
+	 * Now, allow backends to write WAL and update the control file status
+	 * in consequence. The boolean flag allowing backends to write WAL is
+	 * updated while holding ControlFileLock to prevent other backends
+	 * to look at an inconsistent state of the control file in shared
+	 * memory. There is still a small window during which backends can
+	 * write WAL and the control file is still referring to a system not
+	 * in DB_IN_PRODUCTION state while looking at the on-disk control
+	 * file.
+	 *
+	 * Also, the boolean flag to allow WAL is probably atomic in itself,
+	 * we use the info_lck here to ensure that there are no race conditions
+	 * concerning visibility of other recent updates to shared memory.
 	 */
+	LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
+	ControlFile->state = DB_IN_PRODUCTION;
+	ControlFile->time = (pg_time_t) time(NULL);
+
 	SpinLockAcquire(&XLogCtl->info_lck);
 	XLogCtl->SharedRecoveryInProgress = false;
 	SpinLockRelease(&XLogCtl->info_lck);
 
+	UpdateControlFile();
+	LWLockRelease(ControlFileLock);
+
 	/*
 	 * If there were cascading standby servers connected to us, nudge any wal
 	 * sender processes to notice that we've been promoted.

From 371ca61ab87e792864ac2f76f3e0dad7912c247e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 26 Jul 2016 11:39:43 -0400
Subject: [PATCH 1/5] Make command_like output more compact

Consistently print the test name, not the full command, which can be
quite lenghty and include temporary directory names and other
distracting details.
---
 src/test/perl/TestLib.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 649fd82..c7b3262 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -276,8 +276,8 @@ sub command_like
 	my ($stdout, $stderr);
 	print("# Running: " . join(" ", @{$cmd}) . "\n");
 	my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
-	ok($result, "@$cmd exit code 0");
-	is($stderr, '', "@$cmd no stderr");
+	ok($result, "$test_name: exit code 0");
+	is($stderr, '', "$test_name: no stderr");
 	like($stdout, $expected_stdout, "$test_name: matches");
 }
 
-- 
2.9.2

From eabd099bf09d9a4815c29c730d568161c87a67ba Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 26 Jul 2016 10:48:43 -0400
Subject: [PATCH 2/5] pg_ctl: Add tests for promote action

---
 src/bin/pg_ctl/t/003_promote.pl | 41 +++++++++++++++++++++++++++++++++++++++++
 src/test/perl/TestLib.pm        | 11 +++++++++++
 2 files changed, 52 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..d7e7b99
--- /dev/null
+++ b/src/bin/pg_ctl/t/003_promote.pl
@@ -0,0 +1,41 @@
+use strict;
+use warnings;
+
+use PostgresNode;
+use TestLib;
+use Test::More tests => 9;
+
+my $tempdir = TestLib::tempdir;
+
+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(allows_streaming => 1);
+
+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', has_streaming => 1);
+$node_standby->start;
+
+is($node_standby->safe_psql('postgres', 'SELECT pg_is_in_recovery()'),
+   't', 'standby is in recovery');
+
+command_ok([ 'pg_ctl', 'promote', '-D', $node_standby->data_dir ],
+		   'pg_ctl promote of standby runs');
+
+$node_standby->poll_query_until('postgres', 'SELECT NOT pg_is_in_recovery()');
+
+is($node_standby->safe_psql('postgres', 'SELECT pg_is_in_recovery()'),
+   'f', 'promoted standby is not in recovery');
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index c7b3262..51b533e 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
 );
@@ -281,4 +282,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, "$test_name: exit code not 0");
+	like($stderr, $expected_stderr, "$test_name: matches");
+}
+
 1;
-- 
2.9.2

From b5bc405a85c6a84c7916f5cc2c3225438df70315 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 26 Jul 2016 11:23:43 -0400
Subject: [PATCH 3/5] 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.

Add a wait_seconds argument to get_controlfile() so that pg_ctl can wait
a bit for a consistent pg_control file.  Otherwise, pg_ctl operations
might fail on rare occasions when the server is updating the file at the
same time.
---
 src/backend/utils/misc/pg_controldata.c |  8 ++--
 src/bin/pg_controldata/pg_controldata.c |  2 +-
 src/bin/pg_ctl/pg_ctl.c                 | 35 ++++++++------
 src/common/controldata_utils.c          | 83 ++++++++++++++++++++-------------
 src/include/common/controldata_utils.h  |  2 +-
 5 files changed, 76 insertions(+), 54 deletions(-)

diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c
index 34ee76a..13f0523 100644
--- a/src/backend/utils/misc/pg_controldata.c
+++ b/src/backend/utils/misc/pg_controldata.c
@@ -51,7 +51,7 @@ pg_control_system(PG_FUNCTION_ARGS)
 	tupdesc = BlessTupleDesc(tupdesc);
 
 	/* read the control file */
-	ControlFile = get_controlfile(DataDir, NULL);
+	ControlFile = get_controlfile(DataDir, NULL, NULL);
 
 	values[0] = Int32GetDatum(ControlFile->pg_control_version);
 	nulls[0] = false;
@@ -127,7 +127,7 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
 	tupdesc = BlessTupleDesc(tupdesc);
 
 	/* Read the control file. */
-	ControlFile = get_controlfile(DataDir, NULL);
+	ControlFile = get_controlfile(DataDir, NULL, NULL);
 
 	/*
 	 * Calculate name of the WAL file containing the latest checkpoint's REDO
@@ -229,7 +229,7 @@ pg_control_recovery(PG_FUNCTION_ARGS)
 	tupdesc = BlessTupleDesc(tupdesc);
 
 	/* read the control file */
-	ControlFile = get_controlfile(DataDir, NULL);
+	ControlFile = get_controlfile(DataDir, NULL, NULL);
 
 	values[0] = LSNGetDatum(ControlFile->minRecoveryPoint);
 	nulls[0] = false;
@@ -294,7 +294,7 @@ pg_control_init(PG_FUNCTION_ARGS)
 	tupdesc = BlessTupleDesc(tupdesc);
 
 	/* read the control file */
-	ControlFile = get_controlfile(DataDir, NULL);
+	ControlFile = get_controlfile(DataDir, NULL, NULL);
 
 	values[0] = Int32GetDatum(ControlFile->maxAlign);
 	nulls[0] = false;
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index 96619a2..c107601 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -155,7 +155,7 @@ main(int argc, char *argv[])
 	}
 
 	/* get a copy of the control file */
-	ControlFile = get_controlfile(DataDir, progname);
+	ControlFile = get_controlfile(DataDir, progname, NULL);
 
 	/*
 	 * This slightly-chintzy coding will work as long as the control file
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index efc0729..fac3986 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -19,6 +19,8 @@
 
 #include "postgres_fe.h"
 
+#include "catalog/pg_control.h"
+#include "common/controldata_utils.h"
 #include "libpq-fe.h"
 #include "pqexpbuffer.h"
 
@@ -96,7 +98,6 @@ static char postopts_file[MAXPGPATH];
 static char version_file[MAXPGPATH];
 static char pid_file[MAXPGPATH];
 static char backup_file[MAXPGPATH];
-static char recovery_file[MAXPGPATH];
 static char promote_file[MAXPGPATH];
 
 #ifdef WIN32
@@ -988,15 +989,17 @@ do_stop(void)
 		/*
 		 * If backup_label exists, an online backup is running. Warn the user
 		 * that smart shutdown will wait for it to finish. However, if
-		 * recovery.conf is also present, we're recovering from an online
+		 * the server is in archive recovery, we're recovering from an online
 		 * backup instead of performing one.
 		 */
 		if (shutdown_mode == SMART_MODE &&
-			stat(backup_file, &statbuf) == 0 &&
-			stat(recovery_file, &statbuf) != 0)
+			stat(backup_file, &statbuf) == 0)
 		{
-			print_msg(_("WARNING: online backup mode is active\n"
-						"Shutdown will not complete until pg_stop_backup() is called.\n\n"));
+			ControlFileData *control_file_data = get_controlfile(pg_data, progname, &wait_seconds);
+			if (control_file_data->state != DB_IN_ARCHIVE_RECOVERY)
+				print_msg(_("WARNING: online backup mode is active\n"
+							"Shutdown will not complete until pg_stop_backup() is called.\n\n"));
+			pfree(control_file_data);
 		}
 
 		print_msg(_("waiting for server to shut down..."));
@@ -1076,15 +1079,17 @@ do_restart(void)
 		/*
 		 * If backup_label exists, an online backup is running. Warn the user
 		 * that smart shutdown will wait for it to finish. However, if
-		 * recovery.conf is also present, we're recovering from an online
+		 * the server is in archive recovery, we're recovering from an online
 		 * backup instead of performing one.
 		 */
 		if (shutdown_mode == SMART_MODE &&
-			stat(backup_file, &statbuf) == 0 &&
-			stat(recovery_file, &statbuf) != 0)
+			stat(backup_file, &statbuf) == 0)
 		{
-			print_msg(_("WARNING: online backup mode is active\n"
-						"Shutdown will not complete until pg_stop_backup() is called.\n\n"));
+			ControlFileData *control_file_data = get_controlfile(pg_data, progname, &wait_seconds);
+			if (control_file_data->state != DB_IN_ARCHIVE_RECOVERY)
+				print_msg(_("WARNING: online backup mode is active\n"
+							"Shutdown will not complete until pg_stop_backup() is called.\n\n"));
+			pfree(control_file_data);
 		}
 
 		print_msg(_("waiting for server to shut down..."));
@@ -1168,7 +1173,7 @@ do_promote(void)
 {
 	FILE	   *prmfile;
 	pgpid_t		pid;
-	struct stat statbuf;
+	ControlFileData *control_file_data;
 
 	pid = get_pgpid(false);
 
@@ -1187,14 +1192,15 @@ do_promote(void)
 		exit(1);
 	}
 
-	/* If recovery.conf doesn't exist, the server is not in standby mode */
-	if (stat(recovery_file, &statbuf) != 0)
+	control_file_data = get_controlfile(pg_data, progname, &wait_seconds);
+	if (control_file_data->state != DB_IN_ARCHIVE_RECOVERY)
 	{
 		write_stderr(_("%s: cannot promote server; "
 					   "server is not in standby mode\n"),
 					 progname);
 		exit(1);
 	}
+	pfree(control_file_data);
 
 	/*
 	 * For 9.3 onwards, "fast" promotion is performed. Promotion with a full
@@ -2401,7 +2407,6 @@ main(int argc, char **argv)
 		snprintf(version_file, MAXPGPATH, "%s/PG_VERSION", pg_data);
 		snprintf(pid_file, MAXPGPATH, "%s/postmaster.pid", pg_data);
 		snprintf(backup_file, MAXPGPATH, "%s/backup_label", pg_data);
-		snprintf(recovery_file, MAXPGPATH, "%s/recovery.conf", pg_data);
 	}
 
 	switch (ctl_command)
diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index 5592fe7..01002b0 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -35,60 +35,77 @@
  * for pfreeing the result.
  */
 ControlFileData *
-get_controlfile(char *DataDir, const char *progname)
+get_controlfile(const char *DataDir, const char *progname, int *wait_seconds)
 {
 	ControlFileData *ControlFile;
-	int			fd;
 	char		ControlFilePath[MAXPGPATH];
-	pg_crc32c	crc;
 
 	ControlFile = palloc(sizeof(ControlFileData));
 	snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
 
-	if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
+	for (;;)
+	{
+		int			fd;
+		pg_crc32c	crc;
+
+		if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
 #ifndef FRONTEND
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\" for reading: %m",
-						ControlFilePath)));
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not open file \"%s\" for reading: %m",
+							ControlFilePath)));
 #else
-	{
-		fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
-				progname, ControlFilePath, strerror(errno));
-		exit(EXIT_FAILURE);
-	}
+		{
+			fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
+					progname, ControlFilePath, strerror(errno));
+			exit(EXIT_FAILURE);
+		}
 #endif
 
-	if (read(fd, ControlFile, sizeof(ControlFileData)) != sizeof(ControlFileData))
+		if (read(fd, ControlFile, sizeof(ControlFileData)) != sizeof(ControlFileData))
 #ifndef FRONTEND
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not read file \"%s\": %m", ControlFilePath)));
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not read file \"%s\": %m", ControlFilePath)));
 #else
-	{
-		fprintf(stderr, _("%s: could not read file \"%s\": %s\n"),
-				progname, ControlFilePath, strerror(errno));
-		exit(EXIT_FAILURE);
-	}
+		{
+			fprintf(stderr, _("%s: could not read file \"%s\": %s\n"),
+					progname, ControlFilePath, strerror(errno));
+			exit(EXIT_FAILURE);
+		}
 #endif
 
-	close(fd);
+		close(fd);
+
+		/* Check the CRC. */
+		INIT_CRC32C(crc);
+		COMP_CRC32C(crc,
+					(char *) ControlFile,
+					offsetof(ControlFileData, crc));
+		FIN_CRC32C(crc);
 
-	/* Check the CRC. */
-	INIT_CRC32C(crc);
-	COMP_CRC32C(crc,
-				(char *) ControlFile,
-				offsetof(ControlFileData, crc));
-	FIN_CRC32C(crc);
+		if (EQ_CRC32C(crc, ControlFile->crc))
+			break;
+
+		if (wait_seconds && *wait_seconds > 0)
+		{
+			sleep(1);
+			(*wait_seconds)--;
+			continue;
+		}
 
-	if (!EQ_CRC32C(crc, ControlFile->crc))
 #ifndef FRONTEND
 		elog(ERROR, _("calculated CRC checksum does not match value stored in file"));
 #else
-		printf(_("WARNING: Calculated CRC checksum does not match value stored in file.\n"
-				 "Either the file is corrupt, or it has a different layout than this program\n"
-				 "is expecting.  The results below are untrustworthy.\n\n"));
+		if (wait_seconds)
+			fprintf(stderr, _("%s: control file \"%s\" appears to be corrupt\n"),
+					progname, ControlFilePath);
+		else
+			printf(_("WARNING: Calculated CRC checksum does not match value stored in file.\n"
+					 "Either the file is corrupt, or it has a different layout than this program\n"
+					 "is expecting.  The results below are untrustworthy.\n\n"));
 #endif
+	}
 
 	/* Make sure the control file is valid byte order. */
 	if (ControlFile->pg_control_version % 65536 == 0 &&
diff --git a/src/include/common/controldata_utils.h b/src/include/common/controldata_utils.h
index a355d22..283ec55 100644
--- a/src/include/common/controldata_utils.h
+++ b/src/include/common/controldata_utils.h
@@ -12,6 +12,6 @@
 
 #include "catalog/pg_control.h"
 
-extern ControlFileData *get_controlfile(char *DataDir, const char *progname);
+extern ControlFileData *get_controlfile(const char *DataDir, const char *progname, int *wait_seconds);
 
 #endif   /* COMMON_CONTROLDATA_UTILS_H */
-- 
2.9.2

From af156fbcb2e0e59045f814f4dd3d3494d0b2078d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 26 Jul 2016 12:08:49 -0400
Subject: [PATCH 4/5] Delay updating control file to "in production"

Move the updating of the control file to "in production" status until
the point where WAL writes are allowed.  Before, there could be a
significant gap between the control file update and write transactions
actually being allowed.  This makes it more reliable to use the control
status to verify the end of a promotion.

Michael Paquier
---
 src/backend/access/transam/xlog.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f13f9c1..1f8d054 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7349,12 +7349,6 @@ StartupXLOG(void)
 	 */
 	InRecovery = false;
 
-	LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
-	ControlFile->state = DB_IN_PRODUCTION;
-	ControlFile->time = (pg_time_t) time(NULL);
-	UpdateControlFile();
-	LWLockRelease(ControlFileLock);
-
 	/* start the archive_timeout timer running */
 	XLogCtl->lastSegSwitchTime = (pg_time_t) time(NULL);
 
@@ -7412,15 +7406,32 @@ StartupXLOG(void)
 	CompleteCommitTsInitialization();
 
 	/*
-	 * All done.  Allow backends to write WAL.  (Although the bool flag is
-	 * probably atomic in itself, we use the info_lck here to ensure that
-	 * there are no race conditions concerning visibility of other recent
-	 * updates to shared memory.)
+	 * All done with end-of-recovery actions.
+	 *
+	 * Now allow backends to write WAL and update the control file status in
+	 * consequence.  The boolean flag allowing backends to write WAL is
+	 * updated while holding ControlFileLock to prevent other backends to look
+	 * at an inconsistent state of the control file in shared memory.  There
+	 * is still a small window during which backends can write WAL and the
+	 * control file is still referring to a system not in DB_IN_PRODUCTION
+	 * state while looking at the on-disk control file.
+	 *
+	 * Also, although the boolean flag to allow WAL is probably atomic in
+	 * itself, we use the info_lck here to ensure that there are no race
+	 * conditions concerning visibility of other recent updates to shared
+	 * memory.
 	 */
+	LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
+	ControlFile->state = DB_IN_PRODUCTION;
+	ControlFile->time = (pg_time_t) time(NULL);
+
 	SpinLockAcquire(&XLogCtl->info_lck);
 	XLogCtl->SharedRecoveryInProgress = false;
 	SpinLockRelease(&XLogCtl->info_lck);
 
+	UpdateControlFile();
+	LWLockRelease(ControlFileLock);
+
 	/*
 	 * If there were cascading standby servers connected to us, nudge any wal
 	 * sender processes to notice that we've been promoted.
-- 
2.9.2

From 384083f9189dbd691a57f98dfefbf4c836b2d0b6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 26 Jul 2016 12:44:08 -0400
Subject: [PATCH 5/5] 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          | 50 +++++++++++++++++++++++++++++-----------
 src/bin/pg_ctl/t/003_promote.pl  | 18 ++++++++++++++-
 3 files changed, 77 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 fac3986..87cdcf3 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1233,7 +1233,39 @@ 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)
+		{
+			ControlFileData *control_file_data;
+
+			control_file_data = get_controlfile(pg_data, progname, &wait_seconds);
+			state = control_file_data->state;
+			pfree(control_file_data);
+
+			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"));
 }
 
 
@@ -2381,18 +2413,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 d7e7b99..6443926 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 => 9;
+use Test::More tests => 12;
 
 my $tempdir = TestLib::tempdir;
 
@@ -39,3 +39,19 @@
 
 is($node_standby->safe_psql('postgres', 'SELECT pg_is_in_recovery()'),
    'f', 'promoted standby is not in recovery');
+
+# same again with wait option
+$node_standby = get_new_node('standby2');
+$node_standby->init_from_backup($node_primary, 'my_backup', has_streaming => 1);
+$node_standby->start;
+
+is($node_standby->safe_psql('postgres', 'SELECT pg_is_in_recovery()'),
+   '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
+
+is($node_standby->safe_psql('postgres', 'SELECT pg_is_in_recovery()'),
+   'f', 'promoted standby is not in recovery');
-- 
2.9.2

From 04a724675769b2412b739c408abc136d17d52794 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 26 Jul 2016 11:39:43 -0400
Subject: [PATCH v2 1/5] Make command_like output more compact

Consistently print the test name, not the full command, which can be
quite lenghty and include temporary directory names and other
distracting details.
---
 src/test/perl/TestLib.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 649fd82..c7b3262 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -276,8 +276,8 @@ sub command_like
 	my ($stdout, $stderr);
 	print("# Running: " . join(" ", @{$cmd}) . "\n");
 	my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
-	ok($result, "@$cmd exit code 0");
-	is($stderr, '', "@$cmd no stderr");
+	ok($result, "$test_name: exit code 0");
+	is($stderr, '', "$test_name: no stderr");
 	like($stdout, $expected_stdout, "$test_name: matches");
 }
 
-- 
2.9.2

From b7c04b3f6cbe7a37359509363da0701c84063113 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 26 Jul 2016 10:48:43 -0400
Subject: [PATCH v2 2/5] pg_ctl: Add tests for promote action

---
 src/bin/pg_ctl/t/003_promote.pl | 39 +++++++++++++++++++++++++++++++++++++++
 src/test/perl/TestLib.pm        | 11 +++++++++++
 2 files changed, 50 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..1461234
--- /dev/null
+++ b/src/bin/pg_ctl/t/003_promote.pl
@@ -0,0 +1,39 @@
+use strict;
+use warnings;
+
+use PostgresNode;
+use TestLib;
+use Test::More tests => 9;
+
+my $tempdir = TestLib::tempdir;
+
+command_fails_like([ 'pg_ctl', '-D', "$tempdir/nonexistent", 'promote' ],
+				   qr/directory .* does not exist/,
+				   'pg_ctl promote with nonexistent directory');
+
+my $node_primary = get_new_node('primary');
+$node_primary->init(allows_streaming => 1);
+
+command_fails_like([ 'pg_ctl', '-D', $node_primary->data_dir, 'promote' ],
+				   qr/PID file .* does not exist/,
+				   'pg_ctl promote of not running instance fails');
+
+$node_primary->start;
+
+command_fails_like([ 'pg_ctl', '-D', $node_primary->data_dir, 'promote' ],
+				   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', has_streaming => 1);
+$node_standby->start;
+
+is($node_standby->safe_psql('postgres', 'SELECT pg_is_in_recovery()'),
+   't', 'standby is in recovery');
+
+command_ok([ 'pg_ctl', '-D', $node_standby->data_dir, 'promote' ],
+		   'pg_ctl promote of standby runs');
+
+ok($node_standby->poll_query_until('postgres', 'SELECT NOT pg_is_in_recovery()'),
+   'promoted standby is not in recovery');
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index c7b3262..51b533e 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
 );
@@ -281,4 +282,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, "$test_name: exit code not 0");
+	like($stderr, $expected_stderr, "$test_name: matches");
+}
+
 1;
-- 
2.9.2

From cf77e8419bae21a84a2a30d260a70bac2c19498a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 26 Jul 2016 11:23:43 -0400
Subject: [PATCH v2 3/5] 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.

Add a wait_seconds argument to get_controlfile() so that pg_ctl can wait
a bit for a consistent pg_control file.  Otherwise, pg_ctl operations
might fail on rare occasions when the server is updating the file at the
same time.
---
 src/backend/utils/misc/pg_controldata.c | 12 +++++++++
 src/bin/pg_controldata/pg_controldata.c |  4 +++
 src/bin/pg_ctl/pg_ctl.c                 | 47 ++++++++++++++++++++++++++-------
 src/common/controldata_utils.c          | 15 +++++------
 src/include/common/controldata_utils.h  |  2 +-
 5 files changed, 62 insertions(+), 18 deletions(-)

diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c
index 34ee76a..4f9de83 100644
--- a/src/backend/utils/misc/pg_controldata.c
+++ b/src/backend/utils/misc/pg_controldata.c
@@ -52,6 +52,9 @@ pg_control_system(PG_FUNCTION_ARGS)
 
 	/* read the control file */
 	ControlFile = get_controlfile(DataDir, NULL);
+	if (!ControlFile)
+		ereport(ERROR,
+				(errmsg("calculated CRC checksum does not match value stored in file")));
 
 	values[0] = Int32GetDatum(ControlFile->pg_control_version);
 	nulls[0] = false;
@@ -128,6 +131,9 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
 
 	/* Read the control file. */
 	ControlFile = get_controlfile(DataDir, NULL);
+	if (!ControlFile)
+		ereport(ERROR,
+				(errmsg("calculated CRC checksum does not match value stored in file")));
 
 	/*
 	 * Calculate name of the WAL file containing the latest checkpoint's REDO
@@ -230,6 +236,9 @@ pg_control_recovery(PG_FUNCTION_ARGS)
 
 	/* read the control file */
 	ControlFile = get_controlfile(DataDir, NULL);
+	if (!ControlFile)
+		ereport(ERROR,
+				(errmsg("calculated CRC checksum does not match value stored in file")));
 
 	values[0] = LSNGetDatum(ControlFile->minRecoveryPoint);
 	nulls[0] = false;
@@ -295,6 +304,9 @@ pg_control_init(PG_FUNCTION_ARGS)
 
 	/* read the control file */
 	ControlFile = get_controlfile(DataDir, NULL);
+	if (!ControlFile)
+		ereport(ERROR,
+				(errmsg("calculated CRC checksum does not match value stored in file")));
 
 	values[0] = Int32GetDatum(ControlFile->maxAlign);
 	nulls[0] = false;
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index 96619a2..e92feab 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -156,6 +156,10 @@ main(int argc, char *argv[])
 
 	/* get a copy of the control file */
 	ControlFile = get_controlfile(DataDir, progname);
+	if (!ControlFile)
+		printf(_("WARNING: Calculated CRC checksum does not match value stored in file.\n"
+				 "Either the file is corrupt, or it has a different layout than this program\n"
+				 "is expecting.  The results below are untrustworthy.\n\n"));
 
 	/*
 	 * This slightly-chintzy coding will work as long as the control file
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index efc0729..85f6a92 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -19,6 +19,8 @@
 
 #include "postgres_fe.h"
 
+#include "catalog/pg_control.h"
+#include "common/controldata_utils.h"
 #include "libpq-fe.h"
 #include "pqexpbuffer.h"
 
@@ -96,7 +98,6 @@ static char postopts_file[MAXPGPATH];
 static char version_file[MAXPGPATH];
 static char pid_file[MAXPGPATH];
 static char backup_file[MAXPGPATH];
-static char recovery_file[MAXPGPATH];
 static char promote_file[MAXPGPATH];
 
 #ifdef WIN32
@@ -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
@@ -988,12 +991,12 @@ do_stop(void)
 		/*
 		 * If backup_label exists, an online backup is running. Warn the user
 		 * that smart shutdown will wait for it to finish. However, if
-		 * recovery.conf is also present, we're recovering from an online
+		 * the server is in archive recovery, we're recovering from an online
 		 * backup instead of performing one.
 		 */
 		if (shutdown_mode == SMART_MODE &&
 			stat(backup_file, &statbuf) == 0 &&
-			stat(recovery_file, &statbuf) != 0)
+			get_control_dbstate() != DB_IN_ARCHIVE_RECOVERY)
 		{
 			print_msg(_("WARNING: online backup mode is active\n"
 						"Shutdown will not complete until pg_stop_backup() is called.\n\n"));
@@ -1076,12 +1079,12 @@ do_restart(void)
 		/*
 		 * If backup_label exists, an online backup is running. Warn the user
 		 * that smart shutdown will wait for it to finish. However, if
-		 * recovery.conf is also present, we're recovering from an online
+		 * the server is in archive recovery, we're recovering from an online
 		 * backup instead of performing one.
 		 */
 		if (shutdown_mode == SMART_MODE &&
 			stat(backup_file, &statbuf) == 0 &&
-			stat(recovery_file, &statbuf) != 0)
+			get_control_dbstate() != DB_IN_ARCHIVE_RECOVERY)
 		{
 			print_msg(_("WARNING: online backup mode is active\n"
 						"Shutdown will not complete until pg_stop_backup() is called.\n\n"));
@@ -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,35 @@ adjust_data_dir(void)
 }
 
 
+static DBState
+get_control_dbstate(void)
+{
+	DBState ret;
+
+	for (;;)
+	{
+		ControlFileData *control_file_data = get_controlfile(pg_data, progname);
+
+		if (control_file_data)
+		{
+			ret = control_file_data->state;
+			pfree(control_file_data);
+			return ret;
+		}
+
+		if (wait_seconds > 0)
+		{
+			sleep(1);
+			wait_seconds--;
+			continue;
+		}
+
+		write_stderr(_("%s: control file appears to be corrupt\n"), progname);
+		exit(1);
+	}
+}
+
+
 int
 main(int argc, char **argv)
 {
@@ -2401,7 +2431,6 @@ main(int argc, char **argv)
 		snprintf(version_file, MAXPGPATH, "%s/PG_VERSION", pg_data);
 		snprintf(pid_file, MAXPGPATH, "%s/postmaster.pid", pg_data);
 		snprintf(backup_file, MAXPGPATH, "%s/backup_label", pg_data);
-		snprintf(recovery_file, MAXPGPATH, "%s/recovery.conf", pg_data);
 	}
 
 	switch (ctl_command)
diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index 5592fe7..f218d25 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -33,9 +33,11 @@
  *
  * Get controlfile values. The caller is responsible
  * for pfreeing the result.
+ *
+ * Returns NULL if the CRC did not match.
  */
 ControlFileData *
-get_controlfile(char *DataDir, const char *progname)
+get_controlfile(const char *DataDir, const char *progname)
 {
 	ControlFileData *ControlFile;
 	int			fd;
@@ -82,13 +84,10 @@ get_controlfile(char *DataDir, const char *progname)
 	FIN_CRC32C(crc);
 
 	if (!EQ_CRC32C(crc, ControlFile->crc))
-#ifndef FRONTEND
-		elog(ERROR, _("calculated CRC checksum does not match value stored in file"));
-#else
-		printf(_("WARNING: Calculated CRC checksum does not match value stored in file.\n"
-				 "Either the file is corrupt, or it has a different layout than this program\n"
-				 "is expecting.  The results below are untrustworthy.\n\n"));
-#endif
+	{
+		pfree(ControlFile);
+		return NULL;
+	}
 
 	/* Make sure the control file is valid byte order. */
 	if (ControlFile->pg_control_version % 65536 == 0 &&
diff --git a/src/include/common/controldata_utils.h b/src/include/common/controldata_utils.h
index a355d22..f834624 100644
--- a/src/include/common/controldata_utils.h
+++ b/src/include/common/controldata_utils.h
@@ -12,6 +12,6 @@
 
 #include "catalog/pg_control.h"
 
-extern ControlFileData *get_controlfile(char *DataDir, const char *progname);
+extern ControlFileData *get_controlfile(const char *DataDir, const char *progname);
 
 #endif   /* COMMON_CONTROLDATA_UTILS_H */
-- 
2.9.2

From 4938adf0f28ecf83ca5b0d6332b83383d916866e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 26 Jul 2016 12:08:49 -0400
Subject: [PATCH v2 4/5] Delay updating control file to "in production"

Move the updating of the control file to "in production" status until
the point where WAL writes are allowed.  Before, there could be a
significant gap between the control file update and write transactions
actually being allowed.  This makes it more reliable to use the control
status to verify the end of a promotion.

Michael Paquier
---
 src/backend/access/transam/xlog.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f13f9c1..1f8d054 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7349,12 +7349,6 @@ StartupXLOG(void)
 	 */
 	InRecovery = false;
 
-	LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
-	ControlFile->state = DB_IN_PRODUCTION;
-	ControlFile->time = (pg_time_t) time(NULL);
-	UpdateControlFile();
-	LWLockRelease(ControlFileLock);
-
 	/* start the archive_timeout timer running */
 	XLogCtl->lastSegSwitchTime = (pg_time_t) time(NULL);
 
@@ -7412,15 +7406,32 @@ StartupXLOG(void)
 	CompleteCommitTsInitialization();
 
 	/*
-	 * All done.  Allow backends to write WAL.  (Although the bool flag is
-	 * probably atomic in itself, we use the info_lck here to ensure that
-	 * there are no race conditions concerning visibility of other recent
-	 * updates to shared memory.)
+	 * All done with end-of-recovery actions.
+	 *
+	 * Now allow backends to write WAL and update the control file status in
+	 * consequence.  The boolean flag allowing backends to write WAL is
+	 * updated while holding ControlFileLock to prevent other backends to look
+	 * at an inconsistent state of the control file in shared memory.  There
+	 * is still a small window during which backends can write WAL and the
+	 * control file is still referring to a system not in DB_IN_PRODUCTION
+	 * state while looking at the on-disk control file.
+	 *
+	 * Also, although the boolean flag to allow WAL is probably atomic in
+	 * itself, we use the info_lck here to ensure that there are no race
+	 * conditions concerning visibility of other recent updates to shared
+	 * memory.
 	 */
+	LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
+	ControlFile->state = DB_IN_PRODUCTION;
+	ControlFile->time = (pg_time_t) time(NULL);
+
 	SpinLockAcquire(&XLogCtl->info_lck);
 	XLogCtl->SharedRecoveryInProgress = false;
 	SpinLockRelease(&XLogCtl->info_lck);
 
+	UpdateControlFile();
+	LWLockRelease(ControlFileLock);
+
 	/*
 	 * If there were cascading standby servers connected to us, nudge any wal
 	 * sender processes to notice that we've been promoted.
-- 
2.9.2

From 88d426cfb25f02aa990fe24b71a55fb4c316daff Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Fri, 5 Aug 2016 21:35:19 -0400
Subject: [PATCH v2 5/5] 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  | 18 +++++++++++++++-
 3 files changed, 72 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 85f6a92..82840bb 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"));
 }
 
 
@@ -2405,18 +2432,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 1461234..0b6090b 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 => 9;
+use Test::More tests => 12;
 
 my $tempdir = TestLib::tempdir;
 
@@ -37,3 +37,19 @@
 
 ok($node_standby->poll_query_until('postgres', 'SELECT NOT pg_is_in_recovery()'),
    'promoted standby is not in recovery');
+
+# same again with wait option
+$node_standby = get_new_node('standby2');
+$node_standby->init_from_backup($node_primary, 'my_backup', has_streaming => 1);
+$node_standby->start;
+
+is($node_standby->safe_psql('postgres', 'SELECT pg_is_in_recovery()'),
+   't', 'standby is in recovery');
+
+command_ok([ 'pg_ctl', '-D', $node_standby->data_dir, '-w', 'promote' ],
+		   'pg_ctl -w promote of standby runs');
+
+# no wait here
+
+is($node_standby->safe_psql('postgres', 'SELECT pg_is_in_recovery()'),
+   'f', 'promoted standby is not in recovery');
-- 
2.9.2

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 2f0976a..17643cd 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -1943,7 +1943,7 @@ do_help(void)
 			 "                 [-o \"OPTIONS\"]\n"), progname);
 	printf(_("  %s reload  [-D DATADIR] [-s]\n"), progname);
 	printf(_("  %s status  [-D DATADIR]\n"), progname);
-	printf(_("  %s promote [-D DATADIR] [-s]\n"), progname);
+	printf(_("  %s promote [-w] [-t SECS] [-D DATADIR] [-s]\n"), progname);
 	printf(_("  %s kill    SIGNALNAME PID\n"), progname);
 #ifdef WIN32
 	printf(_("  %s register   [-N SERVICENAME] [-U USERNAME] [-P PASSWORD] [-D DATADIR]\n"