Re: Parallel Apply

From: shveta malik <shveta(dot)malik(at)gmail(dot)com>
To: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Andrei Lepikhov <lepihov(at)gmail(dot)com>, wenhui qiu <qiuwenhuifx(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com>
Subject: Re: Parallel Apply
Date: 2026-04-17 07:27:37
Message-ID: CAJpy0uD-rq2UHQhCHaoY7jx4sAJA0CyzAboQKDSEwfuo2CzZpQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Apr 16, 2026 at 8:35 PM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Tuesday, April 14, 2026 9:00 PM Kuroda, Hayato/黒田 隼人 <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> >
> > Other comments were addressed accordingly, please see attached patch set.
>

Thanks for the patches. I have started reviewing it today, A few
comment son 001:

1)
+ elog(DEBUG1, "parallel apply worker worker init relmap for %s",
+ rel->relname);

worker mentioned twice

2)
Calling handle_dependency_on_change() from
handle_streamed_transaction() is misleading, since the former is
intended for non-streaming transactions, while the latter handles
streaming ones.

I am not able to think of a better name for
handle_streamed_transaction() that would make calling the dependency
function from within it feel natural. So the only option I see is to
move handle_dependency_on_change() out. I think should be okay for
this function to be called from multiple places. In fact, this would
likely improve clarity for someone reading the
apply_handle_insert/update/delete code independently.

3)
Since caller of apply_handle_internal_message(), which is
apply_spooled_messages() is called from both leader and pa-worker;
apply_handle_internal_message() may benefit from below sanity check to
ensure only pa-workers intercept PARALLEL_APPLY_INTERNAL_MESSAGE:

Assert(am_parallel_apply_worker())

4)
The name pa_wait_for_depended_transaction() suggests that it is
pa-specific worker. We can retain the name as is, but can we add a
comment atop this funciton saying used by both parallel and leader
worker?

5)
I started reading 002's commit message, I think it is not that clear.
I was trying to find if we have actual value for remote-xid which is
key to hash tbale. But I think it is hash-table for only xid as key
for faster access may be? If so, can we please improve commit messagee
little bit?

thanks
Shveta

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2026-04-17 07:40:40 Re: Fix stats reporting delays in logical parallel apply worker
Previous Message Amit Kapila 2026-04-17 07:19:36 Re: Fix stats reporting delays in logical parallel apply worker