[pgpool-hackers: 1703] Re: kind does not match error in pgpool

[Tatsuo Ishii]

> Hi Ishii-San
> 
> Thanks for your valuable input on this, I have pushed the patch to the
> master branch.
> Do we also need to back port to all branches?

Yes, I think we need to, since this is a bug fix.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

> Regards
> Muhammad Usama
> 
> On Wed, Jul 13, 2016 at 7:02 AM, Tatsuo Ishii <ishii at postgresql.org> wrote:
> 
>> > On Fri, Jul 1, 2016 at 10:22 AM, Tatsuo Ishii <ishii at postgresql.org>
>> wrote:
>> >
>> >> > On Thu, Jun 30, 2016 at 1:51 PM, Tatsuo Ishii <ishii at postgresql.org>
>> >> wrote:
>> >> >
>> >> >> Usama,
>> >> >>
>> >> >> > Totally agreed on both the points. I think you have a valid point
>> and
>> >> >> > unconditionally forwarding all the kind = N messages to frontend is
>> >> the
>> >> >> > best choice. The attached version 2 of the patch does the same as
>> >> >> suggested.
>> >> >>
>> >> >> Thanks for the patch. BTW, I had hard time to test the patch. The
>> >> >> original problem report was talking about VACUUM case, but in
>> >> >> streaming replication mode, VACUUM is sent to primary node only and
>> >> >> kind mismatch error can't happend with VACUUM command.
>> >> >>
>> >> >
>> >> > The original bug report listed a case where one of the PostgreSQL
>> server
>> >> > was throwing a warning message because of transaction wrap around and
>> was
>> >> > suggesting to perform vacuum. The original query was not a VACUUM
>> >> command.
>> >> >
>> >> >
>> >> >> Any suggestion to trigger kind mismatch errors in streaming
>> >> >> replication mode?
>> >> >>
>> >> >
>> >> > Yes I ran into similar issue and mimicking the exact reported scenario
>> >> and
>> >> > getting the transaction wrap around warning is a very hard to
>> generate.
>> >> > So I tested the patch by setting the lower value of
>> client_min_messages
>> >> > variable on one of the backend servers and also set log_statement =
>> 'all'
>> >> >
>> >> > i.e.
>> >> > *Backend 1*
>> >> > log_statement = 'all'
>> >> > client_min_messages = debug2
>> >> >
>> >> > *Backend 2*
>> >> > log_statement = 'all'
>> >> > client_min_messages = notice
>> >> >
>> >> >
>> >> > Doing this ensures that one server will always send a original query
>> as a
>> >> > log message back to client application. And after that I tested the
>> patch
>> >> > using SET commands, since they are sent to all attached backends and
>> >> > because of different client_min_messages backend settings I was
>> getting
>> >> an
>> >> > extra log message form backend 1
>> >>
>> >> Looks like now NOTICE messages are duplicated according to the number
>> >> of backends if the SQL command is sent to each backend.
>> >>
>> >> Before:
>> >>
>> >> test=# begin;
>> >> LOG:  statement: begin;
>> >> BEGIN
>> >>
>> >> After your patch:
>> >>
>> >> test=# begin;
>> >> LOG:  statement: begin;
>> >> LOG:  statement: begin;
>> >> BEGIN
>> >>
>> >> I'm not sure if this is a desirable change.
>> >>
>> >
>> > Thanks for pointing this out, This is definitely not intended and
>> happening
>> > because of the side effect.
>> > But I have been thinking about this, and couldn't think of any cleaner
>> > solution to solve this duplicate notice message. Although I think this
>> is a
>> > very rare scenario, especially in a production environment to have some
>> > configurations where PG backend is forwarding the normal log messages to
>> > front-end client.
>> > Also, if we have to decide between the current state (producing a kind
>> > mismatch error) vs this rare occurrence of duplicate LOG on the client
>> > side, I think we should choose this duplicate log as it is less harmful.
>> > But Offcource the best thing would be to solve this in a clean and
>> > efficient way.
>>
>> Yeah, I couldn't think of any better solution neither. It's apparent
>> that solving the existing kind mismatch error problem is very
>> important. I agree that we should choose living with duplicate log
>> messages in rare cases. Let's leave your patch as it is and
>> commit/push it.
>>
>> Best regards,
>> --
>> Tatsuo Ishii
>> SRA OSS, Inc. Japan
>> English: http://www.sraoss.co.jp/index_en.php
>> Japanese:http://www.sraoss.co.jp
>>