v20240111-0006-add-comment-about-SNAPBUILD_FULL.patch

text/x-patch

Filename: v20240111-0006-add-comment-about-SNAPBUILD_FULL.patch
Type: text/x-patch
Part: 5
Message: Re: logical decoding and replication of sequences, take 2

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v20240111-0006
Subject: add comment about SNAPBUILD_FULL
File+
src/backend/replication/logical/decode.c 16 1
From 2d96af7b89008d906ffc2a76ca906e0f7eac0a68 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.vondra@postgresql.org>
Date: Thu, 11 Jan 2024 14:35:29 +0100
Subject: [PATCH v20240111 6/7] add comment about SNAPBUILD_FULL

---
 src/backend/replication/logical/decode.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index 3462c545bb7..95f224df4a7 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -1432,12 +1432,27 @@ seq_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 	 * change as transactional and queue it. Otherwise it needs to be treated
 	 * as non-transactional, in which case we just send it to the plugin right
 	 * away.
+	 *
+	 * XXX This may incorrectly identify some changes as non-transactional,
+	 * if the relfilenode was created before the snapshot switched to FULL
+	 * (so that smgr_decode skipped it, and it's not in the hash table).
+	 * But we still handle this correctly, because the snapshot can't be
+	 * in CONSISTENT state yet, so the following check will skip the change
+	 * too, and we won't try to apply it (which would fail due to unknown
+	 * relfilenode).
 	 */
 	transactional = ReorderBufferSequenceIsTransactional(ctx->reorder,
 														 target_locator,
 														 NULL);
 
-	/* Skip the change if already processed (per the snapshot). */
+	/*
+	 * Skip the change if already processed (per the snapshot).
+	 *
+	 * XXX It's important to skip all non-transactional changes with snapshot
+	 * that is not consistent yet. The sequence change may be incorrectly
+	 * identified as non-transactional (see above), and we must not attempt
+	 * to process it.
+	 */
 	if (transactional &&
 		!SnapBuildProcessChange(builder, xid, buf->origptr))
 		return;
-- 
2.43.0