[pgpool-hackers: 3813] Re: add a feature: dml object level load balance
Tatsuo Ishii
ishii at sraoss.co.jp
Wed Sep 16 07:45:02 JST 2020
Hi Usama,
I haven't seen the push yet...
From: Muhammad Usama <m.usama at gmail.com>
Subject: Re: [pgpool-hackers: 3592] add a feature: dml object level load balance
Date: Tue, 8 Sep 2020 11:55:22 +0500
Message-ID: <CAEJvTzVnst4j+=CZUAwOniuPsy6+tD5MLob2O3-wCu3Gq4-hUw at mail.gmail.com>
> Hi Ishii-San
>
> On Tue, Sep 8, 2020 at 6:55 AM Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
>
>> Hi Usama,
>>
>> Any progress on this?
>>
>
> Sorry for the delay. I will push the fix today
>
> Thanks
> Best regards
> Muhammad Usama
>
>>
>> Best regards,
>> --
>> Tatsuo Ishii
>> SRA OSS, Inc. Japan
>> English: http://www.sraoss.co.jp/index_en.php
>> Japanese:http://www.sraoss.co.jp
>>
>> From: Muhammad Usama <m.usama at gmail.com>
>> Subject: Re: [pgpool-hackers: 3592] add a feature: dml object level load
>> balance
>> Date: Wed, 22 Jul 2020 09:24:59 +0500
>> Message-ID: <
>> CAEJvTzWrJngV_NEZ19Z8FRuW4bObiwt5soa-ivCMTXDvWPrSCw at mail.gmail.com>
>>
>> > On Wed, Jul 22, 2020 at 7:32 AM Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
>> >
>> >> Hi Usama,
>> >>
>> >> Why the test is under
>> src/test/dml-adaptive-test/simple-query-test/test.sh?
>> >> It would be good if it's located under the standard regression test
>> >> directory.
>> >>
>> >
>> > Thanks for pointing it out, I somehow overlooked it.
>> > I will take care of it and move the simple query test case
>> > to the standard regression directory.
>> >
>> > Thanks
>> > Best Regards
>> >
>> >>
>> >> Best regards,
>> >> --
>> >> Tatsuo Ishii
>> >> SRA OSS, Inc. Japan
>> >> English: http://www.sraoss.co.jp/index_en.php
>> >> Japanese:http://www.sraoss.co.jp
>> >>
>> >> > Hi Sunbiao,
>> >> >
>> >> >
>> >> > I have committed your patch after a few minor modifications and
>> adjusting
>> >> > the documentation.
>> >> >
>> >>
>> https://git.postgresql.org/gitweb?p=pgpool2.git;a=commitdiff;h=761d30a17d04350fb3a67bce43440731c8ba3c90
>> >> >
>> >> >
>> >> > Best Regards
>> >> >
>> >> > On Tue, Jul 7, 2020 at 7:08 AM sunbiao at highgo.com <sunbiao at highgo.com
>> >
>> >> > wrote:
>> >> >
>> >> >> 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 <m.usama at gmail.com>
>> >> >> *Date:* 2020-07-07 01:19
>> >> >> *To:* sunbiao at highgo.com
>> >> >> *CC:* Tatsuo Ishii <ishii at sraoss.co.jp>; pgpool-hackers
>> >> >> <pgpool-hackers at pgpool.net>
>> >> >> *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 <m.usama at gmail.com>
>> >> >>> *Date:* 2020-07-02 05:15
>> >> >>> *To:* sunbiao at highgo.com
>> >> >>> *CC:* Tatsuo Ishii <ishii at sraoss.co.jp>; pgpool-hackers
>> >> >>> <pgpool-hackers at pgpool.net>
>> >> >>> *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 <m.usama at gmail.com>
>> >> >>>> *CC:* Tatsuo Ishii <ishii at sraoss.co.jp>; pgpool-hackers
>> >> >>>> <pgpool-hackers at pgpool.net>
>> >> >>>> *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 <ishii at sraoss.co.jp>
>> >> >>>> *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
>> >> >>>
>> >> >>>
>> >>
>>
More information about the pgpool-hackers
mailing list