From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "Drouvot, Bertrand" <bdrouvot(at)amazon(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Oh, Mike" <minsoo(at)amazon(dot)com> |
Subject: | Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns |
Date: | 2022-07-28 07:13:40 |
Message-ID: | CAA4eK1LEhoTwkSSmogoJdgg2tgAFxkJni-MyKO8esoTVQomjEA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jul 28, 2022 at 11:56 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Thu, Jul 28, 2022 at 12:21 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Jul 28, 2022 at 7:18 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
>
> While editing back branch patches, I realized that the following
> (parsed->xinfo & XACT_XINFO_HAS_INVALS) and (parsed->nmsgs > 0) are
> equivalent:
>
> + /*
> + * If the COMMIT record has invalidation messages, it could have catalog
> + * changes. It is possible that we didn't mark this transaction as
> + * containing catalog changes when the decoding starts from a commit
> + * record without decoding the transaction's other changes. So, we ensure
> + * to mark such transactions as containing catalog change.
> + *
> + * This must be done before SnapBuildCommitTxn() so that we can include
> + * these transactions in the historic snapshot.
> + */
> + if (parsed->xinfo & XACT_XINFO_HAS_INVALS)
> + SnapBuildXidSetCatalogChanges(ctx->snapshot_builder, xid,
> + parsed->nsubxacts, parsed->subxacts,
> + buf->origptr);
> +
> /*
> * Process invalidation messages, even if we're not interested in the
> * transaction's contents, since the various caches need to always be
> * consistent.
> */
> if (parsed->nmsgs > 0)
> {
> if (!ctx->fast_forward)
> ReorderBufferAddInvalidations(ctx->reorder, xid, buf->origptr,
> parsed->nmsgs, parsed->msgs);
> ReorderBufferXidSetCatalogChanges(ctx->reorder, xid, buf->origptr);
> }
>
> If that's right, I think we can merge these if branches. We can call
> ReorderBufferXidSetCatalogChanges() for top-txn and in
> SnapBuildXidSetCatalogChanges() we mark its subtransactions if top-txn
> is in the list. What do you think?
>
Note that this code doesn't exist in 14 and 15, so we need to create
different patches for those. BTW, how in 13 and lower versions did we
identify and mark subxacts as having catalog changes without our
patch?
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2022-07-28 07:26:19 | Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns |
Previous Message | Alvaro Herrera | 2022-07-28 06:59:20 | Re: LogwrtResult contended spinlock |