public inbox for [email protected]  
help / color / mirror / Atom feed
From: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Date: Fri, 08 Oct 2021 16:50:55 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAD21AoAn-k6OpZ6HSAH_G91tpTXR6KYvkf663kg6EqW-f6sz1w@mail.gmail.com>
References: <CAD21AoCxVNYK0R=SSPchcx1wm6mx=qJ5W8qDVakFCOAy1UxqAQ@mail.gmail.com>
	<[email protected]>
	<CAD21AoAn-k6OpZ6HSAH_G91tpTXR6KYvkf663kg6EqW-f6sz1w@mail.gmail.com>

At Thu, 7 Oct 2021 13:20:14 +0900, Masahiko Sawada <[email protected]> wrote in 
> Another idea to fix this problem would be that before calling
> SnapBuildCommitTxn() we create transaction entries in ReorderBuffer
> for (sub)transactions whose COMMIT record has XACT_XINFO_HAS_INVALS,
> and then mark all of them as catalog-changed by calling
> ReorderBufferXidSetCatalogChanges(). I've attached a PoC patch for
> this idea. What the patch does is essentially the same as what the
> proposed patch does. But the patch doesn't modify the
> SnapBuildCommitTxn(). And we remember the list of last running
> transactions in reorder buffer and the list is periodically purged
> during decoding RUNNING_XACTS records, eventually making it empty.

I came up with the third way.  SnapBuildCommitTxn already properly
handles the case where a ReorderBufferTXN with
RBTXN_HAS_CATALOG_CHANGES.  So this issue can be resolved by create
such ReorderBufferTXNs in SnapBuildProcessRunningXacts.

One problem with this is that change creates the case where multiple
ReorderBufferTXNs share the same first_lsn.  I haven't come up with a
clean idea to avoid relaxing the restriction of AssertTXNLsnOrder..

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 46e66608cf..503116764f 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -887,9 +887,14 @@ AssertTXNLsnOrder(ReorderBuffer *rb)
 		if (cur_txn->end_lsn != InvalidXLogRecPtr)
 			Assert(cur_txn->first_lsn <= cur_txn->end_lsn);
 
-		/* Current initial LSN must be strictly higher than previous */
+		/*
+		 * Current initial LSN must be strictly higher than previous. except
+		 * this transaction is created by XLOG_RUNNING_XACTS.  If one
+		 * XLOG_RUNNING_XACTS creates multiple transactions, they share the
+		 * same LSN. See SnapBuildProcessRunningXacts.
+		 */
 		if (prev_first_lsn != InvalidXLogRecPtr)
-			Assert(prev_first_lsn < cur_txn->first_lsn);
+			Assert(prev_first_lsn <= cur_txn->first_lsn);
 
 		/* known-as-subtxn txns must not be listed */
 		Assert(!rbtxn_is_known_subxact(cur_txn));
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index a5333349a8..58859112dc 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1097,6 +1097,20 @@ SnapBuildProcessRunningXacts(SnapBuild *builder, XLogRecPtr lsn, xl_running_xact
 	 */
 	if (builder->state < SNAPBUILD_CONSISTENT)
 	{
+		/*
+		 * At the time we passed the first XLOG_RUNNING_XACTS record, the
+		 * transactions notified by the record may have updated
+		 * catalogs. Register the transactions with marking them as having
+		 * caused catalog changes.  The worst misbehavior here is some spurious
+		 * invalidation at decoding start.
+		 */
+		if (builder->state == SNAPBUILD_START)
+		{
+			for (int i = 0 ; i < running->xcnt + running->subxcnt ; i++)
+				ReorderBufferXidSetCatalogChanges(builder->reorder,
+												  running->xids[i], lsn);
+		}
+
 		/* returns false if there's no point in performing cleanup just yet */
 		if (!SnapBuildFindSnapshot(builder, lsn, running))
 			return;


Attachments:

  [text/plain] register_running_xacts_at_logical_decoding_start_PoC.txt (2.0K, ../[email protected]/2-register_running_xacts_at_logical_decoding_start_PoC.txt)
  download | inline diff:
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 46e66608cf..503116764f 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -887,9 +887,14 @@ AssertTXNLsnOrder(ReorderBuffer *rb)
 		if (cur_txn->end_lsn != InvalidXLogRecPtr)
 			Assert(cur_txn->first_lsn <= cur_txn->end_lsn);
 
-		/* Current initial LSN must be strictly higher than previous */
+		/*
+		 * Current initial LSN must be strictly higher than previous. except
+		 * this transaction is created by XLOG_RUNNING_XACTS.  If one
+		 * XLOG_RUNNING_XACTS creates multiple transactions, they share the
+		 * same LSN. See SnapBuildProcessRunningXacts.
+		 */
 		if (prev_first_lsn != InvalidXLogRecPtr)
-			Assert(prev_first_lsn < cur_txn->first_lsn);
+			Assert(prev_first_lsn <= cur_txn->first_lsn);
 
 		/* known-as-subtxn txns must not be listed */
 		Assert(!rbtxn_is_known_subxact(cur_txn));
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index a5333349a8..58859112dc 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1097,6 +1097,20 @@ SnapBuildProcessRunningXacts(SnapBuild *builder, XLogRecPtr lsn, xl_running_xact
 	 */
 	if (builder->state < SNAPBUILD_CONSISTENT)
 	{
+		/*
+		 * At the time we passed the first XLOG_RUNNING_XACTS record, the
+		 * transactions notified by the record may have updated
+		 * catalogs. Register the transactions with marking them as having
+		 * caused catalog changes.  The worst misbehavior here is some spurious
+		 * invalidation at decoding start.
+		 */
+		if (builder->state == SNAPBUILD_START)
+		{
+			for (int i = 0 ; i < running->xcnt + running->subxcnt ; i++)
+				ReorderBufferXidSetCatalogChanges(builder->reorder,
+												  running->xids[i], lsn);
+		}
+
 		/* returns false if there's no point in performing cleanup just yet */
 		if (!SnapBuildFindSnapshot(builder, lsn, running))
 			return;


view thread (136+ messages)  latest in thread

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
  In-Reply-To: <[email protected]>

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

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