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

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, 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-20T05:18:55Z
Lists: pgsql-hackers
On Wed, Jul 20, 2022 at 9:01 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Wed, Jul 20, 2022 at 12:11 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Tue, Jul 19, 2022 at 7:28 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > Why do you think we can't remove
> > ReorderBufferInitialXactsSetCatalogChanges() from the back branch
> > patch? I think we don't need to change the existing function
> > ReorderBufferXidHasCatalogChanges() but instead can have a wrapper
> > like SnapBuildXidHasCatalogChanges() similar to master branch patch.
>
> IIUC we need to change SnapBuildCommitTxn() but it's exposed.
>
> Currently, we call like DecodeCommit() -> SnapBuildCommitTxn() ->
> ReorderBufferXidHasCatalogChanges(). If we have a wrapper function, we
> call like DecodeCommit() -> SnapBuildCommitTxn() ->
> SnapBuildXidHasCatalogChanges() ->
> ReorderBufferXidHasCatalogChanges(). In
> SnapBuildXidHasCatalogChanges(), we need to check if the transaction
> has XACT_XINFO_HAS_INVALS, which means DecodeCommit() needs to pass
> either parsed->xinfo or (parsed->xinfo & XACT_XINFO_HAS_INVALS != 0)
> down to SnapBuildXidHasCatalogChanges(). However, since
> SnapBuildCommitTxn(), between DecodeCommit() and
> SnapBuildXidHasCatalogChanges(), is exposed we cannot change it.
>

Agreed.

> Another idea would be to have functions, say
> SnapBuildCommitTxnWithXInfo() and SnapBuildCommitTxn_ext(). The latter
> does actual work of handling transaction commits and both
> SnapBuildCommitTxn() and SnapBuildCommit() call
> SnapBuildCommitTxnWithXInfo() with different arguments.
>

Do you want to say DecodeCommit() instead of SnapBuildCommit() in
above para? Yet another idea could be to have another flag
RBTXN_HAS_INVALS which will be set by DecodeCommit for top-level TXN.
Then, we can retrieve it even for each of the subtxn's if and when
required.

-- 
With Regards,
Amit Kapila.



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.