<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Nov 9, 2022 at 5:50 AM Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp">ishii@sraoss.co.jp</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Usama,<br>
<br>
Thank you for the update. New patch looks good to me. All regression<br>
tests passed. Please go ahead and commit/push the patch as soon as<br>
possible. Pengbo is waiting for your commit so that she can release<br>
Pgpool-II beta1, which was supposed to be released yesterday.<br></blockquote><div><br></div><div>Thanks, I have pushed the commit to the master branch. We can proceed with beta1.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Subtle point:<br>
<br>
- there are some trailing white spaces in the patch.<br>
<br>
$ git apply ~/dynamic_spare_process_management_latest.diff <br>
/home/t-ishii/dynamic_spare_process_management_latest.diff:11: trailing whitespace.<br>
<br>
/home/t-ishii/dynamic_spare_process_management_latest.diff:37: trailing whitespace.<br>
<br>
/home/t-ishii/dynamic_spare_process_management_latest.diff:53: trailing whitespace.<br>
<br>
/home/t-ishii/dynamic_spare_process_management_latest.diff:84: trailing whitespace.<br>
<br>
warning: 4 lines add whitespace errors.<br></blockquote><div>Fixed </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
- In the document patch, <br>
<br>
+         <entry>Static</entry><br>
+         <entry><br>
+          All children are pre-forked at startup.<br>
+         </entry><br>
<br>
If "Static" is compatible with the behavior in pre 4.4, then I would<br>
suggest to add wording saying so.<br></blockquote><div><br></div><div>Done.</div><div> </div><div><br></div><div>Best regards</div><div>Muhammad Usama</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Best reagards,<br>
--<br>
Tatsuo Ishii<br>
SRA OSS LLC<br>
English: <a href="http://www.sraoss.co.jp/index_en/" rel="noreferrer" target="_blank">http://www.sraoss.co.jp/index_en/</a><br>
Japanese:<a href="http://www.sraoss.co.jp" rel="noreferrer" target="_blank">http://www.sraoss.co.jp</a><br>
<br>
<br>
> Hi,<br>
> <br>
> Thank you peng for testing and coming up with a minimal test case.<br>
> <br>
> On Mon, Oct 24, 2022 at 3:38 PM Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>> wrote:<br>
> <br>
>> Hi Usama, Pengbo,<br>
>><br>
>> >> after applying the patch, I have run regression test and encountered 7<br>
>> >> timeout.<br>
>> >><br>
>> >> testing 008.dbredirect...timeout.<br>
>> >> testing 018.detach_primary...timeout.<br>
>> >> testing 033.prefer_lower_standby_delay...timeout.<br>
>> >> testing 034.promote_node...timeout.<br>
>> >> testing 075.detach_primary_left_down_node...timeout.<br>
>> >> testing 076.copy_hang...timeout.<br>
>> >> testing 077.invalid_failover_node...timeout.<br>
>> ><br>
>> > I found that some tests above failed when running pgpool_setup with 3<br>
>> nodes.<br>
>> ><br>
>> > It seems that after running pcp_recovery_node, all child processes exited<br>
>> > with the messages "exited with success and will not be restarted".<br>
>> ><br>
>> > It can be reproduced by running "pgpool_setup -m s -n 3" command.<br>
>> > The log is attached.<br>
>><br>
>> Peng,<br>
>> Reliably reproduced here. Thank you for the minimal test case. This<br>
>> helped my investigation a lot.<br>
>><br>
>> Usama,<br>
>> I have looked in to this a little bit and I have doubt in following<br>
>> code fragment (src/main/pgpool_main.c around line 1981)<br>
>><br>
>>                                 if (pid == process_info[i].pid)<br>
>>                                 {<br>
>>                                         found = true;<br>
>>                                         /* if found, fork a new child */<br>
>>                                         if (!switching && !exiting &&<br>
>> restart_child &&<br>
>><br>
>> pool_config->process_management = PM_DYNAMIC)<br>
>><br>
>> Shouldn't this be as following?<br>
>><br>
>><br>
>> pool_config->process_management != PM_DYNAMIC)<br>
>><br>
>> After this modification, all regression tests passed.<br>
>><br>
> <br>
> @Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>>  Thanks for pointing this out. It was a<br>
> silly mistake on my part.<br>
> <br>
> I am attaching the updated patch with the fix that also includes the<br>
> documentation updates for the feature.<br>
> <br>
> Kindly review<br>
> <br>
> <br>
> Thanks<br>
> Best  regards<br>
> Muhammad Usama<br>
> <br>
> Best reagards,<br>
>> --<br>
>> Tatsuo Ishii<br>
>> SRA OSS LLC<br>
>> English: <a href="http://www.sraoss.co.jp/index_en/" rel="noreferrer" target="_blank">http://www.sraoss.co.jp/index_en/</a><br>
>> Japanese:<a href="http://www.sraoss.co.jp" rel="noreferrer" target="_blank">http://www.sraoss.co.jp</a><br>
>><br>
</blockquote></div></div>