On Thu, Dec 15, 2016 at 10:26 AM, Magnus Hagander <mag...@hagander.net>
wrote:
>
>
> On Thu, Dec 15, 2016 at 10:04 AM, Magnus Hagander <mag...@hagander.net>
> wrote:
>
>> I've started work on a patch to make pg_basebackup use the temporary
>> slots feature that has been committed (thanks Petr!!). The reason for this
>> is to avoid the cases where a burst of traffic on the master during the
>> backup can cause the receive log part of the basebackup to fall far enough
>> behind that it fails.
>>
>> I have a few considerations at this point, about interaction with
>> existing options.
>>
>> Right now, we have -S/--slot which specifies a slot name. If you want to
>> use that, you have to create the slot ahead of time, and it will be a
>> permanent slot (of course). This is primarily documented as a feature to
>> use for replication (to make sure xlog is kept around until the standby is
>> started up), but it does also solve the same problem. But to use it for
>> base backups today you have to manually create the slot, then base backup,
>> then drop the slot, which is error prone.
>>
>> My thought is that if -S/--slot is not specified, but -X stream is, then
>> we use a temporary slot always. This obviously requires the server to be
>> configured with enough slots (I still think we should change the default
>> here, but that's a different topic), but I think that's acceptable. Then we
>> should add a "--no-slot" to make it revert to previous behaviour.
>>
>> Does that seem reasonable? Or would people prefer it to default to off?
>>
>>
So here's a patch that does this, for discussion. It implements the
following behavior for -X:
* When used with <10.0 servers, behave just like before.
* When -S <name> is specified, behave just like before (use an existing
replication slot, fail if it does not exist)
* When used on 10.0 with no -S, create and use a temporary replication slot
while running, with name pg_basebackup_<pid>.
* When used with 10.0 with no -S but --no-slot specified, run without a
slot like before.
--
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 e2875df..85bc7ef 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;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
