Re: Initial COPY of Logical Replication is too slow - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Initial COPY of Logical Replication is too slow
Date
Msg-id CAA4eK1LYF4h+ehT9GvCV62U+Cr_uAKYRgy+3TcUXS7-DLxkq=A@mail.gmail.com
Whole thread
In response to Re: Initial COPY of Logical Replication is too slow  (Masahiko Sawada <sawada.mshk@gmail.com>)
List pgsql-hackers
On Thu, Apr 2, 2026 at 3:53 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Tue, Mar 31, 2026 at 12:40 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Tue, Mar 31, 2026 at 5:07 AM Zhijie Hou (Fujitsu)
> > <houzj.fnst@fujitsu.com> wrote:
> > >
> > > On Tuesday, March 31, 2026 5:36 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > On Wed, Mar 25, 2026 at 2:19 PM Peter Smith <smithpb2250@gmail.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.



pgsql-hackers by date:

Previous
From: Antonin Houska
Date:
Subject: Re: table AM option passing
Next
From: Peter Smith
Date:
Subject: Re: Initial COPY of Logical Replication is too slow