On 5/21/15 8:42 AM, Peter Eisentraut wrote:
> I wonder why pg_basebackup doesn't have any support for replication slots.
> 
> When relying on replication slots to hang on to WAL data, there is a gap
> between when pg_basebackup finishes and streaming replication is started
> where WAL data could be thrown away by the primary.
> 
> Looking at the code, the -X stream method could easily specify a
> replication slot.  (Might be nice if it could also create it in the same
> run.)

Here is a patch set for this.

(If you're looking at the patch and wondering why there is no code to
actually do anything with the replication slot, that's because the code
that does the WAL streaming is already aware of replication slots
because of the pg_receivexlog functionality that it also serves.  So all
that needs to be done is set replication_slot.)

See also the concurrent discussion on (optionally?) initializing
restart_lsn when the replication slot is created
(http://www.postgresql.org/message-id/flat/b8d538ac5587c84898b261fcb8c7d8a41fde1...@ex10-mbx-36009.ant.amazon.com#b8d538ac5587c84898b261fcb8c7d8a41fde1...@ex10-mbx-36009.ant.amazon.com),
which might have an impact on the details of this change.


>From a718282cbc18566732a0d9c3e82c8fca752f4812 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Tue, 30 Jun 2015 21:15:05 -0400
Subject: [PATCH 1/3] pg_basebackup: Add tests for -R option

---
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 9 ++++++++-
 src/test/perl/TestLib.pm                     | 8 ++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index c8c9250..0ddfffe 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -2,7 +2,7 @@
 use warnings;
 use Cwd;
 use TestLib;
-use Test::More tests => 35;
+use Test::More tests => 39;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -138,3 +138,10 @@
 command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
 	'pg_basebackup tar with long symlink target');
 psql 'postgres', "DROP TABLESPACE tblspc3;";
+
+command_ok([ 'pg_basebackup', '-D', "$tempdir/backupR", '-R' ],
+	'pg_basebackup -R runs');
+ok(-f "$tempdir/backupR/recovery.conf", 'recovery.conf was created');
+my $recovery_conf = slurp_file "$tempdir/backupR/recovery.conf";
+like($recovery_conf, qr/^standby_mode = 'on'$/m, 'recovery.conf sets standby_mode');
+like($recovery_conf, qr/^primary_conninfo = '.*port=$ENV{PGPORT}.*'$/m, 'recovery.conf sets primary_conninfo');
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index ef42366..60bd7d5 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -11,6 +11,7 @@ our @EXPORT = qw(
   start_test_server
   restart_test_server
   psql
+  slurp_file
   system_or_bail
 
   command_ok
@@ -129,6 +130,13 @@ sub psql
 	run [ 'psql', '-X', '-q', '-d', $dbname, '-f', '-' ], '<', \$sql or die;
 }
 
+sub slurp_file
+{
+	local $/;
+	local @ARGV = @_;
+	<>
+}
+
 sub system_or_bail
 {
 	system(@_) == 0 or BAIL_OUT("system @_ failed: $?");
-- 
2.4.5

>From 934309249f1fed9fd008eccd32906fbfd763811a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Tue, 30 Jun 2015 21:15:29 -0400
Subject: [PATCH 2/3] pg_basebackup: Add tests for -X option

---
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 13 ++++++++++++-
 src/test/perl/TestLib.pm                     | 10 ++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 0ddfffe..52f1575 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -2,7 +2,7 @@
 use warnings;
 use Cwd;
 use TestLib;
-use Test::More tests => 39;
+use Test::More tests => 44;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -46,6 +46,10 @@
 	'pg_basebackup runs');
 ok(-f "$tempdir/backup/PG_VERSION", 'backup was created');
 
+is_deeply([sort(slurp_dir("$tempdir/backup/pg_xlog/"))],
+		  [sort qw(. .. archive_status)],
+		  'no WAL files copied');
+
 command_ok(
 	[   'pg_basebackup', '-D', "$tempdir/backup2", '--xlogdir',
 		"$tempdir/xlog2" ],
@@ -145,3 +149,10 @@
 my $recovery_conf = slurp_file "$tempdir/backupR/recovery.conf";
 like($recovery_conf, qr/^standby_mode = 'on'$/m, 'recovery.conf sets standby_mode');
 like($recovery_conf, qr/^primary_conninfo = '.*port=$ENV{PGPORT}.*'$/m, 'recovery.conf sets primary_conninfo');
+
+command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxf", '-X', 'fetch' ],
+	'pg_basebackup -X fetch runs');
+ok(grep(/^[0-9A-F]{24}$/, slurp_dir("$tempdir/backupxf/pg_xlog")), 'WAL files copied');
+command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxs", '-X', 'stream' ],
+	'pg_basebackup -X stream runs');
+ok(grep(/^[0-9A-F]{24}$/, slurp_dir("$tempdir/backupxf/pg_xlog")), 'WAL files copied');
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 60bd7d5..cde95e6 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -11,6 +11,7 @@ our @EXPORT = qw(
   start_test_server
   restart_test_server
   psql
+  slurp_dir
   slurp_file
   system_or_bail
 
@@ -130,6 +131,15 @@ sub psql
 	run [ 'psql', '-X', '-q', '-d', $dbname, '-f', '-' ], '<', \$sql or die;
 }
 
+sub slurp_dir
+{
+	my ($dir) = @_;
+	opendir(my $dh, $dir) or die;
+	my @direntries = readdir $dh;
+	closedir $dh;
+	return @direntries;
+}
+
 sub slurp_file
 {
 	local $/;
-- 
2.4.5

>From bb87d51eaeb0a0258093b9eaaa0253aa009af3ac Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Tue, 30 Jun 2015 21:15:35 -0400
Subject: [PATCH 3/3] pg_basebackup: Add --slot option

This option specifies a replication slot for WAL streaming (-X stream),
so that there can be continuous replication slot use between WAL
streaming during the base backup and the start of regular streaming
replication.
---
 doc/src/sgml/ref/pg_basebackup.sgml          | 17 +++++++++++++++++
 src/bin/pg_basebackup/pg_basebackup.c        | 24 +++++++++++++++++++++++-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 23 ++++++++++++++++++++++-
 src/test/perl/TestLib.pm                     |  5 ++++-
 4 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index cb8b8a3..a6e08cf 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -224,6 +224,23 @@ <title>Options</title>
      </varlistentry>
 
      <varlistentry>
+      <term><option>-S <replaceable>slotname</replaceable></option></term>
+      <term><option>--slot=<replaceable class="parameter">slotname</replaceable></option></term>
+      <listitem>
+       <para>
+        This option can only be used together with <literal>-X
+        stream</literal>.  It causes the WAL streaming to use the specified
+        replication slot.  If the base backup is intended to be used as a
+        streaming replication standby using replication slots, it should then
+        use the same replication slot name
+        in <filename>recovery.conf</filename>.  That way, it is ensured that
+        the server does not remove any necessary WAL data in the time between
+        the end of the base backup and the start of streaming replication.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
       <term><option>-T <replaceable class="parameter">olddir</replaceable>=<replaceable class="parameter">newdir</replaceable></option></term>
       <term><option>--tablespace-mapping=<replaceable class="parameter">olddir</replaceable>=<replaceable class="parameter">newdir</replaceable></option></term>
       <listitem>
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 5dd2887..9607f29 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -239,6 +239,7 @@ usage(void)
 	  "                         (in kB/s, or use suffix \"k\" or \"M\")\n"));
 	printf(_("  -R, --write-recovery-conf\n"
 			 "                         write recovery.conf after backup\n"));
+	printf(_("  -S, --slot=SLOTNAME    replication slot to use\n"));
 	printf(_("  -T, --tablespace-mapping=OLDDIR=NEWDIR\n"
 	  "                         relocate tablespace in OLDDIR to NEWDIR\n"));
 	printf(_("  -x, --xlog             include required WAL files in backup (fetch mode)\n"));
@@ -1536,6 +1537,13 @@ GenerateRecoveryConf(PGconn *conn)
 	appendPQExpBuffer(recoveryconfcontents, "primary_conninfo = '%s'\n", escaped);
 	free(escaped);
 
+	if (replication_slot)
+	{
+		escaped = escape_quotes(replication_slot);
+		appendPQExpBuffer(recoveryconfcontents, "primary_slot_name = '%s'\n", replication_slot);
+		free(escaped);
+	}
+
 	if (PQExpBufferBroken(recoveryconfcontents) ||
 		PQExpBufferDataBroken(conninfo_buf))
 	{
@@ -1934,6 +1942,7 @@ main(int argc, char **argv)
 		{"checkpoint", required_argument, NULL, 'c'},
 		{"max-rate", required_argument, NULL, 'r'},
 		{"write-recovery-conf", no_argument, NULL, 'R'},
+		{"slot", required_argument, NULL, 'S'},
 		{"tablespace-mapping", required_argument, NULL, 'T'},
 		{"xlog", no_argument, NULL, 'x'},
 		{"xlog-method", required_argument, NULL, 'X'},
@@ -1974,7 +1983,7 @@ main(int argc, char **argv)
 		}
 	}
 
-	while ((c = getopt_long(argc, argv, "D:F:r:RT:xX:l:zZ:d:c:h:p:U:s:wWvP",
+	while ((c = getopt_long(argc, argv, "D:F:r:RT:xX:l:zZ:d:c:h:p:U:s:S:wWvP",
 							long_options, &option_index)) != -1)
 	{
 		switch (c)
@@ -2001,6 +2010,9 @@ main(int argc, char **argv)
 			case 'R':
 				writerecoveryconf = true;
 				break;
+			case 'S':
+				replication_slot = pg_strdup(optarg);
+				break;
 			case 'T':
 				tablespace_list_append(optarg);
 				break;
@@ -2165,6 +2177,16 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
+	if (replication_slot && !streamwal)
+	{
+		fprintf(stderr,
+				_("%s: replication slots can only be used with WAL streaming\n"),
+				progname);
+		fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+				progname);
+		exit(1);
+	}
+
 	if (strcmp(xlog_dir, "") != 0)
 	{
 		if (format != 'p')
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 52f1575..653d63f 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -2,7 +2,7 @@
 use warnings;
 use Cwd;
 use TestLib;
-use Test::More tests => 44;
+use Test::More tests => 51;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -37,6 +37,7 @@
 	'pg_basebackup fails because of WAL configuration');
 
 open CONF, ">>$tempdir/pgdata/postgresql.conf";
+print CONF "max_replication_slots = 10\n";
 print CONF "max_wal_senders = 10\n";
 print CONF "wal_level = archive\n";
 close CONF;
@@ -156,3 +157,23 @@
 command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxs", '-X', 'stream' ],
 	'pg_basebackup -X stream runs');
 ok(grep(/^[0-9A-F]{24}$/, slurp_dir("$tempdir/backupxf/pg_xlog")), 'WAL files copied');
+
+command_fails([ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1' ],
+	'pg_basebackup with replication slot fails without -X stream');
+command_fails([ 'pg_basebackup', '-D', "$tempdir/backupxs_sl_fail", '-X', 'stream', '-S', 'slot1' ],
+	'pg_basebackup fails with nonexistent replication slot');
+
+psql 'postgres', q{SELECT * FROM pg_create_physical_replication_slot('slot1')};
+my $lsn = psql 'postgres', q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'slot1'};
+is($lsn, '', 'restart LSN of new slot is null');
+command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxs_sl", '-X', 'stream', '-S', 'slot1' ],
+	'pg_basebackup -X stream with replication slot runs');
+$lsn = psql 'postgres', q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'slot1'};
+like($lsn, qr!^0/[0-9A-Z]{8}$!, 'restart LSN of slot has advanced');
+
+command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxs_sl_R", '-X', 'stream', '-S', 'slot1', '-R' ],
+	'pg_basebackup with replication slot and -R runs');
+$recovery_conf = slurp_file "$tempdir/backupR/recovery.conf";
+like(slurp_file("$tempdir/backupxs_sl_R/recovery.conf"),
+	 qr/^primary_slot_name = 'slot1'$/m,
+	 'recovery.conf sets primary_slot_name');
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index cde95e6..d8ffd0c 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -128,7 +128,10 @@ END
 sub psql
 {
 	my ($dbname, $sql) = @_;
-	run [ 'psql', '-X', '-q', '-d', $dbname, '-f', '-' ], '<', \$sql or die;
+	my ($stdout, $stderr);
+	run [ 'psql', '-X', '-A', '-t', '-q', '-d', $dbname, '-f', '-' ], '<', \$sql, '>', \$stdout, '2>', \$stderr or die;
+	chomp $stdout;
+	return $stdout;
 }
 
 sub slurp_dir
-- 
2.4.5

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to