Horton

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>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Osumi, Takamichi/大墨 昂道 <osumi.takamichi@fujitsu.com>, "Drouvot, Bertrand" <bdrouvot@amazon.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, "Oh, Mike" <minsoo@amazon.com>
Date: 2022-07-19T12:42:22Z
Lists: pgsql-hackers 
On Tue, Jul 19, 2022 at 2:01 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Tue, Jul 19, 2022 at 4:35 PM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
>
> >
> >
> > +       Assert((xcnt > 0) && (xcnt == rb->catchange_ntxns));
> >
> > (xcnt > 0) is obvious here (otherwise means dlist_foreach is broken..).
> > (xcnt == rb->catchange_ntxns) is not what should be checked here. The
> > assert just requires that catchange_txns and catchange_ntxns are
> > consistent so it should be checked just after dlist_empty.. I think.
> >
>
> If we want to check if catchange_txns and catchange_ntxns are
> consistent, should we check (xcnt == rb->catchange_ntxns) as well, no?
> This function requires the caller to use rb->catchange_ntxns as the
> length of the returned array. I think this assertion ensures that the
> actual length of the array is consistent with the length we
> pre-calculated.
>

Right, so, I think it is better to keep this assertion but remove
(xcnt > 0) part as pointed out by Horiguchi-San.

-- 
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.