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>
From: Masahiko Sawada <sawada.mshk@gmail.com>
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>
Date: 2022-07-12T00:48:52Z
Lists: pgsql-hackers
Attachments
- master-v3-0001-Add-catalog-modifying-transactions-to-logical-dec.patch (application/octet-stream) patch v3-0001
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. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
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