[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