Horton

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>

View thread View on postgr.es
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: sawada.mshk@gmail.com
Cc: amit.kapila16@gmail.com, osumi.takamichi@fujitsu.com, bdrouvot@amazon.com, pgsql-hackers@lists.postgresql.org, minsoo@amazon.com
Date: 2022-07-19T07:57:14Z
Lists: pgsql-hackers 
At Tue, 19 Jul 2022 16:02:26 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in 
> On Tue, Jul 19, 2022 at 1:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > Good work. I wonder without comments this may create a problem in the
> > future. OTOH, I don't see adding a check "catchange.xcnt > 0" before
> > freeing the memory any less robust. Also, for consistency, we can use
> > a similar check based on xcnt in the SnapBuildRestore to free the
> > memory in the below code:
> > + /* set catalog modifying transactions */
> > + if (builder->catchange.xip)
> > + pfree(builder->catchange.xip);
> 
> I would hesitate to add comments about preventing the particular
> optimization. I think we do null-pointer-check-then-pfree many place.
> It seems to me that checking the array length before memcpy is more
> natural than checking both the array length and the array existence
> before pfree.

Anyway according to commit message of 46ab07ffda, POSIX forbits
memcpy(NULL, NULL, 0). It seems to me that it is the cause of the
false (or over) optimization.  So if we add some comment, it would be
for memcpy, not pfree..

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.