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

Yugo Nagata nagata at sraoss.co.jp
Tue Oct 13 17:47:32 JST 2015


Ishii-san

Have you read my mail below?

On Wed, 30 Sep 2015 18:21:44 +0900
Yugo Nagata <nagata at sraoss.co.jp> wrote:

> 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>
> _______________________________________________
> pgpool-hackers mailing list
> pgpool-hackers at pgpool.net
> http://www.pgpool.net/mailman/listinfo/pgpool-hackers


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


More information about the pgpool-hackers mailing list