On Mon, Aug 22, 2022 at 10:49 PM kuroda.hayato@fujitsu.com
<kuroda.hayato@fujitsu.com> wrote:
>
> 04. launcher.c
>
> 04.a
>
> + worker->main_worker_pid = is_subworker ? MyProcPid : 0;
>
> You can use InvalidPid instead of 0.
> （I thought pid should be represented by the datatype pid_t, but in some codes it is defined as int...)
>
> 04.b
>
> + worker->main_worker_pid = 0;
>
> You can use InvalidPid instead of 0, same as above.
>
> 05. origin.c
>
> void
> -replorigin_session_setup(RepOriginId node)
> +replorigin_session_setup(RepOriginId node, int acquired_by)
>
> IIUC the same slot can be used only when the apply main worker has already acquired the slot
> and the subworker for the same subscription tries to acquire, but it cannot understand from comments.
> How about adding comments, or an assertion that acquired_by is same as session_replication_state->acquired_by ?
> Moreover acquired_by should be compared with InvalidPid, based on above comments.
>
In general I agree, and I also suggested to use pid_t and InvalidPid
(at least for all the new code)
In practice, please be aware that InvalidPid is -1 (not 0), so
replacing any existing code (e.g. in replorigin_session_setup) that
was already checking for 0 has to be done with lots of care.
------
Kind Regards,
Peter Smith.
Fujitsu Australia.