<div dir="ltr"><div dir="ltr"><br></div><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 22, 2020 at 7:32 AM Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">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>
Why the test is under src/test/dml-adaptive-test/simple-query-test/test.sh?<br>
It would be good if it's located under the standard regression test directory.<br></blockquote><div><br></div><div>Thanks for pointing it out, I somehow overlooked it.</div><div>I will take care of it and move the simple query test case</div><div>to the standard regression directory.</div><div><br></div><div>Thanks</div><div>Best Regards</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 regards,<br>
--<br>
Tatsuo Ishii<br>
SRA OSS, Inc. Japan<br>
English: <a href="http://www.sraoss.co.jp/index_en.php" rel="noreferrer" target="_blank">http://www.sraoss.co.jp/index_en.php</a><br>
Japanese:<a href="http://www.sraoss.co.jp" rel="noreferrer" target="_blank">http://www.sraoss.co.jp</a><br>
<br>
> Hi Sunbiao,<br>
> <br>
> <br>
> I have committed your patch after a few minor modifications and adjusting<br>
> the documentation.<br>
> <a href="https://git.postgresql.org/gitweb?p=pgpool2.git;a=commitdiff;h=761d30a17d04350fb3a67bce43440731c8ba3c90" rel="noreferrer" target="_blank">https://git.postgresql.org/gitweb?p=pgpool2.git;a=commitdiff;h=761d30a17d04350fb3a67bce43440731c8ba3c90</a><br>
> <br>
> <br>
> Best Regards<br>
> <br>
> On Tue, Jul 7, 2020 at 7:08 AM <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a> <<a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a>><br>
> wrote:<br>
> <br>
>> Hi, Usama<br>
>><br>
>> I have seen patch-v7 and tested it.<br>
>> I think removing the parentheses () is a good way.<br>
>><br>
>> Thanks<br>
>> Best Regards<br>
>> ------------------------------<br>
>> <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>><br>
>><br>
>> *From:* Muhammad Usama <<a href="mailto:m.usama@gmail.com" target="_blank">m.usama@gmail.com</a>><br>
>> *Date:* 2020-07-07 01:19<br>
>> *To:* <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>> *CC:* Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>>; pgpool-hackers<br>
>> <<a href="mailto:pgpool-hackers@pgpool.net" target="_blank">pgpool-hackers@pgpool.net</a>><br>
>> *Subject:* Re: Re: [pgpool-hackers: 3592] add a feature: dml object level<br>
>> load balance<br>
>> Hi Sunbiao,<br>
>><br>
>> Thanks for pointing the mistake.<br>
>> So I have yet again changed the patch a little bit.<br>
>> Instead of doing strncasecmp in<br>
>> get_associated_object_from_dml_adaptive_relations function,<br>
>> I have removed the parentheses () from the original token at the time of<br>
>> parsing.<br>
>> What do you think? see the new attached patch.<br>
>><br>
>> Thanks<br>
>> Best Regards<br>
>> Muhammad Usama<br>
>><br>
>> Thanks for the updated patch.<br>
>> On Mon, Jul 6, 2020 at 10:37 AM <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a> <<a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a>><br>
>> wrote:<br>
>><br>
>>> Hi, Usama<br>
>>><br>
>>> I think your idea is better then using json.<br>
>>> I tested patch-v5. There was a bug in the function<br>
>>> "get_associated_object_from_dml_adaptive_relations" , when sql calls a<br>
>>> function.<br>
>>><br>
>><br>
>> Can you please<br>
>><br>
>><br>
>>> I fixed it in the new patch.<br>
>>><br>
>>> Thanks<br>
>>> Best regards.<br>
>>> ------------------------------<br>
>>> <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>>><br>
>>><br>
>>> *From:* Muhammad Usama <<a href="mailto:m.usama@gmail.com" target="_blank">m.usama@gmail.com</a>><br>
>>> *Date:* 2020-07-02 05:15<br>
>>> *To:* <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>>> *CC:* Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>>; pgpool-hackers<br>
>>> <<a href="mailto:pgpool-hackers@pgpool.net" target="_blank">pgpool-hackers@pgpool.net</a>><br>
>>> *Subject:* Re: [pgpool-hackers: 3592] add a feature: dml object level<br>
>>> load balance<br>
>>> Hi Sunbiao,<br>
>>><br>
>>> Thanks for the patch, It is working as expected. Though while reviewing it<br>
>>> I realized we are doing the dml_adaptive_relationship_list parsing at the<br>
>>> start of every<br>
>>> session, That is better than the previous approach but still has room for<br>
>>> improvement.<br>
>>><br>
>>> My idea is to do the parsing at the very beginning while loading the<br>
>>> configuration parameter<br>
>>> and then use that.<br>
>>><br>
>>> Secondly, I do like your idea of using the JSON object for storing the<br>
>>> parsed dml_adaptive_relationship_list<br>
>>> but the string manipulation we were doing for searching the function<br>
>>> names from the list was<br>
>>> pinching me a little since it is done for each statement.<br>
>>><br>
>>> So how about storing the object type at the time of parsing the list and<br>
>>> use that instead<br>
>>> of doing string manipulations for each function name searches.<br>
>>><br>
>>> With these two changes, I have cooked up a quick patch on top of your<br>
>>> 0001-dml-adaptive-patch-v4.<br>
>>> Can you have a look at the attached patch and see if you have any<br>
>>> reservations with this approach?<br>
>>><br>
>>> Also, note that I have just made this patch and haven't tested it, so I<br>
>>> am expecting that you may find<br>
>>> some issues with it :-)<br>
>>><br>
>>><br>
>>> Thanks<br>
>>> Best regards.<br>
>>><br>
>>><br>
>>><br>
>>> On Mon, Jun 29, 2020 at 6:36 AM <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a> <<a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a>><br>
>>> wrote:<br>
>>><br>
>>>> Hi, Usama<br>
>>>><br>
>>>> I made a new patch.Including the following：<br>
>>>> 1.Updated documentation<br>
>>>> 2.Used json to save relationship list<br>
>>>> 3.Changed 'dml_load_balance' to 'dml_adaptive'<br>
>>>> 4.Added extended-query-test<br>
>>>> 5.Tested 'git apply'<br>
>>>><br>
>>>> Thanks<br>
>>>> Best Regards<br>
>>>><br>
>>>> ------------------------------<br>
>>>> <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>>>><br>
>>>><br>
>>>> *From:* <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>>>> *Date:* 2020-06-22 10:57<br>
>>>> *To:* Muhammad Usama <<a href="mailto:m.usama@gmail.com" target="_blank">m.usama@gmail.com</a>><br>
>>>> *CC:* Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>>; pgpool-hackers<br>
>>>> <<a href="mailto:pgpool-hackers@pgpool.net" target="_blank">pgpool-hackers@pgpool.net</a>><br>
>>>> *Subject:* Re: [pgpool-hackers: 3592] add a feature: dml object level<br>
>>>> load balance<br>
>>>> Hi, Usama<br>
>>>><br>
>>>> I use the raw_expression_tree_walker() function to find out relname.<br>
>>>> This function is only valid for DML statements<br>
>>>> (SELECT/INSERT/UPDATE/DELETE).<br>
>>>> So I named this feature ‘dml_load_balance’.<br>
>>>><br>
>>>> ‘adaptive’ it feels like that can parse all statements. It can't<br>
>>>> actually.<br>
>>>> Maybe we can call it 'dml_adaptive'.<br>
>>>><br>
>>>> According to comments from you and Tatsuo Ishii, i will make a new patch.<br>
>>>><br>
>>>> Thanks<br>
>>>> Best Regards<br>
>>>> ------------------------------<br>
>>>> <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>>>><br>
>>>><br>
>>>> *From:* Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>><br>
>>>> *Date:* 2020-06-20 17:50<br>
>>>> *To:* <a href="mailto:m.usama@gmail.com" target="_blank">m.usama@gmail.com</a><br>
>>>> *CC:* <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a>; <a href="mailto:pgpool-hackers@pgpool.net" target="_blank">pgpool-hackers@pgpool.net</a><br>
>>>> *Subject:* Re: [pgpool-hackers: 3592] add a feature: dml object level<br>
>>>> load balance<br>
>>>> Hi Usama,<br>
>>>><br>
>>>> > Hi Sunbiao,<br>
>>>> ><br>
>>>> > Thanks for the updated patch. Overall the patch looks good and works as<br>
>>>> > expected.<br>
>>>> ><br>
>>>> > However, I am a little concerned about the performance aspect of<br>
>>>> > the check_object_relationship_list() function.<br>
>>>> > Since it is parsing the item in the<br>
>>>> > dml_load_balance_object_relationship_list<br>
>>>> > list every time it is invoked. So I think we need to fissure out the<br>
>>>> way to<br>
>>>> > store the pre-parsed list (which can be constructed at session start)<br>
>>>> and<br>
>>>> > try<br>
>>>> > to save the parsing at each function call.<br>
>>>> ><br>
>>>> > Other than that you need to provide the documentation updates for the<br>
>>>> > feature.<br>
>>>> ><br>
>>>> > Finally, one last comment is how about if we change the<br>
>>>> > disable_load_balance_on_write<br>
>>>> > setting name from 'dml_load_balance' to 'adaptive'?<br>
>>>> ><br>
>>>> ><br>
>>>> > @Tatsuo Ishii <<a href="mailto:ishii@sraoss.co.jp" target="_blank">ishii@sraoss.co.jp</a>><br>
>>>> > What do you think about this feature and patch? If you do not have any<br>
>>>> > reservations then<br>
>>>> > I will commit it after Sunbiao takes care of review comments.<br>
>>>><br>
>>>> Looks good to me except subtle points below:<br>
>>>><br>
>>>> - The test should include tests for extended query:<br>
>>>> i.e. src/test/extended-query-test. There are some tests or<br>
>>>> disable-load-balance. So it's better to add a test to them.<br>
>>>><br>
>>>> - There are extra spaces in the patch. Also it needs rebase.<br>
>>>><br>
>>>> t-ishii$ git apply ~/0001-dml-load-balance-patch-v3.patch<br>
>>>> /home/t-ishii/0001-dml-load-balance-patch-v3.patch:327: space before tab<br>
>>>> in indent.<br>
>>>> * dml_load_balance_object_relationship_list */<br>
>>>> /home/t-ishii/0001-dml-load-balance-patch-v3.patch:679: trailing<br>
>>>> whitespace.<br>
>>>> CREATE OR REPLACE FUNCTION insert_tb_t2_func() RETURNS TRIGGER AS<br>
>>>> $example_table$<br>
>>>> /home/t-ishii/0001-dml-load-balance-patch-v3.patch:680: trailing<br>
>>>> whitespace.<br>
>>>> BEGIN<br>
>>>> /home/t-ishii/0001-dml-load-balance-patch-v3.patch:681: trailing<br>
>>>> whitespace.<br>
>>>> INSERT INTO tb_t2 VALUES (1);<br>
>>>> /home/t-ishii/0001-dml-load-balance-patch-v3.patch:682: trailing<br>
>>>> whitespace.<br>
>>>> RETURN NEW;<br>
>>>> error: patch failed: src/context/pool_session_context.c:170<br>
>>>> error: src/context/pool_session_context.c: patch does not apply<br>
>>>><br>
>>>> > Thanks<br>
>>>> > Best Regards<br>
>>>> > Muhammad Usama<br>
>>>> ><br>
>>>> ><br>
>>>> > Thanks<br>
>>>> > Best Regards<br>
>>>> > Muhammad Usama<br>
>>>> ><br>
>>>> > On Mon, Jun 15, 2020 at 7:25 AM <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a> <<a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>>>> ><br>
>>>> > wrote:<br>
>>>> ><br>
>>>> >> Hi, Usama<br>
>>>> >> I found a problem in patch v2.<br>
>>>> >> I can not use “pool show” to show new added parameter.<br>
>>>> >> So i made patch v3.<br>
>>>> >><br>
>>>> >> I make disable_load_balance_on_write to accept new value.<br>
>>>> >> Set disable_load_balance_on_write = 'dml_load_balance' to enable this<br>
>>>> >> feature.<br>
>>>> >> This new patch contains a test script in path<br>
>>>> >> ‘src/test/dml-load-balance-test’.<br>
>>>> >> If pg installed by default in /usr/local/pgsql, just execute test.sh.<br>
>>>> >> If pg is in other dir, execute ‘test.sh -p /path_to_pg_dir/’.<br>
>>>> >> It will show “success: dml load balance test pass.” , when test pass.<br>
>>>> >><br>
>>>> >> this script will test below sql:<br>
>>>> >> show pool_nodes;<br>
>>>> >><br>
>>>> >> -- test DML<br>
>>>> >> begin ;<br>
>>>> >> insert into tb_dml_insert values (1);<br>
>>>> >> select * from tb_dml_insert ;<br>
>>>> >> commit ;<br>
>>>> >><br>
>>>> >> begin ;<br>
>>>> >> update tb_dml_update SET a = 2;<br>
>>>> >> select * from tb_dml_update ;<br>
>>>> >> commit ;<br>
>>>> >><br>
>>>> >> begin ;<br>
>>>> >> delete from tb_dml_delete;<br>
>>>> >> select * from tb_dml_delete;<br>
>>>> >> commit ;<br>
>>>> >><br>
>>>> >> -- test trigger<br>
>>>> >> begin ;<br>
>>>> >> insert into tb_t1 values (1);<br>
>>>> >> select * from tb_t2 ;<br>
>>>> >> commit ;<br>
>>>> >><br>
>>>> >> -- test function<br>
>>>> >> begin ;<br>
>>>> >> select insert_tb_f_func(6);<br>
>>>> >> select * from tb_f ;<br>
>>>> >> commit ;<br>
>>>> >><br>
>>>> >> -- test view<br>
>>>> >> begin ;<br>
>>>> >> insert into tb_v values (8);<br>
>>>> >> select * from tb_v_view ;<br>
>>>> >> commit ;<br>
>>>> >><br>
>>>> >> Thanks<br>
>>>> >> Best Regards<br>
>>>> >> ------------------------------<br>
>>>> >> <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>>>> >><br>
>>>> >><br>
>>>> >> *From:* Muhammad Usama <<a href="mailto:m.usama@gmail.com" target="_blank">m.usama@gmail.com</a>><br>
>>>> >> *Date:* 2020-06-12 18:08<br>
>>>> >> *To:* <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>>>> >> *CC:* pgpool-hackers <<a href="mailto:pgpool-hackers@pgpool.net" target="_blank">pgpool-hackers@pgpool.net</a>><br>
>>>> >> *Subject:* Re: [pgpool-hackers: 3592] add a feature: dml object level<br>
>>>> >> load balance<br>
>>>> >> Hi Sunbiao,<br>
>>>> >><br>
>>>> >> Can you kindly resend the 0001-dml-load-balance-patch-v2.patch patch?<br>
>>>> I<br>
>>>> >> am not able to download the patch file.<br>
>>>> >><br>
>>>> >> Thanks<br>
>>>> >> Best regards<br>
>>>> >> Muhammad Usama<br>
>>>> >><br>
>>>> >><br>
>>>> >> On Fri, Apr 24, 2020 at 9:55 PM Muhammad Usama <<a href="mailto:m.usama@gmail.com" target="_blank">m.usama@gmail.com</a>><br>
>>>> wrote:<br>
>>>> >><br>
>>>> >>> Hi Sunbiao,<br>
>>>> >>><br>
>>>> >>> Thanks for the patch and it looks line an interesting feature. I have<br>
>>>> >>> just skimmed<br>
>>>> >>> through the patch and I have a few of small comments.<br>
>>>> >>><br>
>>>> >>> 1 - Wouldn't it be better to add a new mode existing<br>
>>>> >>> disable_load_balance_on_write parameter<br>
>>>> >>> instead of adding a new configuration parameter i.e<br>
>>>> >>> dml_object_level_load_balance ?<br>
>>>> >>> You can make disable_load_balance_on_write to accept new value like<br>
>>>> >>> disable_load_balance_on_write = 'object' to enable this feature.<br>
>>>> >>><br>
>>>> >>> 2- The patch contains some invalid changes in<br>
>>>> pgpool.conf.sample-stream<br>
>>>> >>> file<br>
>>>> >>> i.e it changes the default values of black_function_list and<br>
>>>> >>> disable_load_balance_on_write<br>
>>>> >>> configuration parameters which I am sure were not intended<br>
>>>> >>><br>
>>>> >>> 3- The default value for new configuration parameter<br>
>>>> >>> dml_object_level_load_balance_token_list<br>
>>>> >>> better be an empty<br>
>>>> >>><br>
>>>> >>> 4- Instead of attaching a separate test script you could include a<br>
>>>> proper<br>
>>>> >>> test case for the feature.<br>
>>>> >>><br>
>>>> >>> Thanks<br>
>>>> >>> Best Regards<br>
>>>> >>><br>
>>>> >>> Muhammad Usama<br>
>>>> >>> Highgo Software (Canada/China/Pakistan)<br>
>>>> >>> URL : <a href="http://www.highgo.ca" rel="noreferrer" target="_blank">http://www.highgo.ca</a><br>
>>>> >>> ADDR: 10318 WHALLEY BLVD, Surrey, BC<br>
>>>> >>><br>
>>>> >>><br>
>>>> >>><br>
>>>> >>> On Fri, Apr 24, 2020 at 3:43 PM <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a> <<br>
>>>> <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a>><br>
>>>> >>> wrote:<br>
>>>> >>><br>
>>>> >>>> Hi Hackers,<br>
>>>> >>>> If sql like below:<br>
>>>> >>>><br>
>>>> >>>> begin ;<br>
>>>> >>>> update tb_1 SET id = 1;<br>
>>>> >>>> select * from tb_1 ;<br>
>>>> >>>> select * from tb_2 ;<br>
>>>> >>>> select * from tb_3 ;<br>
>>>> >>>> select * from tb_4 ;<br>
>>>> >>>> select * from tb_5 ;<br>
>>>> >>>> commit ;<br>
>>>> >>>><br>
>>>> >>>> when set disable_load_balance_on_write = 'transaction'. write<br>
>>>> queries<br>
>>>> >>>> appear in an explicit transaction, subsequent read queries are not<br>
>>>> load<br>
>>>> >>>> balanced until the transaction ends. so all sql will be sent to<br>
>>>> primary<br>
>>>> >>>> node.<br>
>>>> >>>><br>
>>>> >>>> i think that “update tb_1 SET id = 1” and “select * from tb_1”<br>
>>>> should be<br>
>>>> >>>> sent to primary node.<br>
>>>> >>>> actually, tb_2 tb_3 tb_4 tb_5 can be sent to standby node. if do<br>
>>>> this,<br>
>>>> >>>> will reduce primary load.<br>
>>>> >>>><br>
>>>> >>>> so i made a patch to implement my idea.<br>
>>>> >>>> when transaction start, i will initialize a list to save table name<br>
>>>> of<br>
>>>> >>>> write queries.<br>
>>>> >>>> read queries will check the list, if find the table name in list,<br>
>>>> read<br>
>>>> >>>> queries will be sent to primary.<br>
>>>> >>>> when transaction end, i destroy the list.<br>
>>>> >>>><br>
>>>> >>>> i add two parameter:<br>
>>>> >>>><br>
>>>> >>>> dml_object_level_load_balance = on<br>
>>>> >>>> dml_object_level_load_balance_token_list=<br>
>>>> >>>> 'tb_t1:tb_t2,insert_tb_f_func():tb_f,tb_v:tb_v_view'<br>
>>>> >>>><br>
>>>> >>>> use dml_object_level_load_balance_token_list to set relationships<br>
>>>> >>>> between objects, such as trigger, function, view.<br>
>>>> >>>> If set dml_object_level_load_balance = on,<br>
>>>> disable_load_balance_on_write<br>
>>>> >>>> should be off.<br>
>>>> >>>><br>
>>>> >>>> Is it possible to add this feature?<br>
>>>> >>>> ------------------------------<br>
>>>> >>>> <a href="mailto:sunbiao@highgo.com" target="_blank">sunbiao@highgo.com</a><br>
>>>> >>>> _______________________________________________<br>
>>>> >>>> pgpool-hackers mailing list<br>
>>>> >>>> <a href="mailto:pgpool-hackers@pgpool.net" target="_blank">pgpool-hackers@pgpool.net</a><br>
>>>> >>>> <a href="http://www.pgpool.net/mailman/listinfo/pgpool-hackers" rel="noreferrer" target="_blank">http://www.pgpool.net/mailman/listinfo/pgpool-hackers</a><br>
>>>> >>>><br>
>>>> >>><br>
>>>><br>
>>>><br>
>>><br>
>>> --<br>
>>> ...<br>
>>> Muhammad Usama<br>
>>> Highgo Software (Canada/China/Pakistan)<br>
>>> URL : <a href="http://www.highgo.ca" rel="noreferrer" target="_blank">http://www.highgo.ca</a><br>
>>> ADDR: 10318 WHALLEY BLVD, Surrey, BC<br>
>>><br>
>>><br>
</blockquote></div></div>
</div>