RE: Perform streaming logical transactions by background workers and parallel apply

On Tuesday, January 17, 2023 2:46 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> 
> On Tue, Jan 17, 2023 at 12:37 PM houzj.fnst@fujitsu.com
> <houzj.fnst@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.

Best regards,
Hou zj