RE: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns - Mailing list pgsql-hackers

From shiy.fnst@fujitsu.com
Subject RE: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Date
Msg-id TYAPR01MB63155A51D65ABA4A0F876042FD869@TYAPR01MB6315.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns  (Masahiko Sawada <sawada.mshk@gmail.com>)
Responses Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
List pgsql-hackers
On Tue, Jul 12, 2022 8:49 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> 
> I've attached an updated patch.
> 
> While trying this idea, I noticed there is no API to get the length of
> dlist, as we discussed offlist. Alternative idea was to use List
> (T_XidList) but I'm not sure it's a great idea since deleting an xid
> from the list is O(N), we need to implement list_delete_xid, and we
> need to make sure allocating list node in the reorder buffer context.
> So in the patch, I added a variable, catchange_ntxns, to keep track of
> the length of the dlist. Please review it.
> 

Thanks for your patch. Here are some comments on the master patch.

1.
In catalog_change_snapshot.spec, should we use "RUNNING_XACTS record" instead of
"RUNNING_XACT record" / "XACT_RUNNING record" in the comment?

2.
+         * Since catchange.xip is sorted, we find the lower bound of
+         * xids that sill are interesting.

Typo?
"sill" -> "still"

3.
+     * This array is set once when restoring the snapshot, xids are removed
+     * from the array when decoding xl_running_xacts record, and then eventually
+     * becomes an empty.

+            /* catchange list becomes an empty */
+            pfree(builder->catchange.xip);
+            builder->catchange.xip = NULL;

Should "becomes an empty" be modified to "becomes empty"?

4.
+ * changes that are smaller than ->xmin. Those won't ever get checked via
+ * the ->committed array and ->catchange, respectively. The committed xids will

Should we change 
"the ->committed array and ->catchange"
to
"the ->committed or ->catchange array"
?

Regards,
Shi yu


pgsql-hackers by date:

Previous
From: vignesh C
Date:
Subject: Re: Handle infinite recursion in logical replication setup
Next
From: Amit Kapila
Date:
Subject: Re: Issue with pg_stat_subscription_stats