RE: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns - Mailing list pgsql-hackers

From shiy.fnst@fujitsu.com
Subject RE: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Date
Msg-id TYAPR01MB631503A1D3F38BB938A8EF11FD949@TYAPR01MB6315.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns  (Masahiko Sawada <sawada.mshk@gmail.com>)
List pgsql-hackers
On Tue, Jul 26, 2022 3:52 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> 
> On Tue, Jul 26, 2022 at 2:18 PM Amit Kapila <amit.kapila16@gmail.com>
> wrote:
> >
> > On Tue, Jul 26, 2022 at 7:00 AM Masahiko Sawada
> <sawada.mshk@gmail.com> wrote:
> > >
> > > On Mon, Jul 25, 2022 at 7:57 PM shiy.fnst@fujitsu.com
> > > <shiy.fnst@fujitsu.com> wrote:
> > > >
> > > >
> > > > case 3
> > > > ---------
> > > > There are 64 catalog modifying transactions.
> > > > Decode 100k/500k/1M transactions.
> > > >
> > > >             100k        500k        1M
> > > > master      0.0600      0.1666      0.4876
> > > > patched     0.0620      0.1653      0.4795
> > > >
> > > >
> > > > Summary of the tests:
> > > > After applying this patch, there is a overhead of about 3% in the case
> decoding
> > > > 100k transactions with 64 catalog modifying transactions. This is an
> extreme
> > > > case, so maybe it's okay.
> > >
> > > Yes. If we're worried about the overhead and doing bsearch() is the
> > > cause, probably we can try simplehash instead of the array.
> > >
> >
> > I am not sure if we need to go that far for this extremely corner
> > case. Let's first try your below idea.
> >
> > > An improvement idea is that we pass the parsed->xinfo down to
> > > SnapBuildXidHasCatalogChanges(), and then return from that function
> > > before doing bearch() if the parsed->xinfo doesn't have
> > > XACT_XINFO_HAS_INVALS. That would save calling bsearch() for
> > > non-catalog-modifying transactions. Is it worth trying?
> > >
> >
> > I think this is worth trying and this might reduce some of the
> > overhead as well in the case presented by Shi-San.
> 
> Okay, I've attached an updated patch that does the above idea. Could
> you please do the performance tests again to see if the idea can help
> reduce the overhead, Shi yu?
> 

Thanks for your improvement. I have tested the case which shows overhead before
(decoding 100k transactions with 64 catalog modifying transactions) for the v9
patch, the result is as follows.

master      0.0607
patched     0.0613

There's almost no difference compared with master (less than 1%), which looks
good to me.

Regards,
Shi yu

pgsql-hackers by date:

Previous
From: Dilip Kumar
Date:
Subject: Re: Perform streaming logical transactions by background workers and parallel apply
Next
From: Fujii Masao
Date:
Subject: Re: Refactoring postgres_fdw/connection.c