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