MIME-Version: 1.0
References: <20211008.165055.1621145185927268721.horikyota.ntt@gmail.com>
 <CAD21AoD00wV4gt-53ze+ZB8n4bqJrdH8J_UnDHddy8S2A+a25g@mail.gmail.com>
 <CAA4eK1L_e4r4=10TTzoOiWGQ6hH4vxWAwD=0aofz04czCTV0kQ@mail.gmail.com>
 <20220523.133323.444599955292141474.horikyota.ntt@gmail.com>
 <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>
In-Reply-To: 
 <CAD21AoC4+GeT_HRnCFuy0L37FHUSXOPEpXYp0wbQnFxCMiuD=Q@mail.gmail.com>
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 12 Jul 2022 10:28:17 +0900
Message-ID: 
 <CAD21AoA7WM=ptACtu0cg5XdQ86c5RJDg9efsdPRCxQnUqZ+omg@mail.gmail.com>
Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode
 "base/13237/442428" to relation OID" with catalog modifying txns
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "Drouvot,
 Bertrand" <bdrouvot@amazon.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, "Oh,
 Mike" <minsoo@amazon.com>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAD21AoA7WM%3DptACtu0cg5XdQ86c5RJDg9efsdPRCxQnUqZ%2Bomg%40mail.gmail.com>
Precedence: bulk

On Tue, Jul 12, 2022 at 9:48 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, Jul 8, 2022 at 8:20 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Fri, Jul 8, 2022 at 5:59 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Fri, Jul 8, 2022 at 12:46 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > On Fri, Jul 8, 2022 at 3:27 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > >
> > > >
> > > > > 1.
> > > > > In ReorderBufferGetCatalogChangesXacts(), isn't it better to use the
> > > > > list length of 'catchange_txns' to allocate xids array? If we can do
> > > > > so, then we will save the need to repalloc as well.
> > > >
> > > > Since ReorderBufferGetcatalogChangesXacts() collects all ongoing
> > > > catalog modifying transactions, the length of the array could be
> > > > bigger than the one taken last time. We can start with the previous
> > > > length but I think we cannot remove the need for repalloc.
> > > >
> > >
> > > It is using the list "catchange_txns" to form xid array which
> > > shouldn't change for the duration of
> > > ReorderBufferGetCatalogChangesXacts(). Then the caller frees the xid
> > > array after its use. Next time in
> > > ReorderBufferGetCatalogChangesXacts(), the fresh allocation for xid
> > > array happens, so not sure why repalloc would be required?
> >
> > Oops, I mistook catchange_txns for catchange->xcnt. You're right.
> > Starting with the length of catchange_txns should be sufficient.
> >
>
> I've attached an updated patch.
>
> While trying this idea, I noticed there is no API to get the length of
> dlist, as we discussed offlist. Alternative idea was to use List
> (T_XidList) but I'm not sure it's a great idea since deleting an xid
> from the list is O(N), we need to implement list_delete_xid, and we
> need to make sure allocating list node in the reorder buffer context.
> So in the patch, I added a variable, catchange_ntxns, to keep track of
> the length of the dlist. Please review it.
>

I'm doing benchmark tests and will share the results.

Regards,

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