On Thu, Apr 16, 2026 at 8:35 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Tuesday, April 14, 2026 9:00 PM Kuroda, Hayato/黒田 隼人 <kuroda.hayato@fujitsu.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
