| 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.
| 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 |