public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [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, 02 Aug 2022 15:30:12 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA4eK1KiiXHtj641_G2v7s+S33CVPG9k3OBMVHb3hn+f9YBC7A@mail.gmail.com>
References: <CAA4eK1KCxW8bA+LrXZYsww42KoYo4nQ_Kycz1cw7TR0UXDXfMA@mail.gmail.com>
<CAD21AoBRtPLEYcTi5YOzRbh06xXqM1aZmi9Dkczfbd5=6hKTrg@mail.gmail.com>
<CAA4eK1KiiXHtj641_G2v7s+S33CVPG9k3OBMVHb3hn+f9YBC7A@mail.gmail.com>
At Mon, 1 Aug 2022 20:01:00 +0530, Amit Kapila <[email protected]> wrote in
> On Mon, Aug 1, 2022 at 7:46 AM Masahiko Sawada <[email protected]> wrote:
> >
> > On Fri, Jul 29, 2022 at 3:45 PM Amit Kapila <[email protected]> wrote:
> > >
> >
> > I've attached updated patches for all branches. Please review them.
> >
>
> Thanks, the patches look mostly good to me. I have made minor edits by
> removing 'likely' from a few places as those don't seem to be adding
> much value, changed comments at a few places, and was getting
> compilation in error in v11/10 (snapbuild.c:2111:3: error: ‘for’ loop
> initial declarations are only allowed in C99 mode) which I have fixed.
> See attached, unless there are major comments/suggestions, I am
> planning to push this day after tomorrow (by Wednesday) after another
> pass.
master:
+ * Read the contents of the serialized snapshot to the dest.
Do we need the "the" before the "dest"?
+ {
+ int save_errno = errno;
+
+ CloseTransientFile(fd);
+
+ if (readBytes < 0)
+ {
+ errno = save_errno;
+ ereport(ERROR,
Do we need the CloseTransientFile(fd) there? This call requires errno
to be remembered but anyway OpenTransientFile'd files are to be close
at transaction end. Actually CloseTransientFile() is not called
before error'ing-out at error in other places.
+ * from the LSN-ordered list of toplevel TXNs. We remove TXN from the list
We remove "the" TXN"?
+ if (dlist_is_empty(&rb->catchange_txns))
+ {
+ Assert(rb->catchange_ntxns == 0);
+ return NULL;
+ }
It seems that the assert is far simpler than dlist_is_empty(). Why
don't we swap the conditions for if() and Assert() in the above?
+ * the oldest running transaction窶冱 restart_decoding_lsn is.
The line contains a broken characters.
+ * Either all the xacts got purged or none. It is only possible to
+ * partially remove the xids from this array if one or more of the xids
+ * are still running but not all. That can happen if we start decoding
Assuming this, the commment below seems getting stale.
+ * catalog. We remove xids from this array when they become old enough to
+ * matter, and then it eventually becomes empty.
"We discard this array when the all containing xids are gone. See
SnapBuildPurgeOlderTxn for details." or something like?
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], [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