On Monday, January 6, 2020 6:57:33 PM CET Tom Lane wrote:
> Pierre Ducroquet <p.p...@pinaraf.info> writes:
> > Attached to this email is a patch with better comments regarding the
> > XLogSendLogical change.
>
> Hi,
> This patch entirely fails to apply for me (and for the cfbot too).
> It looks like (a) it's missing a final newline and (b) all the tabs
> have been mangled into spaces, and not correctly mangled either.
> I could probably reconstruct a workable patch if I had to, but
> it seems likely that it'd be easier for you to resend it with a
> little more care about attaching an unmodified attachment.
>
> As for the question of back-patching, it seems to me that it'd
> likely be reasonable to put this into v12, but probably not
> further back. There will be no interest in back-patching
> commit cfdf4dc4f, and it seems like the argument for this
> patch is relatively weak without that.
>
> regards, tom lane
Hi
My deepest apologies for the patch being broken, I messed up when transferring
it between my computers after altering the comments. The verbatim one attached
to this email applies with no issue on current HEAD.
The patch regarding PostmasterIsAlive is completely pointless since v12 where
the function was rewritten, and was included only to help reproduce the issue
on older versions. Back-patching the walsender patch further than v12 would
imply back-patching all the machinery introduced for PostmasterIsAlive
(9f09529952) or another intrusive change there, a too big risk indeed.
Regards
Pierre
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 6e80e67590..4b57db6fc2 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2774,7 +2774,13 @@ XLogSendLogical(void)
{
XLogRecord *record;
char *errm;
- XLogRecPtr flushPtr;
+
+ /*
+ * We'll use the current flush point to determine whether we've caught up.
+ * This variable is static in order to cache it accross calls. This caching
+ * is needed because calling GetFlushRecPtr needs to acquire an expensive lock.
+ */
+ static XLogRecPtr flushPtr = InvalidXLogRecPtr;
/*
* Don't know whether we've caught up yet. We'll set WalSndCaughtUp to
@@ -2791,11 +2797,6 @@ XLogSendLogical(void)
if (errm != NULL)
elog(ERROR, "%s", errm);
- /*
- * We'll use the current flush point to determine whether we've caught up.
- */
- flushPtr = GetFlushRecPtr();
-
if (record != NULL)
{
/*
@@ -2808,7 +2809,15 @@ XLogSendLogical(void)
sentPtr = logical_decoding_ctx->reader->EndRecPtr;
}
- /* Set flag if we're caught up. */
+ /* Initialize flushPtr if needed */
+ if (flushPtr == InvalidXLogRecPtr)
+ flushPtr = GetFlushRecPtr();
+
+ /* If EndRecPtr is past our flushPtr, we must update it to know if we caught up */
+ if (logical_decoding_ctx->reader->EndRecPtr >= flushPtr)
+ flushPtr = GetFlushRecPtr();
+
+ /* If EndRecPtr is still past our flushPtr, it means we caught up */
if (logical_decoding_ctx->reader->EndRecPtr >= flushPtr)
WalSndCaughtUp = true;
