Re: Initial COPY of Logical Replication is too slow

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Jan Wieck <jan(at)wi3ck(dot)info>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Initial COPY of Logical Replication is too slow
Date: 2026-04-02 05:45:21
Message-ID: CAA4eK1LYF4h+ehT9GvCV62U+Cr_uAKYRgy+3TcUXS7-DLxkq=A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Apr 2, 2026 at 3:53 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Tue, Mar 31, 2026 at 12:40 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Tue, Mar 31, 2026 at 5:07 AM Zhijie Hou (Fujitsu)
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > On Tuesday, March 31, 2026 5:36 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > >
> > > > On Wed, Mar 25, 2026 at 2:19 PM Peter Smith <smithpb2250(at)gmail(dot)com>
> > > > wrote:
> > > > >
> > > > > There are many return points, and most of those "if" blocks cannot
> > > > > fall through (they return).
> > > > >
> > > > > I found it slightly difficult to read the code because I kept having
> > > > > to think, "OK, if we reached here, it means pubviaroot must be false,"
> > > > > or "OK, if we reached this far, then puballtables must be false, and
> > > > > pubviaroot must be false," etc.
> > > > >
> > > >
> > > > I can't say exactly why, but I find it difficult to read this function. So, I share
> > > > your concerns about the code of this function.
> > > > Because of its complexity it is difficult to ascertain that the functionality is
> > > > correct or we missed something. Also, considering it is correct today, in its
> > > > current form, it may become difficult to enhance it in future.
> > > >
> > >
> > > I attempted to refactor the code a bit based on my preferred style, as shown in
> > > the attachment. While the number of return points couldn't be reduced, I tried
> > > to eliminate if-else branches where possible. Sharing this top-up patch as a
> > > reference for an alternative style that reduces code size.
> > >
> >
> > Thanks. It looks like a good refactoring! I'd prefer to free the
> > ancestors list to avoid memory leak.
> >
> > I've attached the patch that incorporated all comments I got so far.
> > Feedback is very welcome.
> >
>
> I decided to simplify the code flow in the
> is_table_publishable_in_publication() by taking more proposed changes
> from Hou-san while accepting some memory leak. This function is
> limited to be used only in per-call-memory context so we don't need to
> worry about the actual memory leak. While we would need to change this
> function in the futuer when we want to use it other places too, I
> think it would be better to keep the function simple until then. I
> hope the added new comment also help understand the code flow of this
> function.
>

Yes, the function looks better and helps to understand the flow.
Thanks for improving it.

--
With Regards,
Amit Kapila.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2026-04-02 06:12:03 Re: Initial COPY of Logical Replication is too slow
Previous Message Antonin Houska 2026-04-02 05:39:43 Re: table AM option passing