[pgpool-hackers: 3808] Re: add a feature: dml object level load balance
Muhammad Usama
m.usama at gmail.com
Tue Sep 8 15:55:22 JST 2020
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
> >> >>>
> >> >>>
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200908/465c687d/attachment-0001.html>
More information about the pgpool-hackers
mailing list