Date: Wed, 03 Aug 2022 10:20:27 +0900 (JST)
Message-Id: <20220803.102027.1786766955594381576.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
Subject: Re: [BUG] Logical replication failure "ERROR: could not map
 filenode "base/13237/442428" to relation OID" with catalog modifying txns
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: 
 <CAA4eK1LUis6LrKDi=VAWhP-0OniC4ThbXueAhdaQHpdCg4MvRg@mail.gmail.com>
References: 
 <CAA4eK1KiiXHtj641_G2v7s+S33CVPG9k3OBMVHb3hn+f9YBC7A@mail.gmail.com>
	<20220802.153012.363912034520325077.horikyota.ntt@gmail.com>
	<CAA4eK1LUis6LrKDi=VAWhP-0OniC4ThbXueAhdaQHpdCg4MvRg@mail.gmail.com>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20220803.102027.1786766955594381576.horikyota.ntt%40gmail.com>
Precedence: bulk

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