[pgpool-hackers: 1164] Re: pgpool-II 3.4 problems
Muhammad Usama
m.usama at gmail.com
Thu Nov 19 00:40:25 JST 2015
Hi Ishii-San
Many thanks for the review. I have pushed the fix in 3_4 and master branches
Best regards
Muhammad Usama
On Wed, Nov 18, 2015 at 3:50 AM, Tatsuo Ishii <ishii at postgresql.org> wrote:
> 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
> >> >>> >>
> >> >>>
> >> >>
> >> >>
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20151118/d354c3bf/attachment-0001.html>
More information about the pgpool-hackers
mailing list