On 7/2/15 3:37 AM, Michael Paquier wrote:
> Regarding patch 3, I have more comments:
> 1) I think that documentation should clearly mention that if -R and -S
> are used together, a primary_slot_name entry is added in the
> recovery.conf generated with the slot name defined.
Updated proposal attached.
> 2)
> 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;
> }
> RewindTest.pm has a routine called check_query defined. I would be
> great to extend a bit more psql than what I thought previously by
> returning from it ($result, $stdout, $strerr) and make check_query
> directly use it. This way we have a unique interface to run psql and
> capture output. I don't mind writing this refactoring patch on top of
> your set if that's necessary.
Let's do that as a separate patch. Adding multiple return values makes
calling awkward, so I'd like to sort out the actual use cases before we
do that.
> 3)
> +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"),
> slurp_file is slurping an incorrect file and $recovery_conf is used
> nowhere after, so you could simply remove this line.
Yeah, that was some leftover garbage.
>From 7ecb1794c2aaeea9753af07e2f54f6e483af255f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 21 Jul 2015 21:06:45 -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 | 27 ++++++++++++++++++++++++---
src/bin/pg_basebackup/pg_basebackup.c | 24 +++++++++++++++++++++++-
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 22 +++++++++++++++++++++-
src/test/perl/TestLib.pm | 5 ++++-
4 files changed, 72 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index cb8b8a3..5f8b9b7 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -215,15 +215,36 @@ <title>Options</title>
<listitem>
<para>
- Write a minimal <filename>recovery.conf</filename> in the output directory (or into
- the base archive file when using tar format) to ease setting
- up a standby server.
+ Write a minimal <filename>recovery.conf</filename> in the output
+ directory (or into the base archive file when using tar format) to
+ ease setting up a standby server.
+ The <filename>recovery.conf</filename> file will record the connection
+ settings and, if specified, the replication slot
+ that <application>pg_basebackup</application> is using, so that the
+ streaming replication will use the same settings later on.
</para>
</listitem>
</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 5363680..80de882 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 bf9fdcf..b605fa9 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,22 @@
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');
+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 ca91be9..4fcf8b9 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -168,8 +168,11 @@ END
sub psql
{
my ($dbname, $sql) = @_;
+ my ($stdout, $stderr);
print("# Running SQL command: $sql\n");
- run [ 'psql', '-X', '-q', '-d', $dbname, '-f', '-' ], '<', \$sql or die;
+ run [ 'psql', '-X', '-A', '-t', '-q', '-d', $dbname, '-f', '-' ], '<', \$sql, '>', \$stdout, '2>', \$stderr or die;
+ chomp $stdout;
+ return $stdout;
}
sub slurp_dir
--
2.4.6
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
