[pgpool-hackers: 4223] Re: Dynamic spare process management of Pgpool-II children

[Tatsuo Ishii]

Hi Usama,

Thank you for the update. New patch looks good to me. All regression
tests passed. Please go ahead and commit/push the patch as soon as
possible. Pengbo is waiting for your commit so that she can release
Pgpool-II beta1, which was supposed to be released yesterday.

Subtle point:

- there are some trailing white spaces in the patch.

$ git apply ~/dynamic_spare_process_management_latest.diff 
/home/t-ishii/dynamic_spare_process_management_latest.diff:11: trailing whitespace.
  
/home/t-ishii/dynamic_spare_process_management_latest.diff:37: trailing whitespace.
       
/home/t-ishii/dynamic_spare_process_management_latest.diff:53: trailing whitespace.
       
/home/t-ishii/dynamic_spare_process_management_latest.diff:84: trailing whitespace.
       
warning: 4 lines add whitespace errors.

- In the document patch, 

+         <entry>Static</entry>
+         <entry>
+          All children are pre-forked at startup.
+         </entry>

If "Static" is compatible with the behavior in pre 4.4, then I would
suggest to add wording saying so.

Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp


> Hi,
> 
> Thank you peng for testing and coming up with a minimal test case.
> 
> On Mon, Oct 24, 2022 at 3:38 PM Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
> 
>> Hi Usama, Pengbo,
>>
>> >> after applying the patch, I have run regression test and encountered 7
>> >> timeout.
>> >>
>> >> testing 008.dbredirect...timeout.
>> >> testing 018.detach_primary...timeout.
>> >> testing 033.prefer_lower_standby_delay...timeout.
>> >> testing 034.promote_node...timeout.
>> >> testing 075.detach_primary_left_down_node...timeout.
>> >> testing 076.copy_hang...timeout.
>> >> testing 077.invalid_failover_node...timeout.
>> >
>> > I found that some tests above failed when running pgpool_setup with 3
>> nodes.
>> >
>> > It seems that after running pcp_recovery_node, all child processes exited
>> > with the messages "exited with success and will not be restarted".
>> >
>> > It can be reproduced by running "pgpool_setup -m s -n 3" command.
>> > The log is attached.
>>
>> Peng,
>> Reliably reproduced here. Thank you for the minimal test case. This
>> helped my investigation a lot.
>>
>> Usama,
>> I have looked in to this a little bit and I have doubt in following
>> code fragment (src/main/pgpool_main.c around line 1981)
>>
>>                                 if (pid == process_info[i].pid)
>>                                 {
>>                                         found = true;
>>                                         /* if found, fork a new child */
>>                                         if (!switching && !exiting &&
>> restart_child &&
>>
>> pool_config->process_management = PM_DYNAMIC)
>>
>> Shouldn't this be as following?
>>
>>
>> pool_config->process_management != PM_DYNAMIC)
>>
>> After this modification, all regression tests passed.
>>
> 
> @Tatsuo Ishii <ishii at sraoss.co.jp>  Thanks for pointing this out. It was a
> silly mistake on my part.
> 
> I am attaching the updated patch with the fix that also includes the
> documentation updates for the feature.
> 
> Kindly review
> 
> 
> Thanks
> Best  regards
> Muhammad Usama
> 
> Best reagards,
>> --
>> Tatsuo Ishii
>> SRA OSS LLC
>> English: http://www.sraoss.co.jp/index_en/
>> Japanese:http://www.sraoss.co.jp
>>