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

Mon Jun 22 21:03:38 JST 2020

On Mon, Jun 22, 2020 at 7:58 AM sunbiao at highgo.com <sunbiao at highgo.com>
wrote:

> 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'.
>

Yes seems like a good choice.

>
> According to comments from you and Tatsuo Ishii, i will make a new patch.
>

Thanks
Looking forward to that.

Best Regards
Muhammad Usama

>
> 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
> >>>>
> >>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200622/503c7b25/attachment.html>