<html><head><meta http-equiv="content-type" content="text/html; charset=iso-2022-jp"><style>body { line-height: 1.5; }blockquote { margin-top: 0px; margin-bottom: 0px; margin-left: 0.5em; }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>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>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:ishii@sraoss.co.jp">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">m.usama@gmail.com</a></div><div><b>CC:</b> <a href="mailto:sunbiao@highgo.com">sunbiao@highgo.com</a>; <a href="mailto:pgpool-hackers@pgpool.net">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 <ishii@sraoss.co.jp></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 sunbiao@highgo.com <sunbiao@highgo.com></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>>> sunbiao@highgo.com</div>
<div>>></div>
<div>>></div>
<div>>> *From:* Muhammad Usama <m.usama@gmail.com></div>
<div>>> *Date:* 2020-06-12 18:08</div>
<div>>> *To:* sunbiao@highgo.com</div>
<div>>> *CC:* pgpool-hackers <pgpool-hackers@pgpool.net></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 <m.usama@gmail.com> 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 : http://www.highgo.ca</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 sunbiao@highgo.com <sunbiao@highgo.com></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>>>>> sunbiao@highgo.com</div>
<div>>>>> _______________________________________________</div>
<div>>>>> pgpool-hackers mailing list</div>
<div>>>>> pgpool-hackers@pgpool.net</div>
<div>>>>> http://www.pgpool.net/mailman/listinfo/pgpool-hackers</div>
<div>>>>></div>
<div>>>></div>
</div></blockquote>
</body></html>