public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: Masahiko Sawada <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: Drouvot, Bertrand <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Oh, Mike <[email protected]>
Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Date: Thu, 28 Jul 2022 15:23:54 +0530
Message-ID: <CAA4eK1JxVro2YvP2ocw5Th0den3uZipLHkM+8pOCsPkYmWa3cw@mail.gmail.com> (raw)
In-Reply-To: <CAD21AoCN9Z1D43R5uT84w0HEPBP-gH5Gy5nVcn5Nf3fja47_tQ@mail.gmail.com>
References: <[email protected]>
	<CAD21AoD00wV4gt-53ze+ZB8n4bqJrdH8J_UnDHddy8S2A+a25g@mail.gmail.com>
	<CAA4eK1L_e4r4=10TTzoOiWGQ6hH4vxWAwD=0aofz04czCTV0kQ@mail.gmail.com>
	<[email protected]>
	<CAA4eK1JoKV2qmp916gFk=9SX=Qo21+sN4n-yjbB2b0Q1xxOKJw@mail.gmail.com>
	<CAD21AoAn9T6mS8sMF5=tFV66Wru92s933ihRVunLPEU_+0=Xiw@mail.gmail.com>
	<CAA4eK1Lgfj2ww4V_zMxTmduB0BQpEsS-j7RT8BV-cy7Qyp1E3A@mail.gmail.com>
	<CAD21AoDNJgYkrO1PQ=k6G9MfvRjhawMN-aZVT2spMr_ymmOhCQ@mail.gmail.com>
	<CAD21AoC4x3uOw5rUcSYZkWob5s5ottGt_RPLxCEpHimFRDjrEg@mail.gmail.com>
	<CAA4eK1L_Br0wNHwY1PrnusX1H2bvWR+iUnNC=1anKqhPBtnoMg@mail.gmail.com>
	<CAD21AoBeMVfY6dvg5J8XmSHGbc3+yTA8k_GVRzwZR24zc4TfSw@mail.gmail.com>
	<CAA4eK1Jg+u=ouXbC5EVLVnQ4sUhrR1P+p0LQSjPFmMP4jNYmuw@mail.gmail.com>
	<CAD21AoCEF6FAvRRP6LKzq2-2oVvpdQbmZDwzRAdseA9AA2mE-Q@mail.gmail.com>
	<OSZPR01MB63106E9CE179D80F46CB3154FD8B9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
	<CAD21AoAUx28e-1YQDA6Uhda+=jkidAy+HY8Zv5SYjtXpRcBiVg@mail.gmail.com>
	<CAA4eK1LmyPryHUP3kaWRBbz88aCs5J_8-vL2wqwi5wnSwa24tw@mail.gmail.com>
	<CAD21AoC1H8tDo=h-xsm5Und-c4FQbNCAOuRoBaMhsxNsJrM8vA@mail.gmail.com>
	<CAA4eK1JfcR96cxqb+NYmDueirpvLytfvP-UZPz-E6-U6Lv-+tw@mail.gmail.com>
	<CAD21AoCjfmuCRUpsn+nuqRuWW3gQCBdTKuYGYvYiwLVtpTQYSg@mail.gmail.com>
	<CAA4eK1LXpPnLEdTttCmMbw3bSnrn=k_58ZUDvx3zbAHLjgod2A@mail.gmail.com>
	<CAD21AoBKb_XMdOO0vCAHKieWYqHmJnEN0bGmV5j7PvOGOEkmrQ@mail.gmail.com>
	<CAA4eK1LpxNwGKntRJSND8H-DmN8XuZYoaooQmaX4+g-ePqg2=Q@mail.gmail.com>
	<CAD21AoCzuV5yhOGw8=nVkymkE0KaHkxMMy5R-HsfBQw9=h0asA@mail.gmail.com>
	<CAA4eK1LH54gTKnxhBJRy9GHjoPxp7ggA+B9Cy7VLtXToCVukhw@mail.gmail.com>
	<CAD21AoC8NLGJ-Q+u60cGPSnxXqexEFPLo-5-offsgCE_ikVxRw@mail.gmail.com>
	<CAA4eK1+U1b7P2mMTZHGE0GaT4vYhGsShQx_6070a6xgwj9JQAQ@mail.gmail.com>
	<CAD21AoC_oEy3EvgbJ=asz20Z3ROWtRM01f-F8+F_5VjhCvJX6g@mail.gmail.com>
	<CAD21AoAxuMzBAkOYD8ggxwNbobRx=sDPmy_JnYMkKP2sesu6+w@mail.gmail.com>
	<OSZPR01MB631075F474BAFBDBD6F2DB6EFD959@OSZPR01MB6310.jpnprd01.prod.outlook.com>
	<CAD21AoDGTaT4rdJHKc2hypYeDjdUdmHccWbbtwqYCN5hwkVixA@mail.gmail.com>
	<CAA4eK1LefopSMb6XQ7b6hw+GmZw-wDAOWhq+YXM9iC4Y6Mdtjw@mail.gmail.com>
	<CAD21AoCN9Z1D43R5uT84w0HEPBP-gH5Gy5nVcn5Nf3fja47_tQ@mail.gmail.com>

On Tue, Jul 26, 2022 at 1:22 PM Masahiko Sawada <[email protected]> wrote:
>
> Okay, I've attached an updated patch that does the above idea. Could
> you please do the performance tests again to see if the idea can help
> reduce the overhead, Shi yu?
>

While reviewing the patch for HEAD, I have changed a few comments. See
attached, if you agree with these changes then include them in the
next version.

-- 
With Regards,
Amit Kapila.


Attachments:

  [application/octet-stream] master_v9_amit.diff.patch (2.7K, ../CAA4eK1JxVro2YvP2ocw5Th0den3uZipLHkM+8pOCsPkYmWa3cw@mail.gmail.com/2-master_v9_amit.diff.patch)
  download | inline diff:
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index ab6bae3b6e..ca56f7bfe3 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -247,15 +247,18 @@ struct SnapBuild
 	 * and were running when the snapshot was serialized.
 	 *
 	 * We normally rely on HEAP2_NEW_CID and XLOG_XACT_INVALIDATIONS records to
-	 * know if the transaction has changed the catalog. But it could happen that
-	 * the logical decoding decodes only the commit record of the transaction.
-	 * This array stores the transactions that have modified catalogs and were
-	 * running when serializing a snapshot, and this array is used to add such
-	 * transactions to the snapshot.
+	 * know if the transaction has changed the catalog. But it could happen
+	 * that the logical decoding decodes only the commit record of the
+	 * transaction after restoring the previously serialized snapshot in which
+	 * case we will miss adding the xid to the snapshot and end up looking at
+	 * the catalogs with the wrong snapshot.
 	 *
-	 * This array is set once when restoring the snapshot, we remove xids from
-	 * this array when they become old enough to matter, and then it eventually
-	 * becomes empty.
+	 * Now to avoid the above problem, we serialize the transactions that had
+	 * modified the catalogs and are still running at the time of snapshot
+	 * serialization. We fill this array while restoring the snapshot and then
+	 * refer it while decoding commit to ensure if the xact has modified the
+	 * catalog. We remove xids from this array when they become old enough to
+	 * matter, and then it eventually becomes empty.
 	 */
 	struct
 	{
@@ -924,9 +927,9 @@ SnapBuildAddCommittedTxn(SnapBuild *builder, TransactionId xid)
  * the ->committed or ->catchange array, respectively. The committed xids will
  * get checked via the clog machinery.
  *
- * We can ideally remove the transaction
- * from catchange array once it is finished (committed/aborted) but that could
- * be costly as we need to maintain the xids order in the array.
+ * We can ideally remove the transaction from catchange array once it is
+ * finished (committed/aborted) but that could be costly as we need to maintain
+ * the xids order in the array.
  */
 static void
 SnapBuildPurgeOlderTxn(SnapBuild *builder)
@@ -1171,8 +1174,8 @@ SnapBuildXidHasCatalogChanges(SnapBuild *builder, TransactionId xid,
 		return true;
 
 	/*
-	 * If the commit record of the transaction does not have invalidation
-	 * messages, it did not change catalogs for sure.
+	 * The transactions that have changed catalogs must have invalidation
+	 * info.
 	 */
 	if (!(xinfo & XACT_XINFO_HAS_INVALS))
 		return false;


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], [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: <CAA4eK1JxVro2YvP2ocw5Th0den3uZipLHkM+8pOCsPkYmWa3cw@mail.gmail.com>

* 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