[pgpool-hackers: 1160] Re: pgpool-II 3.4 problems
Tatsuo Ishii
ishii at postgresql.org
Tue Nov 17 11:31:36 JST 2015
Hi Usama,
I have taken a look at your patch.
1) Below diff seems to be unnecessary because even without the "+ if
(cache_connection)", the result would be same.
- /*
- * For those special databases, and when frontend client exits abnormally,
- * we don't cache connection to backend.
- */
- if ((sp &&
- (!strcmp(sp->database, "template0") ||
- !strcmp(sp->database, "template1") ||
- !strcmp(sp->database, "postgres") ||
- !strcmp(sp->database, "regression"))) ||
- (*frontend != NULL &&
- ((*frontend)->socket_state == POOL_SOCKET_EOF ||
- (*frontend)->socket_state == POOL_SOCKET_ERROR)))
- cache_connection = false;
-
+ if (cache_connection)
+ {
+ /*
+ * For those special databases, and when frontend client exits abnormally,
+ * we don't cache connection to backend.
+ */
+ if ((sp &&
+ (!strcmp(sp->database, "template0") ||
+ !strcmp(sp->database, "template1") ||
+ !strcmp(sp->database, "postgres") ||
+ !strcmp(sp->database, "regression"))) ||
+ (*frontend != NULL &&
+ ((*frontend)->socket_state == POOL_SOCKET_EOF ||
+ (*frontend)->socket_state == POOL_SOCKET_ERROR)))
+ cache_connection = false;
+ }
2) Shouldn't frontend_invalid variable be bool because it takes only 0
or 1. If so, geterrposition()'s return type should be declared as
bool.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
> Hi Ishii San
>
> Can you please have a look at the attached patch for 3.4 branch.
> Basically it introduces a new error level FRONTEND_ERROR for ereport(), to
> inform the error handler not to cache the current backend connection. Other
> than that, this FRONTEND_ERROR error level behaves and works similar to the
> ereport(ERROR..)
>
> Thanks
> Best regards
> Muhammad Usama
>
>
> On Thu, Oct 15, 2015 at 4:45 PM, Muhammad Usama <m.usama at gmail.com> wrote:
>
>>
>>
>> On Thu, Oct 15, 2015 at 2:23 AM, Tatsuo Ishii <ishii at postgresql.org>
>> wrote:
>>
>>> Usama,
>>>
>>> Can you please propose a patch to implement similar thing with 3.4? I
>>> think it will make 3.4 more robust. I have no idea how to implement
>>> it with 3.4's exception mechanism.
>>>
>>
>> sure I will work on this
>>
>> 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
>>>
>>> > Hi Ishii San
>>> >
>>> > I think this difference of behavior between 3.3. And 3.4 is caused by
>>> the
>>> > commit "cb735f22441001b6afdbb5bac72808db66094ca9" that was not ported to
>>> > the 3.4 branch.
>>> >
>>> >
>>> http://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=cb735f22441001b6afdbb5bac72808db66094ca9
>>> >
>>> > Thanks
>>> > Best regards
>>> > Muhammad Usama
>>> >
>>> >
>>> >
>>> > On Tue, Oct 13, 2015 at 2:45 PM, Tatsuo Ishii <ishii at postgresql.org>
>>> wrote:
>>> >
>>> >> > Usama,
>>> >> >
>>> >> > While I have been working on:
>>> >> > http://www.pgpool.net/mantisbt/view.php?id=147
>>> >>
>>> >> Oops. This is wrong. Here is the correct one.
>>> >>
>>> >> http://www.pgpool.net/mantisbt/view.php?id=145
>>> >>
>>> >> > I found there was an interesting behavior change between pgpool-II
>>> 3.3
>>> >> > and 3.4.
>>> >> >
>>> >> > It turns out that the bug was related to the action when
>>> >> > client_idle_limit is reached. In 3.3, when client_idle_limit is
>>> >> > reached, following statement is executed (pool_process_query.c):
>>> >> >
>>> >> > if (idle_count > pool_config->client_idle_limit)
>>> >> > {
>>> >> > pool_log("pool_process_query: child connection forced to
>>> terminate
>>> >> due to client_idle_limit (%d) reached,
>>> >> > pool_config->client_idle_limit);
>>> >> > pool_send_error_message(frontend, MAJOR(backend),
>>> >> > "57000", "connection terminated due to client idle
>>> limit
>>> >> reached",
>>> >> > "","", __FILE__, __LINE__);
>>> >> > return POOL_END_WITH_FRONTEND_ERROR;
>>> >> > }
>>> >> >
>>> >> > then goes to this (child.c)
>>> >> >
>>> >> > if (status == POOL_END_WITH_FRONTEND_ERROR ||
>>> >> > pool_config->connection_cache == 0 ||
>>> >> > !strcmp(sp->database, "template0") ||
>>> >> > !strcmp(sp->database, "template1") ||
>>> >> > !strcmp(sp->database, "postgres") ||
>>> >> > !strcmp(sp->database, "regression"))
>>> >> > {
>>> >> > reset_connection();
>>> >> > pool_close(frontend);
>>> >> > pool_send_frontend_exits(backend);
>>> >> > pool_discard_cp(sp->user, sp->database, sp->major);
>>> >> > }
>>> >> >
>>> >> > This results in closing connection to backend.
>>> >> >
>>> >> > In 3.4,
>>> >> >
>>> >> > if (idle_count > pool_config->client_idle_limit)
>>> >> > {
>>> >> > ereport(ERROR,
>>> >> > (pool_error_code("57000"),
>>> >> > errmsg("unable to read data"),
>>> >> > errdetail("child connection forced to terminate due
>>> to
>>> >> client_idle_limit:%d is reached",
>>> >> > pool_config->client_idle_limit)));
>>> >> > }
>>> >> >
>>> >> > Then jump to backend_cleanup() and call pool_process_query() to
>>> >> > execute queries defined in reset_query_list. This results in sending
>>> >> > typically "DISCARD ALL" to backend. The problem reported in the bug
>>> id
>>> >> > 147 happens in this route (actually the reporter claims that
>>> pgpool-II
>>> >> > hangs in sending "DISCARD").
>>> >> >
>>> >> > In summary, when client idle limit reaches, pgpool-II 3.3 disconnects
>>> >> > connections to backend, while 3.4 does not close the connections to
>>> >> > backend and sends DISCARD to backend.
>>> >> >
>>> >> > We have a few reports that pgpool-II 3.4 hangs while sending DISCARD
>>> >> > and I wonder those problems somewhat related to the behavior change
>>> >> > described above because the code path in 3.4 is executed even if
>>> >> > client_idle_limit is not reached (elog(ERROR...)).
>>> >> >
>>> >> > Usama,
>>> >> >
>>> >> > Was this behavior change intentional?
>>> >> >
>>> >> > Best regards,
>>> >> > --
>>> >> > Tatsuo Ishii
>>> >> > SRA OSS, Inc. Japan
>>> >> > English: http://www.sraoss.co.jp/index_en.php
>>> >> > Japanese:http://www.sraoss.co.jp
>>> >> > _______________________________________________
>>> >> > pgpool-hackers mailing list
>>> >> > pgpool-hackers at pgpool.net
>>> >> > http://www.pgpool.net/mailman/listinfo/pgpool-hackers
>>> >> _______________________________________________
>>> >> pgpool-hackers mailing list
>>> >> pgpool-hackers at pgpool.net
>>> >> http://www.pgpool.net/mailman/listinfo/pgpool-hackers
>>> >>
>>>
>>
>>
More information about the pgpool-hackers
mailing list