On Tue, Jan 17, 2023 at 6:14 PM houzj.f...@fujitsu.com <houzj.f...@fujitsu.com> wrote: > > On Tuesday, January 17, 2023 2:46 PM Masahiko Sawada <sawada.m...@gmail.com> > wrote: > > > > On Tue, Jan 17, 2023 at 12:37 PM houzj.f...@fujitsu.com > > <houzj.f...@fujitsu.com> wrote: > > > Attach the new version 0001 patch which addressed all other comments. > > > > > > > Thank you for updating the patch. Here is one comment: > > > > @@ -426,14 +427,24 @@ pg_stat_get_activity(PG_FUNCTION_ARGS) > > > > /* > > * Show the leader only for active parallel > > workers. This > > - * leaves the field as NULL for the > > leader of a parallel > > - * group. > > + * leaves the field as NULL for the > > leader of a parallel group > > + * or the leader of parallel apply workers. > > */ > > if (leader && leader->pid != > > beentry->st_procpid) > > { > > values[28] = > > Int32GetDatum(leader->pid); > > nulls[28] = false; > > } > > + else > > + { > > + int > > leader_pid = GetLeaderApplyWorkerPid(beentry->st_procpid); > > + > > + if (leader_pid != InvalidPid) > > + { > > + values[28] = > > Int32GetDatum(leader_pid); > > + nulls[28] = false; > > + } > > + } > > } > > > > I'm slightly concerned that there could be overhead of executing > > GetLeaderApplyWorkerPid () for every backend process except for parallel > > query workers. The number of such backends could be large and > > GetLeaderApplyWorkerPid() acquires the lwlock. For example, does it make > > sense to check (st_backendType == B_BG_WORKER) before calling > > GetLeaderApplyWorkerPid()? Or it might not be a problem since it's > > LogicalRepWorkerLock which is not likely to be contended. > > Thanks for the comment and I think your suggestion makes sense. > I have added the check before getting the leader pid. Here is the new version > patch.
Thank you for updating the patch. Looks good to me. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com