[pgpool-hackers: 1072] Re: [pgpool-committers: 2702] pgpool: Mega patch to enhance performance in extended protocol mode.

[Yugo Nagata]

Ishii-san

Thank you for reply. I understand that we don't have to be worry
about memory leak.

BTW, I found other questions about the commit.

(1)
> - New data structure called "pending message queue" is created to
>   manage which response of particular message is pending.  When
>   Parse/Bind/Close message are received, each message is en-queued.
>   The information is used to process those response messages, when
>   Parse complete/Bind completes and Close compete message are received
>   because they don't have any information regarding statement/portal.

In the codes, this structure is used only for Close message
(in Close() and CloseComplete()). This is enough and the 
structure is not necessary for Parse/Bind?

(2)
In CloseComplete() function, the condition for removing message is
"kind == ' '". Is should be "kind != ' '" ?

pool_remove_sent_message(' ', name) looks odd to me.

1865     if (kind == ' ')
1866     {
1867         pool_remove_sent_message(kind, name);
1868         ereport(DEBUG1,
1869                 (errmsg("CloseComplete: remove uncompleted message. kind:%c, name:%s",
1870                         kind, name)));
1871     }



On Wed, 30 Sep 2015 14:04:10 +0900 (JST)
Tatsuo Ishii <ishii at postgresql.org> wrote:

> [remving Cc: to pgpool-committers to keep the list clean]
> 
> > Ishii-san
> > 
> > Looking at the commit, I found that pool_sent_message_destory()
> > isn't called in pool_remove_sent_message() which is commented
> > out with XXX. Is it needed? I think this causes memory leak.
> > 
> > @@ -304,7 +317,8 @@ bool pool_remove_sent_message(char kind, const char *name)
> >                 if (msglist->sent_messages[i]->kind == kind &&
> >                         !strcmp(msglist->sent_messages[i]->name, name))
> >                 {
> > -                       pool_sent_message_destroy(msglist->sent_messages[i]);
> > +                       //XXX
> > +                       //pool_sent_message_destroy(msglist->sent_messages[i]);
> >                         break;
> >                 }
> >         }
> 
> Because the commit removes most flush messages, it is possible that
> pending messages remain (not yet receiving response message from
> backend).
> 
> Also, the memory for sent_message is in the session context, when a
> session ends, it will be freed anyway.
> 
> However the code is apparently a temporary one, I should have cleaned
> earlier. Will work on it.
> 
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp


-- 
Yugo Nagata <nagata at sraoss.co.jp>