public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: Masahiko Sawada <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: Drouvot, Bertrand <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Oh, Mike <[email protected]>
Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Date: Thu, 7 Jul 2022 12:10:06 +0530
Message-ID: <CAA4eK1KMegGo+QwMxJcHMbYN-3QOLH6hD2thWXA2_M3MLH01Ug@mail.gmail.com> (raw)
In-Reply-To: <CAD21AoBHo3WQ+vaeiNzXuNw__08veBT9qYgCUD3jNLWG=4GzFw@mail.gmail.com>
References: <[email protected]>
	<CAD21AoD00wV4gt-53ze+ZB8n4bqJrdH8J_UnDHddy8S2A+a25g@mail.gmail.com>
	<CAA4eK1L_e4r4=10TTzoOiWGQ6hH4vxWAwD=0aofz04czCTV0kQ@mail.gmail.com>
	<[email protected]>
	<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>

On Thu, Jul 7, 2022 at 8:21 AM Masahiko Sawada <[email protected]> wrote:
>
> On Wed, Jul 6, 2022 at 5:55 PM Amit Kapila <[email protected]> wrote:
> >
> > On Wed, Jul 6, 2022 at 12:19 PM Masahiko Sawada <[email protected]> wrote:
> > >
> > > On Tue, Jul 5, 2022 at 8:00 PM Amit Kapila <[email protected]> wrote:
> > > >
> > > > 2. Are we anytime removing transaction ids from catchanges->xip array?
> > >
> > > No.
> > >
> > > > If not, is there a reason for the same? I think we can remove it
> > > > either at commit/abort or even immediately after adding the xid/subxid
> > > > to committed->xip array.
> > >
> > > It might be a good idea but I'm concerned that removing XID from the
> > > array at every commit/abort or after adding it to committed->xip array
> > > might be costly as it requires adjustment of the array to keep its
> > > order. Removing XIDs from the array would make bsearch faster but the
> > > array is updated reasonably often (every 15 sec).
> > >
> >
> > Fair point. However, I am slightly worried that we are unnecessarily
> > searching in this new array even when ReorderBufferTxn has the
> > required information. To avoid that, in function
> > SnapBuildXidHasCatalogChange(), we can first check
> > ReorderBufferXidHasCatalogChanges() and then check the array if the
> > first check doesn't return true. Also, by the way, do we need to
> > always keep builder->catchanges.xip updated via SnapBuildRestore()?
> > Isn't it sufficient that we just read and throw away contents from a
> > snapshot if builder->catchanges.xip is non-NULL?
>
> IIUC catchanges.xip is restored only once when restoring a consistent
> snapshot via SnapBuildRestore(). I think it's necessary to set
> catchanges.xip for later use in SnapBuildXidHasCatalogChange(). Or did
> you mean via SnapBuildSerialize()?∫
>

Sorry, I got confused about the way restore is used. You are right, it
will be done once. My main worry is that we shouldn't look at
builder->catchanges.xip array on an ongoing basis which I think can be
dealt with by one of the ideas you mentioned below. But, I think we
can still follow the other suggestion related to moving
ReorderBufferXidHasCatalogChanges() check prior to checking array.

> >
> > I had additionally thought if can further optimize this solution to
> > just store this additional information when we need to serialize for
> > checkpoint record but I think that won't work because walsender can
> > restart even without resatart of server in which case the same problem
> > can occur.
>
> Yes, probably we need to write catalog modifying transactions for
> every serialized snapshot.
>
> > I am not if sure there is a way to further optimize this
> > solution, let me know if you have any ideas?
>
> I suppose that writing additional information to serialized snapshots
> would not be a noticeable overhead since we need 4 bytes per
> transaction and we would not expect there is a huge number of
> concurrent catalog modifying transactions. But both collecting catalog
> modifying transactions (especially when there are many ongoing
> transactions) and bsearch'ing on the XID list every time decoding the
> COMMIT record could bring overhead.
>
> A solution for the first point would be to keep track of catalog
> modifying transactions by using a linked list so that we can avoid
> checking all ongoing transactions.
>

This sounds reasonable to me.

> Regarding the second point, on reflection, I think we need to look up
> the XID list until all XID in the list is committed/aborted. We can
> remove XIDs from the list after adding it to committed.xip as you
> suggested. Or when decoding a RUNNING_XACTS record, we can remove XIDs
> older than builder->xmin from the list like we do for committed.xip in
> SnapBuildPurgeCommittedTxn().
>

I think doing along with RUNNING_XACTS should be fine. At each
commit/abort, the cost could be high because we need to maintain the
sort order. In general, I feel any one of these should be okay because
once the array becomes empty, it won't be used again and there won't
be any operation related to it during ongoing replication.

-- 
With Regards,
Amit Kapila.





view thread (136+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
  In-Reply-To: <CAA4eK1KMegGo+QwMxJcHMbYN-3QOLH6hD2thWXA2_M3MLH01Ug@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox