Fix snapshot handling in logicalmsg_decode

Whe decoding a transactional logical message, logicalmsg_decode called
SnapBuildGetOrBuildSnapshot. But we may not have a consistent snapshot
yet at that point. We don't actually need the snapshot in this case
(during replay we'll have the snapshot from the transaction), so in
practice this is harmless. But in assert-enabled build this crashes.

Fixed by requesting the snapshot only in non-transactional case, where
we are guaranteed to have SNAPBUILD_CONSISTENT.

Backpatch to 11. The issue exists since 9.6.

Backpatch-through: 11
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/84d60912-6eab-9b84-5de3-41765a5449e8@enterprisedb.com

diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index 54efe2e4b44c33bf0f08a382fb0d9bb2b019c82c..1c0058fc8fd0ea8fdda00c44b6fe5b82f5ff1c5f 100644 (file)
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -628,7 +628,7 @@ DecodeLogicalMsgOp(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
    TransactionId xid = XLogRecGetXid(r);
    uint8       info = XLogRecGetInfo(r) & ~XLR_INFO_MASK;
    RepOriginId origin_id = XLogRecGetOrigin(r);
-   Snapshot    snapshot;
+   Snapshot    snapshot = NULL;
    xl_logical_message *message;
 
    if (info != XLOG_LOGICAL_MESSAGE)
@@ -658,7 +658,17 @@ DecodeLogicalMsgOp(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
              SnapBuildXactNeedsSkip(builder, buf->origptr)))
        return;
 
-   snapshot = SnapBuildGetOrBuildSnapshot(builder, xid);
+   /*
+    * If this is a non-transactional change, get the snapshot we're expected
+    * to use. We only get here when the snapshot is consistent, and the
+    * change is not meant to be skipped.
+    *
+    * For transactional changes we don't need a snapshot, we'll use the
+    * regular snapshot maintained by ReorderBuffer. We just leave it NULL.
+    */
+   if (!message->transactional)
+       snapshot = SnapBuildGetOrBuildSnapshot(builder, xid);
+
    ReorderBufferQueueMessage(ctx->reorder, xid, snapshot, buf->endptr,
                              message->transactional,
                              message->message, /* first part of message is

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 709365fc8c6599529a5a2463cc3e7ce31688c67c..721fa652d25d8df0611dec280e21a68231704fe8 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -821,6 +821,13 @@ ReorderBufferQueueMessage(ReorderBuffer *rb, TransactionId xid,
 
        Assert(xid != InvalidTransactionId);
 
+       /*
+        * We don't expect snapshots for transactional changes - we'll use the
+        * snapshot derived later during apply (unless the change gets
+        * skipped).
+        */
+       Assert(!snapshot);
+
        oldcontext = MemoryContextSwitchTo(rb->context);
 
        change = ReorderBufferGetChange(rb);
@@ -839,6 +846,9 @@ ReorderBufferQueueMessage(ReorderBuffer *rb, TransactionId xid,
        ReorderBufferTXN *txn = NULL;
        volatile Snapshot snapshot_now = snapshot;
 
+       /* Non-transactional changes require a valid snapshot. */
+       Assert(snapshot_now);
+
        if (xid != InvalidTransactionId)
            txn = ReorderBufferTXNByXid(rb, xid, true, NULL, lsn, true);