On 21 December 2016 at 21:03, Ants Aasma <ants.aa...@eesti.ee> wrote:
> There was a !HotStandbyActive() check there previously, I was not sure
> if it was only to avoid sending useless messages or was necessary
> because something was not initialized otherwise. Looks like it is not
> needed and can be removed. Revised patch that does so attached.
Ah, see, I'm blind. Too much multi-tasking. Turns out patch review
with a toddler's help is hard ;)
HotStandbyActive() call is actually just checking if we're still in
crash or archive recovery, per
/*
* SharedHotStandbyActive indicates if we're still in crash or archive
* recovery. Protected by info_lck.
*/
bool SharedHotStandbyActive;
so it is appropriate to defer any hot standby feedback until then. By
that point we've definitely finished setting up replication slots for
one thing, and know for sure if we have something useful to say and
won't send the wrong thing.
It looks to me like we should continue to bail out if !HotStandbyActive() . The
Assert(!master_has_standby_xmin)
can go, since there's now a valid case where master_has_standby_xmin
can be true before HotStandbyActive() is true.
Here's a revised version. I haven't run it against your tests yet.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 5d86d42e30ccb3dd3e1b227b102384762d66e9dd Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Wed, 21 Dec 2016 21:28:32 +0800
Subject: [PATCH] Reset xmin if hot_standby_feedback is turned off while
shutdown
If the user turns hot_standby_feedback off while we're shut down or the
walreciever is restarting, we won't remember that we sent an xmin to the
server. This never used to matter since the xmin was tied to the walsender
backend PGPROC, but now that we have replication slots it's persistent across
connections.
Always send at least one hot_standby_feedback message after walreceiver
startup, even if hot_standby_feedback is off, to ensure any saved
slot xmin from a prior connection is cleared.
Ants Aasma, Craig Ringer
---
src/backend/replication/walreceiver.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index cc3cf7d..11de996 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -1157,7 +1157,10 @@ XLogWalRcvSendReply(bool force, bool requestReply)
* 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.
+ * to forget about the xmin on this standby. We also send this message
+ * on first connect because a previous connection might have set xmin
+ * on a replication slot. (If we're not using a slot it's harmless to
+ * send a feedback message explicitly setting InvalidTransactionId).
*/
static void
XLogWalRcvSendHSFeedback(bool immed)
@@ -1167,7 +1170,8 @@ XLogWalRcvSendHSFeedback(bool immed)
uint32 nextEpoch;
TransactionId xmin;
static TimestampTz sendTime = 0;
- static bool master_has_standby_xmin = false;
+ /* initially true so we always send at least one feedback message */
+ static bool master_has_standby_xmin = true;
/*
* If the user doesn't want status to be reported to the master, be sure
@@ -1192,14 +1196,17 @@ XLogWalRcvSendHSFeedback(bool immed)
}
/*
- * 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 Hot Standby is not yet accepting connections there is nothing to
+ * send. Check this after the interval has expired to reduce number of
+ * calls.
+ *
+ * Bailing out here also ensures that we don't send feedback until we've
+ * read our own replication slot state, so we don't tell the master to
+ * discard needed xmin or catalog_xmin from any slots that may exist
+ * on this replica.
*/
if (!HotStandbyActive())
- {
- Assert(!master_has_standby_xmin);
return;
- }
/*
* Make the expensive call to get the oldest xmin once we are certain
--
2.5.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
