[pgpool-hackers: 3663] Re: add a feature: dml object level load balance

[Tatsuo Ishii]

Hi Usama,

> Hi Sunbiao,
> 
> Thanks for the updated patch. Overall the patch looks good and works as
> expected.
> 
> However, I am a little concerned about the performance aspect of
> the check_object_relationship_list() function.
> Since it is parsing the item in the
> dml_load_balance_object_relationship_list
> list every time it is invoked. So I think we need to fissure out the way to
> store the pre-parsed list (which can be constructed at session start) and
> try
> to save the parsing at each function call.
> 
> Other than that you need to provide the documentation updates for the
> feature.
> 
> Finally, one last comment is how about if we change the
> disable_load_balance_on_write
> setting name from 'dml_load_balance' to 'adaptive'?
> 
> 
> @Tatsuo Ishii <ishii at sraoss.co.jp>
> What do you think about this feature and patch? If you do not have any
> reservations then
> I will commit it after Sunbiao takes care of review comments.

Looks good to me except subtle points below:

- The test should include tests for extended query:
  i.e. src/test/extended-query-test. There are some tests or
  disable-load-balance. So it's better to add a test to them.

- There are extra spaces in the patch. Also it needs rebase.

t-ishii$ git apply ~/0001-dml-load-balance-patch-v3.patch 
/home/t-ishii/0001-dml-load-balance-patch-v3.patch:327: space before tab in indent.
													 			 * dml_load_balance_object_relationship_list */
/home/t-ishii/0001-dml-load-balance-patch-v3.patch:679: trailing whitespace.
CREATE OR REPLACE FUNCTION insert_tb_t2_func() RETURNS TRIGGER AS $example_table$  
/home/t-ishii/0001-dml-load-balance-patch-v3.patch:680: trailing whitespace.
    BEGIN  
/home/t-ishii/0001-dml-load-balance-patch-v3.patch:681: trailing whitespace.
        INSERT INTO tb_t2 VALUES (1);  
/home/t-ishii/0001-dml-load-balance-patch-v3.patch:682: trailing whitespace.
        RETURN NEW;   
error: patch failed: src/context/pool_session_context.c:170
error: src/context/pool_session_context.c: patch does not apply

> Thanks
> Best Regards
> Muhammad Usama
> 
> 
> Thanks
> Best Regards
> Muhammad Usama
> 
> On Mon, Jun 15, 2020 at 7:25 AM sunbiao at highgo.com <sunbiao at highgo.com>
> wrote:
> 
>> Hi, Usama
>> I found a problem in patch v2.
>> I can not use “pool show” to show new added parameter.
>> So i made patch v3.
>>
>> I make disable_load_balance_on_write to accept new value.
>> Set disable_load_balance_on_write = 'dml_load_balance' to enable this
>> feature.
>> This new patch contains a test script in path
>> ‘src/test/dml-load-balance-test’.
>> If pg installed by default in /usr/local/pgsql, just execute test.sh.
>> If pg is in other dir, execute  ‘test.sh -p /path_to_pg_dir/’.
>> It will show “success: dml load balance test pass.” , when test pass.
>>
>> this script will test below sql:
>> show pool_nodes;
>>
>> -- test DML
>> begin ;
>> insert into tb_dml_insert values (1);
>> select * from tb_dml_insert ;
>> commit ;
>>
>> begin ;
>> update tb_dml_update SET a = 2;
>> select * from tb_dml_update ;
>> commit ;
>>
>> begin ;
>> delete from tb_dml_delete;
>> select * from tb_dml_delete;
>> commit ;
>>
>> -- test trigger
>> begin ;
>> insert into tb_t1 values (1);
>> select * from tb_t2 ;
>> commit ;
>>
>> -- test function
>> begin ;
>> select insert_tb_f_func(6);
>> select * from tb_f ;
>> commit ;
>>
>> -- test view
>> begin ;
>> insert into tb_v values (8);
>> select * from tb_v_view ;
>> commit ;
>>
>> Thanks
>> Best Regards
>> ------------------------------
>> sunbiao at highgo.com
>>
>>
>> *From:* Muhammad Usama <m.usama at gmail.com>
>> *Date:* 2020-06-12 18:08
>> *To:* sunbiao at highgo.com
>> *CC:* pgpool-hackers <pgpool-hackers at pgpool.net>
>> *Subject:* Re: [pgpool-hackers: 3592] add a feature: dml object level
>> load balance
>> Hi Sunbiao,
>>
>> Can you kindly resend the 0001-dml-load-balance-patch-v2.patch patch? I
>> am not able to download the patch file.
>>
>> Thanks
>> Best regards
>> Muhammad Usama
>>
>>
>> On Fri, Apr 24, 2020 at 9:55 PM Muhammad Usama <m.usama at gmail.com> wrote:
>>
>>> Hi Sunbiao,
>>>
>>> Thanks for the patch and it looks line an interesting feature. I have
>>> just skimmed
>>> through the patch and I have a few of small comments.
>>>
>>> 1 - Wouldn't it be better to add a new mode existing
>>> disable_load_balance_on_write parameter
>>> instead of adding a new configuration parameter i.e
>>> dml_object_level_load_balance ?
>>> You can make disable_load_balance_on_write to accept new value like
>>> disable_load_balance_on_write = 'object' to enable this feature.
>>>
>>> 2- The patch contains some invalid changes in pgpool.conf.sample-stream
>>> file
>>> i.e it changes the default values of black_function_list and
>>> disable_load_balance_on_write
>>> configuration parameters which I am sure were not intended
>>>
>>> 3- The default value for new configuration parameter
>>> dml_object_level_load_balance_token_list
>>> better be an empty
>>>
>>> 4- Instead of attaching a separate test script you could include a proper
>>> test case for the feature.
>>>
>>> Thanks
>>> Best Regards
>>>
>>> Muhammad Usama
>>> Highgo Software (Canada/China/Pakistan)
>>> URL : http://www.highgo.ca
>>> ADDR: 10318 WHALLEY BLVD, Surrey, BC
>>>
>>>
>>>
>>> On Fri, Apr 24, 2020 at 3:43 PM sunbiao at highgo.com <sunbiao at highgo.com>
>>> wrote:
>>>
>>>> Hi Hackers,
>>>> If sql like below:
>>>>
>>>> begin ;
>>>> update tb_1 SET id = 1;
>>>> select * from tb_1 ;
>>>> select * from tb_2 ;
>>>> select * from tb_3 ;
>>>> select * from tb_4 ;
>>>> select * from tb_5 ;
>>>> commit ;
>>>>
>>>> when set disable_load_balance_on_write = 'transaction'. write queries
>>>> appear in an explicit transaction, subsequent read queries are not load
>>>> balanced until the transaction ends. so all sql will be sent to primary
>>>> node.
>>>>
>>>> i think that “update tb_1 SET id = 1” and “select * from tb_1” should be
>>>> sent to primary node.
>>>> actually, tb_2 tb_3 tb_4 tb_5 can be sent to standby node. if do this,
>>>> will reduce primary load.
>>>>
>>>> so i made a patch to implement my idea.
>>>> when transaction start, i will initialize a list to save table name of
>>>> write queries.
>>>> read queries will check the list, if find the table name in list, read
>>>> queries will be sent to primary.
>>>> when transaction end, i destroy the list.
>>>>
>>>> i add two parameter:
>>>>
>>>> dml_object_level_load_balance = on
>>>> dml_object_level_load_balance_token_list=
>>>> 'tb_t1:tb_t2,insert_tb_f_func():tb_f,tb_v:tb_v_view'
>>>>
>>>> use dml_object_level_load_balance_token_list to set relationships
>>>> between objects, such as trigger, function, view.
>>>> If set dml_object_level_load_balance = on, disable_load_balance_on_write
>>>> should be off.
>>>>
>>>> Is it possible to add this feature?
>>>> ------------------------------
>>>> sunbiao at highgo.com
>>>> _______________________________________________
>>>> pgpool-hackers mailing list
>>>> pgpool-hackers at pgpool.net
>>>> http://www.pgpool.net/mailman/listinfo/pgpool-hackers
>>>>
>>>