Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns

On Tue, Jul 26, 2022 at 2:18 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Jul 26, 2022 at 7:00 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Mon, Jul 25, 2022 at 7:57 PM shiy(dot)fnst(at)fujitsu(dot)com
> > <shiy(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > Hi,
> > >
> > > I did some performance test for the master branch patch (based on v6 patch) to
> > > see if the bsearch() added by this patch will cause any overhead.
> >
> > Thank you for doing performance tests!
> >
> > >
> > > I tested them three times and took the average.
> > >
> > > The results are as follows, and attach the bar chart.
> > >
> > > case 1
> > > ---------
> > > No catalog modifying transaction.
> > > Decode 800k pgbench transactions. (8 clients, 100k transactions per client)
> > >
> > > master 7.5417
> > > patched 7.4107
> > >
> > > case 2
> > > ---------
> > > There's one catalog modifying transaction.
> > > Decode 100k/500k/1M transactions.
> > >
> > > 100k 500k 1M
> > > master 0.0576 0.1491 0.4346
> > > patched 0.0586 0.1500 0.4344
> > >
> > > 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
> > >
> > > (Because the result of case 3 shows that there is a overhead of about 3% in the
> > > case decoding 100k transactions with 64 catalog modifying transactions, I
> > > tested the next run of 100k xacts with or without catalog modifying
> > > transactions, to see if it affects subsequent decoding.)
> > >
> > > case 4.1
> > > ---------
> > > After the test steps in case 3 (64 catalog modifying transactions, decode 100k
> > > transactions), run 100k xacts and then decode.
> > >
> > > master 0.3699
> > > patched 0.3701
> > >
> > > case 4.2
> > > ---------
> > > After the test steps in case 3 (64 catalog modifying transactions, decode 100k
> > > transactions), run 64 DDLs(without checkpoint) and 100k xacts, then decode.
> > >
> > > master 0.3687
> > > patched 0.3696
> > >
> > > 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?

Regards,

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