Turning off hot_standby_feedback
There is a bug in hot_standby_feedback that causes the xmin of the
walsender process to not be reset when hot_standby_feedback is turned
off after it had previously sent at least one feedback message.
Clearly, that is a bad thing and deserves backpatching to 9.1, unless
I hear otherwise real soon.
Attached patch sends an immediate feedback message if it is turned off
on the standby. No protocol changes, just changes to how message is
handled and when it is sent.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
hs_feedback_off.v1.patchapplication/octet-stream; name=hs_feedback_off.v1.patchDownload
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 1a75059..d326266 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1682,8 +1682,10 @@ The commands accepted in walsender mode are:
</term>
<listitem>
<para>
- The standby's current xmin. This may be 0, if the standby does not
- support feedback, or is not yet in Hot Standby state.
+ The standby's current xmin. This may be 0, if the standby is
+ sending notification that Hot Standby feedback will no longer
+ be sent on this connection. Later non-zero messages may
+ reinitiate the feedback mechanism.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 7359297..37d5e08 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -47,6 +47,7 @@
#include <unistd.h>
#include "access/timeline.h"
+#include "access/transam.h"
#include "access/xlog_internal.h"
#include "libpq/pqformat.h"
#include "libpq/pqsignal.h"
@@ -138,7 +139,7 @@ static void XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len);
static void XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr);
static void XLogWalRcvFlush(bool dying);
static void XLogWalRcvSendReply(bool force, bool requestReply);
-static void XLogWalRcvSendHSFeedback(void);
+static void XLogWalRcvSendHSFeedback(bool immed);
static void ProcessWalSndrMessage(XLogRecPtr walEnd, TimestampTz sendTime);
/* Signal handlers */
@@ -406,6 +407,7 @@ WalReceiverMain(void)
{
got_SIGHUP = false;
ProcessConfigFile(PGC_SIGHUP);
+ XLogWalRcvSendHSFeedback(true);
}
/* Wait a while for data to arrive */
@@ -496,7 +498,7 @@ WalReceiverMain(void)
}
XLogWalRcvSendReply(requestReply, requestReply);
- XLogWalRcvSendHSFeedback();
+ XLogWalRcvSendHSFeedback(false);
}
}
@@ -1059,46 +1061,60 @@ XLogWalRcvSendReply(bool force, bool requestReply)
/*
* Send hot standby feedback message to primary, plus the current time,
* in case they don't have a watch.
+ *
+ * If the user disables feedback, send one final message to tell sender
+ * to forget about the xmin on this standby.
*/
static void
-XLogWalRcvSendHSFeedback(void)
+XLogWalRcvSendHSFeedback(bool immed)
{
TimestampTz now;
TransactionId nextXid;
uint32 nextEpoch;
TransactionId xmin;
static TimestampTz sendTime = 0;
+ static bool master_has_standby_xmin = false;
/*
* If the user doesn't want status to be reported to the master, be sure
* to exit before doing anything at all.
*/
- if (wal_receiver_status_interval <= 0 || !hot_standby_feedback)
+ if ((wal_receiver_status_interval <= 0 || !hot_standby_feedback) &&
+ !master_has_standby_xmin)
return;
/* Get current timestamp. */
now = GetCurrentTimestamp();
- /*
- * Send feedback at most once per wal_receiver_status_interval.
- */
- if (!TimestampDifferenceExceeds(sendTime, now,
+ if (!immed)
+ {
+ /*
+ * Send feedback at most once per wal_receiver_status_interval.
+ */
+ if (!TimestampDifferenceExceeds(sendTime, now,
wal_receiver_status_interval * 1000))
- return;
- sendTime = now;
+ return;
+ sendTime = now;
+ }
/*
* If Hot Standby is not yet active there is nothing to send. Check this
* after the interval has expired to reduce number of calls.
*/
if (!HotStandbyActive())
+ {
+ Assert(!master_has_standby_xmin);
return;
+ }
/*
* Make the expensive call to get the oldest xmin once we are certain
* everything else has been checked.
*/
- xmin = GetOldestXmin(true, false);
+ if (hot_standby_feedback)
+ xmin = GetOldestXmin(true, false);
+ else
+ xmin = InvalidTransactionId;
/*
* Get epoch and adjust if nextXid and oldestXmin are different sides of
@@ -1118,6 +1134,10 @@ XLogWalRcvSendHSFeedback(void)
pq_sendint(&reply_message, xmin, 4);
pq_sendint(&reply_message, nextEpoch, 4);
walrcv_send(reply_message.data, reply_message.len);
+ if (TransactionIdIsValid(xmin))
+ master_has_standby_xmin = true;
+ else
+ master_has_standby_xmin = false;
}
/*
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 10e4050..490b121 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -870,9 +870,12 @@ ProcessStandbyHSFeedbackMessage(void)
feedbackXmin,
feedbackEpoch);
- /* Ignore invalid xmin (can't actually happen with current walreceiver) */
+ /* Unset WalSender's xmin if the feedback message value is invalid */
if (!TransactionIdIsNormal(feedbackXmin))
+ {
+ MyPgXact->xmin = InvalidTransactionId;
return;
+ }
/*
* Check that the provided xmin/epoch are sane, that is, not in the future
On Sun, Feb 3, 2013 at 5:25 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
There is a bug in hot_standby_feedback that causes the xmin of the
walsender process to not be reset when hot_standby_feedback is turned
off after it had previously sent at least one feedback message.Clearly, that is a bad thing and deserves backpatching to 9.1, unless
I hear otherwise real soon.
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4 February 2013 15:52, Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, Feb 3, 2013 at 5:25 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
There is a bug in hot_standby_feedback that causes the xmin of the
walsender process to not be reset when hot_standby_feedback is turned
off after it had previously sent at least one feedback message.Clearly, that is a bad thing and deserves backpatching to 9.1, unless
I hear otherwise real soon.+1.
Patches for 9.1 and 9.2 different to the one for HEAD.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
hs_feedback_off.92.v1.patchapplication/octet-stream; name=hs_feedback_off.92.v1.patchDownload
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 3d72a16..08c2f00 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1617,7 +1617,10 @@ The commands accepted in walsender mode are:
</term>
<listitem>
<para>
- The standby's current xmin.
+ The standby's current xmin. This may be 0, if the standby is
+ sending notification that Hot Standby feedback will no
+ longer be sent on this connection. Later non-zero messages may
+ reinitiate the feedback mechanism.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 7a0102d..5cbabc1 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -38,6 +38,7 @@
#include <signal.h>
#include <unistd.h>
+#include "access/transam.h"
#include "access/xlog_internal.h"
#include "libpq/pqsignal.h"
#include "miscadmin.h"
@@ -123,7 +124,7 @@ static void XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len);
static void XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr);
static void XLogWalRcvFlush(bool dying);
static void XLogWalRcvSendReply(void);
-static void XLogWalRcvSendHSFeedback(void);
+static void XLogWalRcvSendHSFeedback(bool immed);
static void ProcessWalSndrMessage(XLogRecPtr walEnd, TimestampTz sendTime);
/* Signal handlers */
@@ -312,6 +313,7 @@ WalReceiverMain(void)
{
got_SIGHUP = false;
ProcessConfigFile(PGC_SIGHUP);
+ XLogWalRcvSendHSFeedback(true);
}
/* Wait a while for data to arrive */
@@ -340,7 +342,7 @@ WalReceiverMain(void)
* master anyway, to report any progress in applying WAL.
*/
XLogWalRcvSendReply();
- XLogWalRcvSendHSFeedback();
+ XLogWalRcvSendHSFeedback(false);
}
}
}
@@ -621,7 +623,7 @@ XLogWalRcvFlush(bool dying)
if (!dying)
{
XLogWalRcvSendReply();
- XLogWalRcvSendHSFeedback();
+ XLogWalRcvSendHSFeedback(false);
}
}
}
@@ -681,45 +683,62 @@ XLogWalRcvSendReply(void)
/*
* Send hot standby feedback message to primary, plus the current time,
* in case they don't have a watch.
+ *
+ * If the user disables feedback, send one final message to tell sender
+ * to forget about the xmin on this standby.
*/
static void
-XLogWalRcvSendHSFeedback(void)
+XLogWalRcvSendHSFeedback(bool immed)
{
char buf[sizeof(StandbyHSFeedbackMessage) + 1];
TimestampTz now;
TransactionId nextXid;
uint32 nextEpoch;
TransactionId xmin;
+ static TimestampTz sendTime = 0;
+ static bool master_has_standby_xmin = false;
/*
* If the user doesn't want status to be reported to the master, be sure
* to exit before doing anything at all.
*/
- if (wal_receiver_status_interval <= 0 || !hot_standby_feedback)
+ if ((wal_receiver_status_interval <= 0 || !hot_standby_feedback) &&
+ !master_has_standby_xmin)
return;
/* Get current timestamp. */
now = GetCurrentTimestamp();
- /*
- * Send feedback at most once per wal_receiver_status_interval.
- */
- if (!TimestampDifferenceExceeds(feedback_message.sendTime, now,
+ if (!immed)
+ {
+ /*
+ * Send feedback at most once per wal_receiver_status_interval.
+ */
+ if (!TimestampDifferenceExceeds(sendTime, now,
wal_receiver_status_interval * 1000))
- return;
+ return;
+ }
+
+ sendTime = now;
/*
* If Hot Standby is not yet active there is nothing to send. Check this
* after the interval has expired to reduce number of calls.
*/
if (!HotStandbyActive())
+ {
+ Assert(!master_has_standby_xmin);
return;
+ }
/*
* Make the expensive call to get the oldest xmin once we are certain
* everything else has been checked.
*/
- xmin = GetOldestXmin(true, false);
+ if (hot_standby_feedback)
+ xmin = GetOldestXmin(true, false);
+ else
+ xmin = InvalidTransactionId;
/*
* Get epoch and adjust if nextXid and oldestXmin are different sides of
@@ -744,6 +763,10 @@ XLogWalRcvSendHSFeedback(void)
buf[0] = 'h';
memcpy(&buf[1], &feedback_message, sizeof(StandbyHSFeedbackMessage));
walrcv_send(buf, sizeof(StandbyHSFeedbackMessage) + 1);
+ if (TransactionIdIsValid(xmin))
+ master_has_standby_xmin = true;
+ else
+ master_has_standby_xmin = false;
}
/*
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 6c27449..e8f654f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -655,9 +655,12 @@ ProcessStandbyHSFeedbackMessage(void)
reply.xmin,
reply.epoch);
- /* Ignore invalid xmin (can't actually happen with current walreceiver) */
+ /* Unset WalSender's xmin if the feedback message value is invalid */
if (!TransactionIdIsNormal(reply.xmin))
+ {
+ MyPgXact->xmin = InvalidTransactionId;
return;
+ }
/*
* Check that the provided xmin/epoch are sane, that is, not in the future
hs_feedback_off.91.v1.patchapplication/octet-stream; name=hs_feedback_off.91.v1.patchDownload
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index e743880..8a0755e 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1584,7 +1584,10 @@ The commands accepted in walsender mode are:
</term>
<listitem>
<para>
- The standby's current xmin.
+ The standby's current xmin. This may be 0, if the standby is
+ sending notification that Hot Standby feedback will no
+ longer be sent on this connection. Later non-zero messages may
+ reinitiate the feedback mechanism.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 7058b88..e6a5350 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -124,7 +124,7 @@ static void XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len);
static void XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr);
static void XLogWalRcvFlush(bool dying);
static void XLogWalRcvSendReply(void);
-static void XLogWalRcvSendHSFeedback(void);
+static void XLogWalRcvSendHSFeedback(bool immed);
/* Signal handlers */
static void WalRcvSigHupHandler(SIGNAL_ARGS);
@@ -310,6 +310,7 @@ WalReceiverMain(void)
{
got_SIGHUP = false;
ProcessConfigFile(PGC_SIGHUP);
+ XLogWalRcvSendHSFeedback(true);
}
/* Wait a while for data to arrive */
@@ -338,7 +339,7 @@ WalReceiverMain(void)
* master anyway, to report any progress in applying WAL.
*/
XLogWalRcvSendReply();
- XLogWalRcvSendHSFeedback();
+ XLogWalRcvSendHSFeedback(false);
}
}
}
@@ -591,7 +592,7 @@ XLogWalRcvFlush(bool dying)
if (!dying)
{
XLogWalRcvSendReply();
- XLogWalRcvSendHSFeedback();
+ XLogWalRcvSendHSFeedback(false);
}
}
}
@@ -651,45 +652,62 @@ XLogWalRcvSendReply(void)
/*
* Send hot standby feedback message to primary, plus the current time,
* in case they don't have a watch.
+ *
+ * If the user disables feedback, send one final message to tell sender
+ * to forget about the xmin on this standby.
*/
static void
-XLogWalRcvSendHSFeedback(void)
+XLogWalRcvSendHSFeedback(bool immed)
{
char buf[sizeof(StandbyHSFeedbackMessage) + 1];
TimestampTz now;
TransactionId nextXid;
uint32 nextEpoch;
TransactionId xmin;
+ static TimestampTz sendTime = 0;
+ static bool master_has_standby_xmin = false;
/*
* If the user doesn't want status to be reported to the master, be sure
* to exit before doing anything at all.
*/
- if (wal_receiver_status_interval <= 0 || !hot_standby_feedback)
+ if ((wal_receiver_status_interval <= 0 || !hot_standby_feedback) &&
+ !master_has_standby_xmin)
return;
/* Get current timestamp. */
now = GetCurrentTimestamp();
- /*
- * Send feedback at most once per wal_receiver_status_interval.
- */
- if (!TimestampDifferenceExceeds(feedback_message.sendTime, now,
+ if (!immed)
+ {
+ /*
+ * Send feedback at most once per wal_receiver_status_interval.
+ */
+ if (!TimestampDifferenceExceeds(sendTime, now,
wal_receiver_status_interval * 1000))
- return;
+ return;
+ }
+
+ sendTime = now;
/*
* If Hot Standby is not yet active there is nothing to send. Check this
* after the interval has expired to reduce number of calls.
*/
if (!HotStandbyActive())
+ {
+ Assert(!master_has_standby_xmin);
return;
+ }
/*
* Make the expensive call to get the oldest xmin once we are certain
* everything else has been checked.
*/
- xmin = GetOldestXmin(true, false);
+ if (hot_standby_feedback)
+ xmin = GetOldestXmin(true, false);
+ else
+ xmin = InvalidTransactionId;
/*
* Get epoch and adjust if nextXid and oldestXmin are different sides of
@@ -714,4 +732,8 @@ XLogWalRcvSendHSFeedback(void)
buf[0] = 'h';
memcpy(&buf[1], &feedback_message, sizeof(StandbyHSFeedbackMessage));
walrcv_send(buf, sizeof(StandbyHSFeedbackMessage) + 1);
+ if (TransactionIdIsValid(xmin))
+ master_has_standby_xmin = true;
+ else
+ master_has_standby_xmin = false;
}
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 7ccab20..0a2e654 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -622,9 +622,12 @@ ProcessStandbyHSFeedbackMessage(void)
reply.xmin,
reply.epoch);
- /* Ignore invalid xmin (can't actually happen with current walreceiver) */
+ /* Unset WalSender's xmin if the feedback message value is invalid */
if (!TransactionIdIsNormal(reply.xmin))
+ {
+ MyProc->xmin = InvalidTransactionId;
return;
+ }
/*
* Check that the provided xmin/epoch are sane, that is, not in the future