RE: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Yu Shi (Fujitsu) <shiy.fnst@fujitsu.com>
From: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>, 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>
Date: 2022-07-26T09:24:58Z
Lists: 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
Commits
-
Fix the incorrect assertion introduced in commit 7f13ac8123.
- d2169c998553 16.0 landed
-
Fix catalog lookup with the wrong snapshot during logical decoding.
- 7f13ac812313 16.0 landed
- 272248a0c1b1 15.0 landed
- 68dcce247f1a 14.6 landed
- 547b963683e3 13.9 landed
- 794460783125 12.13 landed
- e721123b7a0c 11.18 landed
- bf0718c137c4 10.23 landed
-
Clean up assorted failures under clang's -fsanitize=undefined checks.
- 46ab07ffda9d 15.0 cited