On Fri, Dec 16, 2016 at 7:40 AM, Michael Paquier <[email protected]>
wrote:
> On Fri, Dec 16, 2016 at 3:32 PM, Magnus Hagander <[email protected]>
> wrote:
> > I don't really know how to write a good test for it. I mean, we could
> run it
> > with the parameter, but how to we make it actually verify the slot? Make
> a
> > really big database to make it guaranteed to be slow enough that we can
> > notice it in a different session?
>
> No, not that. I was meaning tests to trigger the error code paths with
> the compatibility switches you are adding.
>
There is only one switch - --no-slot. I guess you mean rune one backup with
that one? Sure, that's easy enough to do. I'm not sure how much it really
tests, but let's do it :)
You mean something like this, right?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 1f15a17..f912ed0 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -246,6 +246,20 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--no-slot</option></term>
+ <listitem>
+ <para>
+ This option prevents the creation of a temporary replication slot
+ during the backup even if it's supported by the server.
+ </para>
+ <para>
+ Temporary replication slots are created by default if no slot name
+ is given with the <literal>-S</literal> when using log streaming.
+ </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 f7ba9a9..fc171c1 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -61,6 +61,11 @@ typedef struct TablespaceList
*/
#define MINIMUM_VERSION_FOR_PG_WAL 100000
+/*
+ * Temporary replication slots are supported from version 10.
+ */
+#define MINIMUM_VERSION_FOR_TEMP_SLOTS 100000
+
/* Global options */
static char *basedir = NULL;
static TablespaceList tablespace_dirs = {NULL, NULL};
@@ -79,6 +84,8 @@ static bool do_sync = true;
static int standby_message_timeout = 10 * 1000; /* 10 sec = default */
static pg_time_t last_progress_report = 0;
static int32 maxrate = 0; /* no limit by default */
+static char *replication_slot = NULL;
+static bool temp_replication_slot = true;
static bool success = false;
static bool made_new_pgdata = false;
@@ -323,6 +330,7 @@ usage(void)
printf(_(" -R, --write-recovery-conf\n"
" write recovery.conf after backup\n"));
printf(_(" -S, --slot=SLOTNAME replication slot to use\n"));
+ printf(_(" --no-slot prevent creation of temporary replication slot\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"));
@@ -452,6 +460,7 @@ typedef struct
char xlog[MAXPGPATH]; /* directory or tarfile depending on mode */
char *sysidentifier;
int timeline;
+ bool temp_slot;
} logstreamer_param;
static int
@@ -471,6 +480,16 @@ LogStreamerMain(logstreamer_param *param)
stream.do_sync = do_sync;
stream.mark_done = true;
stream.partial_suffix = NULL;
+ stream.replication_slot = replication_slot;
+ stream.temp_slot = param->temp_slot;
+ if (stream.temp_slot && !stream.replication_slot)
+ {
+ /* Generate a name for the temporary slot */
+ char name[32];
+
+ snprintf(name, sizeof(name), "pg_basebackup_%d", getpid());
+ stream.replication_slot = pstrdup(name);
+ }
if (format == 'p')
stream.walmethod = CreateWalDirectoryMethod(param->xlog, do_sync);
@@ -557,6 +576,11 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier)
PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ?
"pg_xlog" : "pg_wal");
+ /* Temporary replication slots are only supported in 10 and newer */
+ if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_TEMP_SLOTS)
+ param->temp_slot = false;
+ else
+ param->temp_slot = temp_replication_slot;
if (format == 'p')
{
@@ -2052,11 +2076,13 @@ main(int argc, char **argv)
{"verbose", no_argument, NULL, 'v'},
{"progress", no_argument, NULL, 'P'},
{"xlogdir", required_argument, NULL, 1},
+ {"no-slot", no_argument, NULL, 2},
{NULL, 0, NULL, 0}
};
int c;
int option_index;
+ bool no_slot = false;
progname = get_progname(argv[0]);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_basebackup"));
@@ -2106,7 +2132,16 @@ main(int argc, char **argv)
writerecoveryconf = true;
break;
case 'S':
+
+ /*
+ * When specifying replication slot name, use a permanent
+ * slot.
+ */
replication_slot = pg_strdup(optarg);
+ temp_replication_slot = false;
+ break;
+ case 2:
+ no_slot = true;
break;
case 'T':
tablespace_list_append(optarg);
@@ -2268,7 +2303,7 @@ main(int argc, char **argv)
exit(1);
}
- if (replication_slot && !streamwal)
+ if ((replication_slot || no_slot) && !streamwal)
{
fprintf(stderr,
_("%s: replication slots can only be used with WAL streaming\n"),
@@ -2278,6 +2313,20 @@ main(int argc, char **argv)
exit(1);
}
+ if (no_slot)
+ {
+ if (replication_slot)
+ {
+ fprintf(stderr,
+ _("%s: --no-slot cannot be used with slot name\n"),
+ progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+ temp_replication_slot = false;
+ }
+
if (strcmp(xlog_dir, "") != 0)
{
if (format != 'p')
diff --git a/src/bin/pg_basebackup/pg_receivexlog.c b/src/bin/pg_basebackup/pg_receivexlog.c
index 99445e6..e8389f6 100644
--- a/src/bin/pg_basebackup/pg_receivexlog.c
+++ b/src/bin/pg_basebackup/pg_receivexlog.c
@@ -41,6 +41,7 @@ static bool do_create_slot = false;
static bool slot_exists_ok = false;
static bool do_drop_slot = false;
static bool synchronous = false;
+static char *replication_slot = NULL;
static void usage(void);
@@ -340,6 +341,8 @@ StreamLog(void)
stream.mark_done = false;
stream.walmethod = CreateWalDirectoryMethod(basedir, stream.do_sync);
stream.partial_suffix = ".partial";
+ stream.replication_slot = replication_slot;
+ stream.temp_slot = false;
ReceiveXlogStream(conn, &stream);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index cb5f989..be4d78a 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -44,6 +44,7 @@ static bool do_create_slot = false;
static bool slot_exists_ok = false;
static bool do_start_slot = false;
static bool do_drop_slot = false;
+static char *replication_slot = NULL;
/* filled pairwise with option, value. value may be NULL */
static char **options;
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 4382e5d..7d04882 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -455,10 +455,10 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
* synchronous_standby_names, but we've protected them against it so
* far, so let's continue to do so unless specifically requested.
*/
- if (replication_slot != NULL)
+ if (stream->replication_slot != NULL)
{
reportFlushPosition = true;
- sprintf(slotcmd, "SLOT \"%s\" ", replication_slot);
+ sprintf(slotcmd, "SLOT \"%s\" ", stream->replication_slot);
}
else
{
@@ -509,6 +509,24 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
}
/*
+ * Create temporary replication slot if one is needed
+ */
+ if (stream->temp_slot)
+ {
+ snprintf(query, sizeof(query),
+ "CREATE_REPLICATION_SLOT \"%s\" TEMPORARY PHYSICAL RESERVE_WAL",
+ stream->replication_slot);
+ res = PQexec(conn, query);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ {
+ fprintf(stderr, _("%s: could not create temporary replication slot \"%s\": %s\n"),
+ progname, stream->replication_slot, PQerrorMessage(conn));
+ PQclear(res);
+ return false;
+ }
+ }
+
+ /*
* initialize flush position to starting point, it's the caller's
* responsibility that that's sane.
*/
diff --git a/src/bin/pg_basebackup/receivelog.h b/src/bin/pg_basebackup/receivelog.h
index b5913ea..01776d3 100644
--- a/src/bin/pg_basebackup/receivelog.h
+++ b/src/bin/pg_basebackup/receivelog.h
@@ -37,13 +37,15 @@ typedef struct StreamCtl
* often */
bool synchronous; /* Flush immediately WAL data on write */
bool mark_done; /* Mark segment as done in generated archive */
- bool do_sync; /* Flush to disk to ensure consistent state
- * of data */
+ bool do_sync; /* Flush to disk to ensure consistent state of
+ * data */
stream_stop_callback stream_stop; /* Stop streaming when returns true */
WalWriteMethod *walmethod; /* How to write the WAL */
char *partial_suffix; /* Suffix appended to partially received files */
+ char *replication_slot; /* Replication slot to use, or NULL */
+ bool temp_slot; /* Create temporary replication slot */
} StreamCtl;
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 595eaff..7f2e078 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -38,7 +38,6 @@ char *connection_string = NULL;
char *dbhost = NULL;
char *dbuser = NULL;
char *dbport = NULL;
-char *replication_slot = NULL;
char *dbname = NULL;
int dbgetpassword = 0; /* 0=auto, -1=never, 1=always */
static bool have_password = false;
diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h
index d2d5a6d..33d6afb 100644
--- a/src/bin/pg_basebackup/streamutil.h
+++ b/src/bin/pg_basebackup/streamutil.h
@@ -23,7 +23,6 @@ extern char *dbuser;
extern char *dbport;
extern char *dbname;
extern int dbgetpassword;
-extern char *replication_slot;
/* Connection kept global so we can disconnect easily */
extern PGconn *conn;
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 7811093..8e786c1 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -4,7 +4,7 @@ use Cwd;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 69;
+use Test::More tests => 70;
program_help_ok('pg_basebackup');
program_version_ok('pg_basebackup');
@@ -239,6 +239,9 @@ $node->command_ok(
[ 'pg_basebackup', '-D', "$tempdir/backupxst", '-X', 'stream', '-Ft' ],
'pg_basebackup -X stream runs in tar mode');
ok(-f "$tempdir/backupxst/pg_wal.tar", "tar file was created");
+$node->command_ok(
+ [ 'pg_basebackup', '-D', "$tempdir/backupnoslot", '-X', 'stream', '--no-slot' ],
+ 'pg_basebackup -X stream runs with --no-slot');
$node->command_fails(
[ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1' ],
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
