On Mon, Aug 22, 2022 at 10:49 PM kuroda.hay...@fujitsu.com <kuroda.hay...@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.
