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

[Bo Peng]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: statement_level_load_balancing_v2.patch
Type: application/octet-stream
Size: 30888 bytes
Desc: not available
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20190329/ad6baae5/attachment-0001.obj>