Horton

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

Yu Shi (Fujitsu) <shiy.fnst@fujitsu.com>

View thread View on postgr.es
From: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "Drouvot, Bertrand" <bdrouvot@amazon.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, "Oh, Mike" <minsoo@amazon.com>
Date: 2022-07-15T06:32:10Z
Lists: pgsql-hackers 
On Mon, Jul 11, 2022 9:54 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> 
> I've attached an updated patch, please review it.
> 

Thanks for your patch. Here are some comments for the REL14-v1 patch.

1.
+		Size		sz = sizeof(TransactionId) * nxacts;;

There is a redundant semicolon at the end.

2.
+	workspace = MemoryContextAlloc(rb->context, rb->n_initial_running_xacts);

Should it be:
+	workspace = MemoryContextAlloc(rb->context, sizeof(TransactionId) * rb->n_initial_running_xacts);

3.
+	/* bound check if there is at least one transaction to be removed */
+	if (NormalTransactionIdPrecedes(rb->initial_running_xacts[0],
+									running->oldestRunningXid))
+		return;
+

Here, I think it should return if rb->initial_running_xacts[0] is older than
oldestRunningXid, right? Should it be changed to:

+	if (!NormalTransactionIdPrecedes(rb->initial_running_xacts[0],
+									running->oldestRunningXid))
+		return;

4.
+	if ((parsed->xinfo & XACT_XINFO_HAS_INVALS) != 0)

Maybe we can change it like the following, to be consistent with other places in
this file. It's also fine if you don't change it.

+	if (parsed->xinfo & XACT_XINFO_HAS_INVALS)


Regards,
Shi yu

Commits

  1. Fix the incorrect assertion introduced in commit 7f13ac8123.

  2. Fix catalog lookup with the wrong snapshot during logical decoding.

  3. Clean up assorted failures under clang's -fsanitize=undefined checks.