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

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: amit.kapila16@gmail.com
Cc: sawada.mshk@gmail.com, shiy.fnst@fujitsu.com, bdrouvot@amazon.com, pgsql-hackers@lists.postgresql.org, minsoo@amazon.com
Date: 2022-08-03T01:20:27Z
Lists: pgsql-hackers
At Tue, 2 Aug 2022 13:54:43 +0530, Amit Kapila <amit.kapila16@gmail.com> wrote in 
> On Tue, Aug 2, 2022 at 12:00 PM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> >
> > +       {
> > +               int                     save_errno = errno;
> > +
> > +               CloseTransientFile(fd);
> > +
> > +               if (readBytes < 0)
> > +               {
> > +                       errno = save_errno;
> > +                       ereport(ERROR,
> >
> > Do we need the CloseTransientFile(fd) there?  This call requires errno
> > to be remembered but anyway OpenTransientFile'd files are to be close
> > at transaction end.  Actually CloseTransientFile() is not called
> > before error'ing-out at error in other places.
..
> We just moved it to a separate function as the same code is being
> duplicated to multiple places.

There are code paths that doesn't CloseTransientFile() explicitly,
too.  If there were no need of save_errno there, that'd be fine.  But
otherwise I guess we prefer to let the orphan fds closed by ERROR and
I don't think we need to preserve the less-preferred code pattern (if
we actually prefer not to have the explicit call).

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



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.