public inbox for [email protected]  
help / color / mirror / Atom feed
From: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Date: Tue, 19 Oct 2021 15:43:28 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <OSBPR01MB48886D77AC8AD25B16C28B5CEDBD9@OSBPR01MB4888.jpnprd01.prod.outlook.com>
References: <CAD21AoD00wV4gt-53ze+ZB8n4bqJrdH8J_UnDHddy8S2A+a25g@mail.gmail.com>
	<[email protected]>
	<OSBPR01MB48886D77AC8AD25B16C28B5CEDBD9@OSBPR01MB4888.jpnprd01.prod.outlook.com>

At Tue, 19 Oct 2021 02:45:24 +0000, "[email protected]" <[email protected]> wrote in 
> On Thursday, October 14, 2021 11:21 AM Kyotaro Horiguchi <[email protected]> wrote:
> > It adds a call to ReorderBufferAssignChild but usually subtransactions are
> > assigned to top level elsewherae.  Addition to that
> > ReorderBufferCommitChild() called just later does the same thing.  We are
> > adding the third call to the same function, which looks a bit odd.
> It can be odd. However, we
> have a check at the top of ReorderBufferAssignChild
> to judge if the sub transaction is already associated or not
> and skip the processings if it is.

My question was why do we need to make the extra call to
ReorerBufferCommitChild when XACT_XINFO_HAS_INVALS in spite of the
existing call to the same fuction that unconditionally made.  It
doesn't cost so much but also it's not free.

> > And I'm not sure it is wise to mark all subtransactions as "catalog changed"
> > always when the top transaction is XACT_XINFO_HAS_INVALS.
> In order to avoid this,
> can't we have a new flag (for example, in reorderbuffer struct) to check
> if we start decoding from RUNNING_XACTS, which is similar to the first patch of [1]
> and use it at DecodeCommit ? This still leads to some extra specific codes added
> to DecodeCommit and this solution becomes a bit similar to other previous patches though.

If it is somehow wrong in any sense that we add subtransactions in
SnapBuildProcessRunningXacts (for example, we should avoid relaxing
the assertion condition.), I think we would go another way.  Otherwise
we don't even need that additional flag.  (But Sawadasan's recent PoC
also needs that relaxation.)

ASAICS, and unless I'm missing something (that odds are rtlatively
high:p), we need the specially added subransactions only for the
transactions that were running at passing the first RUNNING_XACTS,
becuase otherwise (substantial) subtransactions are assigned to
toplevel by the first record of the subtransaction.

Before reaching consistency, DecodeCommit feeds the subtransactions to
ReorderBufferForget individually so the subtransactions are not needed
to be assigned to the top transaction at all. Since the
subtransactions added by the first RUNNING_XACT are processed that
way, we don't need in the first place to call ReorderBufferCommitChild
for such subtransactions.

> [1] - https://www.postgresql.org/message-id/81D0D8B0-E7C4-4999-B616-1E5004DBDCD2%40amazon.com

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center





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: <[email protected]>

* 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