From 2a428c76d58bfaed024660238d62c0e0a77e9fca Mon Sep 17 00:00:00 2001 From: wangw Date: Thu, 21 Apr 2022 15:01:02 +0800 Subject: [PATCH v1] Fix the logical replication timeout during large transactions. The problem is that we don't send keep-alive messages for a long time while processing large transactions during logical replication where we don't send any data of such transactions. This can happen when the table modified in the transaction is not published. We do try to send the keep_alive if necessary at the end of the transaction (via WalSndWriteData()) but by that time the subscriber-side can timeout and exit. To fix this we try to send the keepalive message if required after processing certain threshold of changes. --- src/backend/replication/logical/logical.c | 51 ++++++++++++++++++++- src/backend/replication/pgoutput/pgoutput.c | 4 ++ src/backend/replication/walsender.c | 39 ++++++++++++++-- src/include/replication/logical.h | 5 ++ 4 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 61902be3b0..91283f1de7 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -546,15 +546,38 @@ OutputPluginWrite(struct LogicalDecodingContext *ctx, bool last_write) } /* - * Update progress tracking (if supported). + * Update progress tracking and try to send a keepalive message (if supported). + * + * For a large transaction, if we don't send any change to the downstream for a + * long time (exceeds the wal_receiver_timeout of standby) then it can timeout. + * This can happen when all or most of the changes are not published. */ void OutputPluginUpdateProgress(struct LogicalDecodingContext *ctx) { + static int changes_count = 0; + if (!ctx->update_progress) return; - ctx->update_progress(ctx, ctx->write_location, ctx->write_xid); + /* + * We don't want to try sending a keepalive message after processing each + * change as that can have overhead. Tests revealed that there is no + * noticeable overhead in doing it after continuously processing 100 or so + * changes. + */ +#define CHANGES_THRESHOLD 100 + + /* + * If we are at the end of transaction LSN, update progress tracking. + * Otherwise, after continuously processing CHANGES_THRESHOLD changes, we + * try to send a keepalive message if required. + */ + if (ctx->end_xact || ++changes_count >= CHANGES_THRESHOLD) + { + ctx->update_progress(ctx, ctx->write_location, ctx->write_xid); + changes_count = 0; + } } /* @@ -623,6 +646,9 @@ startup_cb_wrapper(LogicalDecodingContext *ctx, OutputPluginOptions *opt, bool i /* set output state */ ctx->accept_writes = false; + /* set lsn state */ + ctx->end_xact = false; + /* do the actual work: call callback */ ctx->callbacks.startup_cb(ctx, opt, is_init); @@ -650,6 +676,9 @@ shutdown_cb_wrapper(LogicalDecodingContext *ctx) /* set output state */ ctx->accept_writes = false; + /* set lsn state */ + ctx->end_xact = false; + /* do the actual work: call callback */ ctx->callbacks.shutdown_cb(ctx); @@ -685,6 +714,9 @@ begin_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn) ctx->write_xid = txn->xid; ctx->write_location = txn->first_lsn; + /* set lsn state */ + ctx->end_xact = false; + /* do the actual work: call callback */ ctx->callbacks.begin_cb(ctx, txn); @@ -716,6 +748,9 @@ commit_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn, ctx->write_xid = txn->xid; ctx->write_location = txn->end_lsn; /* points to the end of the record */ + /* set lsn state */ + ctx->end_xact = true; + /* do the actual work: call callback */ ctx->callbacks.commit_cb(ctx, txn, commit_lsn); @@ -754,6 +789,9 @@ change_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn, */ ctx->write_location = change->lsn; + /* set lsn state */ + ctx->end_xact = false; + ctx->callbacks.change_cb(ctx, txn, relation, change); /* Pop the error context stack */ @@ -794,6 +832,9 @@ truncate_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn, */ ctx->write_location = change->lsn; + /* set lsn state */ + ctx->end_xact = false; + ctx->callbacks.truncate_cb(ctx, txn, nrelations, relations, change); /* Pop the error context stack */ @@ -821,6 +862,9 @@ filter_by_origin_cb_wrapper(LogicalDecodingContext *ctx, RepOriginId origin_id) /* set output state */ ctx->accept_writes = false; + /* set lsn state */ + ctx->end_xact = false; + /* do the actual work: call callback */ ret = ctx->callbacks.filter_by_origin_cb(ctx, origin_id); @@ -858,6 +902,9 @@ message_cb_wrapper(ReorderBuffer *cache, ReorderBufferTXN *txn, ctx->write_xid = txn != NULL ? txn->xid : InvalidTransactionId; ctx->write_location = message_lsn; + /* set lsn state */ + ctx->end_xact = false; + /* do the actual work: call callback */ ctx->callbacks.message_cb(ctx, txn, message_lsn, transactional, prefix, message_size, message); diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 79735da21a..2ae00a70cb 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -385,6 +385,8 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, RelationSyncEntry *relentry; Relation ancestor = NULL; + OutputPluginUpdateProgress(ctx); + if (!is_publishable_relation(relation)) return; @@ -514,6 +516,8 @@ pgoutput_truncate(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, int nrelids; Oid *relids; + OutputPluginUpdateProgress(ctx); + old = MemoryContextSwitchTo(data->context); relids = palloc0(nrelations * sizeof(Oid)); diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index fe0d2ee2ed..b885fac833 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -240,6 +240,7 @@ static void ProcessStandbyMessage(void); static void ProcessStandbyReplyMessage(void); static void ProcessStandbyHSFeedbackMessage(void); static void ProcessRepliesIfAny(void); +static void ProcessPendingWrites(void); static void WalSndKeepalive(bool requestReply); static void WalSndKeepaliveIfNecessary(void); static void WalSndCheckTimeOut(void); @@ -1295,6 +1296,16 @@ WalSndWriteData(LogicalDecodingContext *ctx, XLogRecPtr lsn, TransactionId xid, } /* If we have pending write here, go to slow path */ + ProcessPendingWrites(); +} + +/* + * Wait until there is no pending write. Also process replies from the other + * side and check timeouts during that. + */ +static void +ProcessPendingWrites(void) +{ for (;;) { int wakeEvents; @@ -1348,24 +1359,44 @@ WalSndWriteData(LogicalDecodingContext *ctx, XLogRecPtr lsn, TransactionId xid, * LogicalDecodingContext 'update_progress' callback. * * Write the current position to the lag tracker (see XLogSendPhysical). + * + * In logical replication, if too many changes are processed then try to send a + * keepalive message. It might avoid a timeout in the subscriber. */ static void WalSndUpdateProgress(LogicalDecodingContext *ctx, XLogRecPtr lsn, TransactionId xid) { static TimestampTz sendTime = 0; TimestampTz now = GetCurrentTimestamp(); + bool end_xact = ctx->end_xact; /* * Track lag no more than once per WALSND_LOGICAL_LAG_TRACK_INTERVAL_MS to * avoid flooding the lag tracker when we commit frequently. + * + * We don't have a mechanism to get the ack for any LSN other than end xact + * LSN from the downstream. So, we track lag only for end of transaction + * LSN. */ #define WALSND_LOGICAL_LAG_TRACK_INTERVAL_MS 1000 - if (!TimestampDifferenceExceeds(sendTime, now, + if (end_xact && TimestampDifferenceExceeds(sendTime, now, WALSND_LOGICAL_LAG_TRACK_INTERVAL_MS)) - return; + { + LagTrackerWrite(lsn, now); + sendTime = now; + } - LagTrackerWrite(lsn, now); - sendTime = now; + /* + * Try to send a keepalive if required. We don't need to try sending keep + * alive messages at the transaction end as that will be done at a later + * point in time. This is required only for large transactions where we + * don't send any changes to the downstream and the receiver can timeout + * due to that. + */ + if (!end_xact && + now >= TimestampTzPlusMilliseconds(last_reply_timestamp, + wal_sender_timeout / 2)) + ProcessPendingWrites(); } /* diff --git a/src/include/replication/logical.h b/src/include/replication/logical.h index c2f2475e5d..6455c2d2bf 100644 --- a/src/include/replication/logical.h +++ b/src/include/replication/logical.h @@ -49,6 +49,11 @@ typedef struct LogicalDecodingContext */ bool fast_forward; + /* + * Is the end of transaction LSN? + */ + bool end_xact; + OutputPluginCallbacks callbacks; OutputPluginOptions options; -- 2.23.0.windows.1