[pgpool-hackers: 1162] Re: pgpool-II 3.4 problems
Tatsuo Ishii
ishii at postgresql.org
Wed Nov 18 07:50:03 JST 2015
Hi Usama,
> Hi Ishii-San
>
> Thanks for reviewing the patch.
>
> On Tue, Nov 17, 2015 at 7:31 AM, Tatsuo Ishii <ishii at postgresql.org> wrote:
>
>> 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.
>>
>
> Yes this is correct the result will be same after this new if statement.
> The reason I made this change was to bypass if statement with four
> strcmp()s when we already knew the cache_connection will remain false.
> Although might not make much difference but done this to improve
> performance of this condition statement.
Ok, that makes sense.
>> 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.
>>
>
> Yes, you are right. frontend_invalid should've been bool. I have updated
> this in attached version of the patch.
Thanks. Looks good to me.
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