public inbox for [email protected]  
help / color / mirror / Atom feed
From: Masahiko Sawada <[email protected]>
To: Amit Kapila <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: Drouvot, Bertrand <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Oh, Mike <[email protected]>
Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Date: Tue, 19 Jul 2022 10:03:45 +0900
Message-ID: <CAD21AoDfZCmjUS74t+_7j2g+MshYcp7026mufR3SRKW6XAgAHA@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1K07S5tiHq7P+T51PDhHxaYiyfWZW4OP52gtTxBGV2EMA@mail.gmail.com>
References: <[email protected]>
	<CAD21AoD00wV4gt-53ze+ZB8n4bqJrdH8J_UnDHddy8S2A+a25g@mail.gmail.com>
	<CAA4eK1L_e4r4=10TTzoOiWGQ6hH4vxWAwD=0aofz04czCTV0kQ@mail.gmail.com>
	<[email protected]>
	<CAA4eK1JoKV2qmp916gFk=9SX=Qo21+sN4n-yjbB2b0Q1xxOKJw@mail.gmail.com>
	<CAD21AoAn9T6mS8sMF5=tFV66Wru92s933ihRVunLPEU_+0=Xiw@mail.gmail.com>
	<CAA4eK1Lgfj2ww4V_zMxTmduB0BQpEsS-j7RT8BV-cy7Qyp1E3A@mail.gmail.com>
	<CAD21AoDNJgYkrO1PQ=k6G9MfvRjhawMN-aZVT2spMr_ymmOhCQ@mail.gmail.com>
	<CAD21AoC4x3uOw5rUcSYZkWob5s5ottGt_RPLxCEpHimFRDjrEg@mail.gmail.com>
	<CAA4eK1L_Br0wNHwY1PrnusX1H2bvWR+iUnNC=1anKqhPBtnoMg@mail.gmail.com>
	<CAA4eK1KMsU5PFHOvTD=3jHQ6aPa8N39eGwAVMVA6S0sXw1kMdw@mail.gmail.com>
	<CAD21AoD8v=pE+3AMezPrWJo=hpijNmop-XRbZ=3YG5zT5pBMnQ@mail.gmail.com>
	<CAA4eK1KuSxRhwUoJWVW--EseKcob94-hrP+JMCGvGTR+Fnx2aA@mail.gmail.com>
	<CAD21AoBHo3WQ+vaeiNzXuNw__08veBT9qYgCUD3jNLWG=4GzFw@mail.gmail.com>
	<CAA4eK1KMegGo+QwMxJcHMbYN-3QOLH6hD2thWXA2_M3MLH01Ug@mail.gmail.com>
	<CAD21AoCF=8SKOJYWF12fPEwXc5GLU2SvhfZ3QoR93Qqn+6oeSg@mail.gmail.com>
	<CAA4eK1LuFtCifCBY=uB_Orzjx-tc0O8DZqfFE_JuHptngeCcwA@mail.gmail.com>
	<CAD21AoCBPL10=X83g6J2CXQRE11kXgiyzfVY2m=hS1O7YRqiWQ@mail.gmail.com>
	<CAA4eK1JyJL7dACPQ51hQmSz5KrxghmABr_J9AZbSGzSLW3Ghig@mail.gmail.com>
	<CAD21AoCAPkPxsqbe6ZTqOe0bcVrEWHmA3ObYQtyu0Bv8hC=z_g@mail.gmail.com>
	<CAD21AoC4+GeT_HRnCFuy0L37FHUSXOPEpXYp0wbQnFxCMiuD=Q@mail.gmail.com>
	<CAD21AoA7WM=ptACtu0cg5XdQ86c5RJDg9efsdPRCxQnUqZ+omg@mail.gmail.com>
	<CAD21AoDFC5btYbcGOLMWimotg+N45pXJqyEOhbfsOHdz8XUKpg@mail.gmail.com>
	<CAA4eK1KZHnHhZdL1-Fa-6+sw4JHEPjLjZ=+wTwRHnkooXoxp2A@mail.gmail.com>
	<CAD21AoDJiAiyp3AHbaPJ1w=twWs2sThUiNDxvpJ+k98voV2PSg@mail.gmail.com>
	<CAA4eK1KxnYuhU5AyusDAP1SKtVmwAS1wYJ+VNL155x=swbx+gA@mail.gmail.com>
	<CAD21AoAyNPrOFg+QGh+=4205TU0=yrE+QyMgzStkH85uBZXptQ@mail.gmail.com>
	<TYCPR01MB8373EFE369CD515805C16292ED8B9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CAD21AoCdiqOcVmZgoh3nRxhPNqueUGCbPRkzcmjSgD6GUU2=gA@mail.gmail.com>
	<CAA4eK1K07S5tiHq7P+T51PDhHxaYiyfWZW4OP52gtTxBGV2EMA@mail.gmail.com>

On Mon, Jul 18, 2022 at 1:12 PM Amit Kapila <[email protected]> wrote:
>
> On Fri, Jul 15, 2022 at 8:09 PM Masahiko Sawada <[email protected]> wrote:
> >
> > This patch should have the fix for the issue that Shi yu reported. Shi
> > yu, could you please test it again with this patch?
> >
>
> Can you explain the cause of the failure and your fix for the same?

@@ -1694,6 +1788,8 @@ out:
    /* be tidy */
    if (ondisk)
        pfree(ondisk);
+   if (catchange_xip)
+       pfree(catchange_xip);

Regarding the above code in the previous version patch, looking at the
generated assembler code shared by Shi yu offlist, I realized that the
“if (catchange_xip)” is removed (folded) by gcc optimization. This is
because we dereference catchange_xip before null-pointer check as
follow:

+   /* copy catalog modifying xacts */
+   sz = sizeof(TransactionId) * catchange_xcnt;
+   memcpy(ondisk_c, catchange_xip, sz);
+   COMP_CRC32C(ondisk->checksum, ondisk_c, sz);
+   ondisk_c += sz;

Since sz is 0 in this case, memcpy doesn’t do anything actually.

By checking the assembler code, I’ve confirmed that gcc does the
optimization for these code and setting
-fno-delete-null-pointer-checks flag prevents the if statement from
being folded. Also, I’ve confirmed that adding the check if
"catchange.xcnt > 0” before the null-pointer check also can prevent
that. Adding a check  if "catchange.xcnt > 0” looks more robust. I’ve
added a similar check for builder->committed.xcnt as well for
consistency. builder->committed.xip could have no transactions.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/





view thread (136+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
  In-Reply-To: <CAD21AoDfZCmjUS74t+_7j2g+MshYcp7026mufR3SRKW6XAgAHA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox