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