pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: ocean_li_996 <ocean_li_996@163.com>
To: cca5507 <cca5507@qq.com>
Cc: Haiyang Li <mohen.lhy@alibaba-inc.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Xuneng Zhou <xunengzhou@gmail.com>
Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #19109: catalog lookup with the wrong snapshot duringlogical decoding causes coredump
Date: Fri, 21 Nov 2025 02:16:17 +0800 (CST)
Message-ID: <74c88ce1.a69e.19aa27b3fbe.Coremail.ocean_li_996@163.com> (raw)
In-Reply-To: <tencent_273266783D07B7765EA1E2851CB5F2294108@qq.com>
References: <19109-4a9ce36715d69632@postgresql.org>
	<CABPTF7W883TcGrY71vVDTdEJgxJ993_Fe+fHTT+1Ttk6oDH3YQ@mail.gmail.com>
	<13b5ed51-0a02-4b3a-b8b3-b33505f1fe6c.mohen.lhy@alibaba-inc.com>
	<aRKbgWylZllp4vYF@paquier.xyz>
	<3bb4539e-c342-4e15-94f1-803203f1072e.mohen.lhy@alibaba-inc.com>
	<2b9e5ac8.136f.19a8f7297ee.Coremail.ocean_li_996@163.com>
	<tencent_273266783D07B7765EA1E2851CB5F2294108@qq.com>

Hi ChangAo,


At 2025-11-19 11:23:19, "cca5507" <cca5507@qq.com> wrote:
> I reported this bug before, and there is a commitfest entry for it:
>
> https://commitfest.postgresql.org/patch/5029/
>
> The attached patches are the latest patches taken from the commitfest.


Thanks for your report and patches. 


> This bug seem to affect a lot of PG major versions.


Absolutely. I can reproduced it from 11 to 19. Then, I researched the source code. IMUC, this bug exists since
logical replication intrduced(v9.4). It seems that snapshot builder does nothing except update builder->xmin in
SNAPBUILD_BUILDING_SNAPSHOT state. But, tracking xid which >= builder->next_pahse_at is reasonable
for me. BTW,  this bug can result in unpredictable behavior, ranging from various error reports to core dumps. 
It deserves some extra attention, I think. 


It’s somewhat unfortunate that the issue remains unresolved so far. Hopefully we can address it in the
upcoming discussion or commitfest.


> They seem to need a rebase, but I'd like to see your thoughts on these patches first.


Some comments:
1 ====
@@ -1301,6 +1338,7 @@ DecodeTXNNeedSkip(LogicalDecodingContext *ctx, XLogRecordBuffer *buf,
Oid txn_dbid, RepOriginId origin_id)
{
if (SnapBuildXactNeedsSkip(ctx->snapshot_builder, buf->origptr) ||
+ SnapBuildCurrentState(ctx->snapshot_builder) < SNAPBUILD_CONSISTENT ||
(txn_dbid != InvalidOid && txn_dbid != ctx->slot->data.database) ||
FilterByOrigin(ctx, origin_id))
return true;
It seems that this change will cause unexpected bahavior.  For expample, it will make the xact commited in SNAPBUILD_FULL_SNAPSHOT
to be skipped in DecodeCommit(). Do you mean "SnapBuildCurrentState(ctx->snapshot_builder) < SNAPBUILD_FULL_SNAPSHOT"?


2 ===
@@ -282,9 +287,11 @@ xact_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
{
TransactionId xid;
xl_xact_invals *invals;
+ bool has_snapshot;
...

+ else if (!ctx->fast_forward && has_snapshot)
ReorderBufferImmediateInvalidation(ctx->reorder,
invals->nmsgs,
invals->msgs);
May be the following change is more clear for me:
if (TransactionIdIsValid(xid) &&
SnapBuildCurrentState(builder) >= SNAPBUILD_FULL_SNAPSHOT)
{
ReorderBufferXidSetCatalogChanges(ctx->reorder, xid,
buf->origptr);
break;
}


3===
@@ -416,7 +423,22 @@ heap2_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
*/
if (SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT ||
ctx->fast_forward)
+ {
+ /*
+ * Note that during or after BUILDING_SNAPSHOT, we need handle the xlog
+ * that might mark a transaction as catalog modifying because the snapshot
+ * only tracks catalog modifying transactions. The transaction before
+ * BUILDING_SNAPSHOT will not be tracked anyway(see SnapBuildCommitTxn()
+ * for details), so just return.
+ */
+ if (SnapBuildCurrentState(builder) >= SNAPBUILD_BUILDING_SNAPSHOT)
+ {
+ /* Currently only XLOG_HEAP2_NEW_CID means a catalog modifying */
+ if (info == XLOG_HEAP2_NEW_CID && TransactionIdIsValid(xid))
+ ReorderBufferXidSetCatalogChanges(ctx->reorder, xid, buf->origptr);
+ }
return;
+ }
It should be removed from v18 and master because commit 243e9b4 intrduced.


4===
IMUC, v4-0001 patch is to only mark transactions with catalog changes during the buildsnapshot phase, 
without performing any other actions.  I’m wondering if we should make the behavior of snapshot building
the same in both SNAPBUILD_BUILDING_SNAPSHOT and SNAPBUILD_FULL_SNAPSHOT states,
except in cases where a base snapshot is needed. If so, the attached patch is for the master version.


Regards,
Haiyang Li

Attachments:

  [application/octet-stream] v5-0001-master-Track-transactions-committed-in-BUILDING_SNAPSHOT.patch (1.2K, ../74c88ce1.a69e.19aa27b3fbe.Coremail.ocean_li_996@163.com/3-v5-0001-master-Track-transactions-committed-in-BUILDING_SNAPSHOT.patch)
  download | inline diff:
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index 78f9a0a11c4..fc4986d3358 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -206,10 +206,11 @@ xact_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 	uint8		info = XLogRecGetInfo(r) & XLOG_XACT_OPMASK;
 
 	/*
-	 * If the snapshot isn't yet fully built, we cannot decode anything, so
-	 * bail out.
+	 * Before BUILDING_SNAPSHOT, we cannot decode anything because we don't
+	 * have snapshot and the transaction will not be tracked by snapshot
+	 * anyway(see SnapBuildCommitTxn() for details), so bail out.
 	 */
-	if (SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT)
+	if (SnapBuildCurrentState(builder) < SNAPBUILD_BUILDING_SNAPSHOT)
 		return;
 
 	switch (info)
@@ -414,7 +415,7 @@ heap2_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 	 * If we don't have snapshot or we are just fast-forwarding, there is no
 	 * point in decoding changes.
 	 */
-	if (SnapBuildCurrentState(builder) < SNAPBUILD_FULL_SNAPSHOT ||
+	if (SnapBuildCurrentState(builder) < SNAPBUILD_BUILDING_SNAPSHOT ||
 		ctx->fast_forward)
 		return;
 


view thread (11+ messages)  latest in thread

Message-ID: <74c88ce1.a69e.19aa27b3fbe.Coremail.ocean_li_996@163.com>
Permalink:  ../74c88ce1.a69e.19aa27b3fbe.Coremail.ocean_li_996@163.com/
Also on:    postgresql.org/message-id/74c88ce1.a69e.19aa27b3fbe.Coremail.ocean_li_996@163.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: ocean_li_996@163.com, cca5507@qq.com, mohen.lhy@alibaba-inc.com, michael@paquier.xyz, xunengzhou@gmail.com, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #19109: catalog lookup with the wrong snapshot duringlogical decoding causes coredump
  In-Reply-To: <74c88ce1.a69e.19aa27b3fbe.Coremail.ocean_li_996@163.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox