On Thu, Apr 16, 2026 at 8:35 PM Zhijie Hou (Fujitsu) <[email protected]> wrote: > > On Tuesday, April 14, 2026 9:00 PM Kuroda, Hayato/黒田 隼人 > <[email protected]> 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
