[pgpool-hackers: 3288] Re: New feature proposal: statement_level_load_balancing

Tatsuo Ishii ishii at sraoss.co.jp
Fri Mar 29 18:05:01 JST 2019


Hi Peng,

Now everything looks good. Please go ahead and commit/push.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

> Ishii-san,
> 
> Thank you for reviewing my patch.
> 
> On Fri, 29 Mar 2019 09:12:24 +0900 (JST)
> Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
> 
>> > Hi All
>> > 
>> > Here is a patch for new feature "statement_level_load_balancing".
>> > This feature enables selecting load balancing node per statement.
>> > 
>> > The current feature for load balancing, the load balancing node is decided 
>> > at the session start time and will not be changed until the session ends. 
>> > 
>> > When set to statement_level_load_balancing = on, the load balancing node 
>> > is decided for each read query.
>> >  
>> > For example, in applications that use connection pooling remain connections 
>> > open to the backend server, because the session may be held for a long time, 
>> > the load balancing node does not change until the session ends.
>> > In such applications, when statement_level_load_balance is enabled, 
>> > it is possible to decide load balancing node per query, not per session.
>> > 
>> > The default is off.
>> 
>> Great!
>> 
>> Here are some comments on the patch.
>> 
>> (1) There are some unnecessary spaces in the patch.
>> 
>> t-ishii$ git apply ~/statement_level_load_balancing_v1.patch 
>> /home/t-ishii/statement_level_load_balancing_v1.patch:22: trailing whitespace.
>> 		 When set to off, load balancing node is decided at the session start time 
>> /home/t-ishii/statement_level_load_balancing_v1.patch:23: trailing whitespace.
>> 		 and will not be changed until the session ends. 
>> /home/t-ishii/statement_level_load_balancing_v1.patch:24: trailing whitespace.
>> 		 For example, in applications that use connection pooling remain connections 
>> /home/t-ishii/statement_level_load_balancing_v1.patch:25: trailing whitespace.
>> 		 open to the backend server, because the session may be held for a long time, 
>> /home/t-ishii/statement_level_load_balancing_v1.patch:27: trailing whitespace.
>> 		 In such applications, When <varname>statement_level_load_balance</varname> is enabled, 
>> warning: squelched 10 whitespace errors
>> warning: 15 lines add whitespace errors.
> 
> I fixed these unnecessary spaces.
> 
>> (2) I encountered errors while executing extended-query-test.
>> :
>> :
>> testing query-cache.data ... ok.
>> testing select-multi-rows.data ... ok.
>> testing sql-error.data ... ok.
>> testing unable_to_bind.data ... ok.
>> ./test.sh: line 226: cd: tests_n3: No such file or directory
>> creating test database with 3 nodes...done.
>> testing README ... grep: /home/t-ishii/work/Pgpool-II/current/pgpool2/src/test/extended-query-test/tests_n3/README: No such file or directory
>> sed: can't read /home/t-ishii/work/Pgpool-II/current/pgpool2/src/test/extended-query-test/expected/README: No such file or directory
>> failed.
>> testing expected ... grep: /home/t-ishii/work/Pgpool-II/current/pgpool2/src/test/extended-query-test/tests_n3/expected: No such file or directory
> 
> I forgot to add these new files.
> I added these file to this new patch.
>  
>> (3) It is stated that "This parameter can be changed by reloading the
>> <productname>Pgpool-II</> configurations." in the manual, but actually
>> it only can be changed when Pgpool-II restarts.
> 
> I changed this parameter to CFGCXT_RELOAD.
> 
>> (4) Now that QUERT_CONTEXT->where_to_send and
>> POOL_PENDING_MESSAGE->node_ids are in the same type bool and the node
>> ids in those data are in the same order, why can't we replace following:
>> 
>> void
>> pool_pending_message_dest_set(POOL_PENDING_MESSAGE * message, POOL_QUERY_CONTEXT * query_context)
>> {
>> 	int			i;
>> 
>> 	for (i = 0; i < MAX_NUM_BACKENDS; i++)
>> 	{
>> 		if (query_context->where_to_send[i])
>> 			message->node_ids[i] = true;
>> 	}
>> 
>> to something like;
>> 
>> memcpy(message->node_ids, query_context->where_to_send, sizeof(message->node_ids));
>> 
>> I think this is much faster than iterating over where_to_send.
> 
> Thank you.
> Yes. Now node_ids is same like where_to_send,
> so we can use memcpy.
> I fixed this in new patch.
> 
>> 
>> Best regards,
>> --
>> Tatsuo Ishii
>> SRA OSS, Inc. Japan
>> English: http://www.sraoss.co.jp/index_en.php
>> Japanese:http://www.sraoss.co.jp
>> 
> 
> 
> -- 
> Bo Peng <pengbo at sraoss.co.jp>
> SRA OSS, Inc. Japan


More information about the pgpool-hackers mailing list