Hello
> Thanks for posting this patch, Sergei. Here is a review to make
> things move on.
Thank you, here is updated patch
> The set of comments you are removing from walreceiver.c to decide if a
> temporary slot needs to be created or not should be moved to
> walreceiverfuncs.c as you move the logic from the WAL receiver startup
> phase to the moment the WAL receiver spawn is requested.
I changed this comments because they describes behavior during change value of
wal_receiver_create_temp_slot.
But yes, I need to add some comments to RequestXLogStreaming.
> It would be more consistent with primary_conn_info and
> primary_slot_name if wal_receiver_create_temp_slot is passed down as
> an argument of RequestXLogStreaming().
Yep, I thought about that. Changed.
> As per the discussion done on this thread, let's also switch the
> parameter default to be disabled.
Done (my vote is also for disabling this option by default).
regards, Sergei
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c1128f89ec..683d87c491 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4140,11 +4140,7 @@ ANY <replaceable class="parameter">num_sync</replaceable> ( <replaceable class="
Specifies whether a WAL receiver should create a temporary replication
slot on the remote instance when no permanent replication slot to use
has been configured (using <xref linkend="guc-primary-slot-name"/>).
- The default is on. The only reason to turn this off would be if the
- remote instance is currently out of available replication slots. This
- parameter can only be set in the <filename>postgresql.conf</filename>
- file or on the server command line. Changes only take effect when the
- WAL receiver process starts a new connection.
+ The default is off. This parameter can only be set at server start.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3813eadfb4..e0f3ed5c2a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -283,6 +283,7 @@ bool StandbyModeRequested = false;
char *PrimaryConnInfo = NULL;
char *PrimarySlotName = NULL;
char *PromoteTriggerFile = NULL;
+bool wal_receiver_create_temp_slot = false;
/* are we currently in standby mode? */
bool StandbyMode = false;
@@ -11901,7 +11902,7 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
}
curFileTLI = tli;
RequestXLogStreaming(tli, ptr, PrimaryConnInfo,
- PrimarySlotName);
+ PrimarySlotName, wal_receiver_create_temp_slot);
receivedUpto = 0;
}
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 2ab15c3cbb..01452a175f 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -73,7 +73,6 @@
/* GUC variables */
-bool wal_receiver_create_temp_slot;
int wal_receiver_status_interval;
int wal_receiver_timeout;
bool hot_standby_feedback;
@@ -348,42 +347,23 @@ WalReceiverMain(void)
WalRcvFetchTimeLineHistoryFiles(startpointTLI, primaryTLI);
/*
- * Create temporary replication slot if no slot name is configured or
- * the slot from the previous run was temporary, unless
- * wal_receiver_create_temp_slot is disabled. We also need to handle
- * the case where the previous run used a temporary slot but
- * wal_receiver_create_temp_slot was changed in the meantime. In that
- * case, we delete the old slot name in shared memory. (This would
+ * Create temporary replication slot if requested. In that
+ * case, we update slot name in shared memory. (This would
* all be a bit easier if we just didn't copy the slot name into
* shared memory, since we won't need it again later, but then we
* can't see the slot name in the stats views.)
*/
- if (slotname[0] == '\0' || is_temp_slot)
+ if (is_temp_slot)
{
- bool changed = false;
+ snprintf(slotname, sizeof(slotname),
+ "pg_walreceiver_%lld",
+ (long long int) walrcv_get_backend_pid(wrconn));
- if (wal_receiver_create_temp_slot)
- {
- snprintf(slotname, sizeof(slotname),
- "pg_walreceiver_%lld",
- (long long int) walrcv_get_backend_pid(wrconn));
-
- walrcv_create_slot(wrconn, slotname, true, 0, NULL);
- changed = true;
- }
- else if (slotname[0] != '\0')
- {
- slotname[0] = '\0';
- changed = true;
- }
+ walrcv_create_slot(wrconn, slotname, true, 0, NULL);
- if (changed)
- {
- SpinLockAcquire(&walrcv->mutex);
- strlcpy(walrcv->slotname, slotname, NAMEDATALEN);
- walrcv->is_temp_slot = wal_receiver_create_temp_slot;
- SpinLockRelease(&walrcv->mutex);
- }
+ SpinLockAcquire(&walrcv->mutex);
+ strlcpy(walrcv->slotname, slotname, NAMEDATALEN);
+ SpinLockRelease(&walrcv->mutex);
}
/*
diff --git a/src/backend/replication/walreceiverfuncs.c b/src/backend/replication/walreceiverfuncs.c
index 89c903e45a..ee0dd693b2 100644
--- a/src/backend/replication/walreceiverfuncs.c
+++ b/src/backend/replication/walreceiverfuncs.c
@@ -216,12 +216,13 @@ ShutdownWalRcv(void)
* Request postmaster to start walreceiver.
*
* recptr indicates the position where streaming should begin, conninfo
- * is a libpq connection string to use, and slotname is, optionally, the name
- * of a replication slot to acquire.
+ * is a libpq connection string to use, slotname is, optionally, the name
+ * of a replication slot to acquire, create_temp_slot indicates to create
+ * a temporary slot when no slotname was given.
*/
void
RequestXLogStreaming(TimeLineID tli, XLogRecPtr recptr, const char *conninfo,
- const char *slotname)
+ const char *slotname, bool create_temp_slot)
{
WalRcvData *walrcv = WalRcv;
bool launch = false;
@@ -248,10 +249,22 @@ RequestXLogStreaming(TimeLineID tli, XLogRecPtr recptr, const char *conninfo,
else
walrcv->conninfo[0] = '\0';
- if (slotname != NULL)
+ /*
+ * Use configured replication slot if present. We ignore create_temp_slot
+ * option here because slotname should be persistent. Otherwise, use
+ * create_temp_slot to determine whether walreceiver should create a
+ * temporary slot itself or not use replication slots at all.
+ */
+ if (slotname != NULL && slotname[0] != '\0')
+ {
strlcpy((char *) walrcv->slotname, slotname, NAMEDATALEN);
+ walrcv->is_temp_slot = false;
+ }
else
+ {
walrcv->slotname[0] = '\0';
+ walrcv->is_temp_slot = create_temp_slot;
+ }
if (walrcv->walRcvState == WALRCV_STOPPED)
{
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 8228e1f390..ec9cd3696c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2029,11 +2029,11 @@ static struct config_bool ConfigureNamesBool[] =
},
{
- {"wal_receiver_create_temp_slot", PGC_SIGHUP, REPLICATION_STANDBY,
+ {"wal_receiver_create_temp_slot", PGC_POSTMASTER, REPLICATION_STANDBY,
gettext_noop("Sets whether a WAL receiver should create a temporary replication slot if no permanent slot is configured."),
},
&wal_receiver_create_temp_slot,
- true,
+ false,
NULL, NULL, NULL
},
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index e1048c0047..d2604a98b4 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -321,7 +321,8 @@
#max_standby_streaming_delay = 30s # max delay before canceling queries
# when reading streaming WAL;
# -1 allows indefinite delay
-#wal_receiver_create_temp_slot = on # create temp slot if primary_slot_name not set
+#wal_receiver_create_temp_slot = off # create temp slot if primary_slot_name not set
+ # (change requires restart)
#wal_receiver_status_interval = 10s # send replies at least this often
# 0 disables
#hot_standby_feedback = off # send info from standby to prevent
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 98b033fc20..12362421d7 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -129,6 +129,7 @@ extern int recoveryTargetAction;
extern int recovery_min_apply_delay;
extern char *PrimaryConnInfo;
extern char *PrimarySlotName;
+extern bool wal_receiver_create_temp_slot;
/* indirectly set via GUC system */
extern TransactionId recoveryTargetXid;
diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h
index e08afc6548..cf3e43128c 100644
--- a/src/include/replication/walreceiver.h
+++ b/src/include/replication/walreceiver.h
@@ -23,7 +23,6 @@
#include "utils/tuplestore.h"
/* user-settable parameters */
-extern bool wal_receiver_create_temp_slot;
extern int wal_receiver_status_interval;
extern int wal_receiver_timeout;
extern bool hot_standby_feedback;
@@ -321,7 +320,8 @@ extern void ShutdownWalRcv(void);
extern bool WalRcvStreaming(void);
extern bool WalRcvRunning(void);
extern void RequestXLogStreaming(TimeLineID tli, XLogRecPtr recptr,
- const char *conninfo, const char *slotname);
+ const char *conninfo, const char *slotname,
+ bool create_temp_slot);
extern XLogRecPtr GetWalRcvWriteRecPtr(XLogRecPtr *latestChunkStart, TimeLineID *receiveTLI);
extern int GetReplicationApplyDelay(void);
extern int GetReplicationTransferLatency(void);
