[pgpool-hackers: 3703] Re: add a feature: dml object level load balance
sunbiao at highgo.com
sunbiao at highgo.com
Tue Jul 7 11:07:27 JST 2020
Hi, Usama
I have seen patch-v7 and tested it.
I think removing the parentheses () is a good way.
Thanks
Best Regards
sunbiao at highgo.com
From: Muhammad Usama
Date: 2020-07-07 01:19
To: sunbiao at highgo.com
CC: Tatsuo Ishii; pgpool-hackers
Subject: Re: Re: [pgpool-hackers: 3592] add a feature: dml object level load balance
Hi Sunbiao,
Thanks for pointing the mistake.
So I have yet again changed the patch a little bit.
Instead of doing strncasecmp in get_associated_object_from_dml_adaptive_relations function,
I have removed the parentheses () from the original token at the time of parsing.
What do you think? see the new attached patch.
Thanks
Best Regards
Muhammad Usama
Thanks for the updated patch.
On Mon, Jul 6, 2020 at 10:37 AM sunbiao at highgo.com <sunbiao at highgo.com> wrote:
Hi, Usama
I think your idea is better then using json.
I tested patch-v5. There was a bug in the function "get_associated_object_from_dml_adaptive_relations" , when sql calls a function.
Can you please
I fixed it in the new patch.
Thanks
Best regards.
sunbiao at highgo.com
From: Muhammad Usama
Date: 2020-07-02 05:15
To: sunbiao at highgo.com
CC: Tatsuo Ishii; pgpool-hackers
Subject: Re: [pgpool-hackers: 3592] add a feature: dml object level load balance
Hi Sunbiao,
Thanks for the patch, It is working as expected. Though while reviewing it
I realized we are doing the dml_adaptive_relationship_list parsing at the start of every
session, That is better than the previous approach but still has room for improvement.
My idea is to do the parsing at the very beginning while loading the configuration parameter
and then use that.
Secondly, I do like your idea of using the JSON object for storing the parsed dml_adaptive_relationship_list
but the string manipulation we were doing for searching the function names from the list was
pinching me a little since it is done for each statement.
So how about storing the object type at the time of parsing the list and use that instead
of doing string manipulations for each function name searches.
With these two changes, I have cooked up a quick patch on top of your 0001-dml-adaptive-patch-v4.
Can you have a look at the attached patch and see if you have any reservations with this approach?
Also, note that I have just made this patch and haven't tested it, so I am expecting that you may find
some issues with it :-)
Thanks
Best regards.
On Mon, Jun 29, 2020 at 6:36 AM sunbiao at highgo.com <sunbiao at highgo.com> wrote:
Hi, Usama
I made a new patch.Including the following：
1.Updated documentation
2.Used json to save relationship list
3.Changed 'dml_load_balance' to 'dml_adaptive'
4.Added extended-query-test
5.Tested 'git apply'
Thanks
Best Regards
sunbiao at highgo.com
From: sunbiao at highgo.com
Date: 2020-06-22 10:57
To: Muhammad Usama
CC: Tatsuo Ishii; pgpool-hackers
Subject: Re: [pgpool-hackers: 3592] add a feature: dml object level load balance
Hi, Usama
I use the raw_expression_tree_walker() function to find out relname.
This function is only valid for DML statements (SELECT/INSERT/UPDATE/DELETE).
So I named this feature ‘dml_load_balance’.
‘adaptive’ it feels like that can parse all statements. It can't actually.
Maybe we can call it 'dml_adaptive'.
According to comments from you and Tatsuo Ishii, i will make a new patch.
Thanks
Best Regards
sunbiao at highgo.com
From: Tatsuo Ishii
Date: 2020-06-20 17:50
To: m.usama at gmail.com
CC: sunbiao at highgo.com; pgpool-hackers at pgpool.net
Subject: Re: [pgpool-hackers: 3592] add a feature: dml object level load balance
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
>>>>
>>>
--
...
Muhammad Usama
Highgo Software (Canada/China/Pakistan)
URL : http://www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200707/b68de2fc/attachment-0001.html>
More information about the pgpool-hackers
mailing list