axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dobri Kitipov <kdobrik.ax...@googlemail.com>
Subject Re: HTTP connection leak and other related issues
Date Wed, 25 Mar 2009 10:11:03 GMT
Hi everybody,
I am not sure that the default ConnectionManager should be changed from
MultiThreadedHttpConnectionManager to SimpleHttpConnectionManager. What
about asynchronous MEPs? I have not tested this, but I suppose that if a
given async client does several invocations then if *NO*
MultiThreadedHttpConnectionManager is in use (i.e. there is no connection
pool) then the NON-blocking mechanism will be corrupted?

Dobri

On Tue, Mar 24, 2009 at 6:40 PM, Alexis Midon <midon@intalio.com> wrote:

> +1 on this. I'm fine with the current behavior regarding HttpClient
> management.
> My only request would be to not instanciate a
> MultiThreadedHttpConnectionManager in the default case. A
> SimpleHttpConnectionManager would be good enough. I agree it's close to be a
> detail but still.
>
> Alexis
>
>
>
> On Mon, Mar 23, 2009 at 9:20 PM, Amila Suriarachchi <
> amilasuriarachchi@gmail.com> wrote:
>
>>
>>
>> On Tue, Mar 24, 2009 at 4:16 AM, Alexis Midon <midon@intalio.com> wrote:
>>
>>>
>>> It's already possible to reuse the HttpClient and with your own
>>> HttpConnectionManager. What you have to do is create the HttpClient by
>>> yourself with your connection manager and push it in the MessageContext.
>>
>>
>> I think this is a good point.
>> So if we set both HTTPConstants.REUSE_HTTP_CLIENT,
>> HTTPConstants.CACHED_HTTP_CLIENT then ultimately transport sender use that
>> client.
>> This means users can manage the way they need to use the httpClient at
>> application level. either they can use one httpClient for all the messages
>> or kept the HttpState at thread local.
>> So IMHO since this is a performance fine tuning,  it is ok to keep it in
>> this way and let users to mange it in the way they need at application
>> level.
>>
>> thanks,
>> Amila.
>>
>>>
>>>
>>> Alexis
>>>
>>>
>>>
>>> On Sun, Mar 22, 2009 at 3:52 AM, Amila Suriarachchi <
>>> amilasuriarachchi@gmail.com> wrote:
>>>
>>>> hi all,
>>>>
>>>> recently I came across this link and according to this[1] creating new
>>>> http client for every request is not good
>>>> in performance wise.
>>>>
>>>> Current get Http client method looks like this
>>>>
>>>> protected HttpClient getHttpClient(MessageContext msgContext) {
>>>>         HttpClient httpClient;
>>>>         Object reuse =
>>>> msgContext.getOptions().getProperty(HTTPConstants.REUSE_HTTP_CLIENT);
>>>>         if (reuse == null) {
>>>>             reuse =
>>>> msgContext.getConfigurationContext().getProperty(HTTPConstants.REUSE_HTTP_CLIENT);
>>>>         }
>>>>         if (reuse != null && JavaUtils.isTrueExplicitly(reuse)) {
>>>>             httpClient = (HttpClient)
>>>> msgContext.getOptions().getProperty(HTTPConstants.CACHED_HTTP_CLIENT);
>>>>             if (httpClient == null) {
>>>>                 httpClient = (HttpClient)
>>>> msgContext.getConfigurationContext()
>>>>                         .getProperty(HTTPConstants.CACHED_HTTP_CLIENT);
>>>>             }
>>>>             if (httpClient != null)
>>>>                 return httpClient;
>>>>             MultiThreadedHttpConnectionManager connectionManager =
>>>>                 new MultiThreadedHttpConnectionManager();
>>>>             httpClient = new HttpClient(connectionManager);
>>>>             msgContext.getConfigurationContext()
>>>>                 .setProperty(HTTPConstants.CACHED_HTTP_CLIENT,
>>>> httpClient);
>>>>         } else {
>>>>             HttpConnectionManager connManager =
>>>>                     (HttpConnectionManager) msgContext.getProperty(
>>>>
>>>> HTTPConstants.MULTITHREAD_HTTP_CONNECTION_MANAGER);
>>>>             if (connManager == null) {
>>>>                 connManager =
>>>>                         (HttpConnectionManager) msgContext.getProperty(
>>>>
>>>> HTTPConstants.MUTTITHREAD_HTTP_CONNECTION_MANAGER);
>>>>             }
>>>>             if(connManager != null){
>>>>                 httpClient = new HttpClient(connManager);
>>>>             } else {
>>>>                 //Multi threaded http connection manager has set as the
>>>> default
>>>>                 connManager = new MultiThreadedHttpConnectionManager();
>>>>                 httpClient = new HttpClient(connManager);
>>>>             }
>>>>         }
>>>>
>>>>         // Get the timeout values set in the runtime
>>>>         initializeTimeouts(msgContext, httpClient);
>>>>         return httpClient;
>>>>     }
>>>>
>>>> According to the above article creating many http client instances are
>>>> not efficient. But if a user switch on the
>>>> HTTPConstants.REUSE_HTTP_CLIENT then he can not give an
>>>> MultiThreadedHttpConnectionManager object where users can define maximum
>>>> pool size, host configurations etc...
>>>>
>>>> Therefore I think it is better to have these features,
>>>>
>>>> 1. Let users to create the MultiThreadedHttpConnectionManager with its
>>>> own even when reusing http client.
>>>> 2. Does Http Client thread safe? if not we need to keep the cache copy
>>>> of the httpClient in a thread local variable.
>>>>
>>>> Does any one knows how to switch on the keep alive and will it make any
>>>> performance improvement?
>>>>
>>>> Anyway I still believe releasing connections  should be done at the user
>>>> level since it degradates the performance.
>>>>
>>>> thanks,
>>>> Amila.
>>>>
>>>>
>>>> [1] http://hc.apache.org/httpclient-3.x/performance.html
>>>>
>>>>
>>>> On Tue, Mar 10, 2009 at 2:26 AM, Andreas Veithen <
>>>> andreas.veithen@gmail.com> wrote:
>>>>
>>>>> I agree that we also need to consider OperationClient and the
>>>>> non-blocking methods. I think that the first step towards a solid
>>>>> solution is actually to write a couple of test cases that provide
>>>>> evidence for these issues and that we can later use for regression
>>>>> testing, but I will have to review the existing unit tests we have.
>>>>>
>>>>> Andreas
>>>>>
>>>>> On Wed, Mar 4, 2009 at 01:21, Alexis Midon <midon@intalio.com>
wrote:
>>>>> > Another case where "Options#setCallTransportCleanup for
>>>>> OperationClient" is
>>>>> > obvious is when you call OperationClient#execute in a non-blocking
>>>>> way.
>>>>> > The caller cannot clean up the transport safely, because the
>>>>> execution might
>>>>> > still be in progress. In that case it's OperationClient
>>>>> responsability to
>>>>> > clean up the transport.
>>>>> >
>>>>> > Alexis
>>>>> >
>>>>> >
>>>>> > On Mon, Mar 2, 2009 at 10:11 AM, Alexis Midon <midon@intalio.com>
>>>>> wrote:
>>>>> >>
>>>>> >> There is still some inconsistencies between how
>>>>> ServiceClient#sendReceive
>>>>> >> and Operation#execute use Options#getCallTransportCleanup.
>>>>> >>
>>>>> >> And I think that would help a lot if the related jira issues
get
>>>>> cleaned
>>>>> >> up a little bit.
>>>>> >>
>>>>> >> Thanks for your time and feedback.
>>>>> >>
>>>>> >> Alexis
>>>>> >>
>>>>> >>
>>>>> >> On Sun, Mar 1, 2009 at 9:02 PM, Amila Suriarachchi
>>>>> >> <amilasuriarachchi@gmail.com> wrote:
>>>>> >>>
>>>>> >>> hi all,
>>>>> >>>
>>>>> >>> On Fri, Feb 27, 2009 at 6:35 PM, Andreas Veithen
>>>>> >>> <andreas.veithen@gmail.com> wrote:
>>>>> >>>>
>>>>> >>>> I think that callTransportCleanup should never be turned
on by
>>>>> default
>>>>> >>>> because it would disable deferred parsing of the response.
What
>>>>> needs
>>>>> >>>> to be done urgently is to improve the documentation
of the
>>>>> >>>> ServiceClient class to make it clear that it is mandatory
to
>>>>> either
>>>>> >>>> call cleanupTransport explicitly or to set callTransportCleanup
to
>>>>> >>>> true. Also the cleanupTransport method itself doesn't
have any
>>>>> >>>> Javadoc.
>>>>> >>>
>>>>> >>> thanks for in-depth analysing  of this issue. If the issue
is not
>>>>> calling
>>>>> >>> to transport clean up then I clearly agree with what Andreas.
>>>>> >>>
>>>>> >>> Axis2 uses deffered building. There when the user gets the
response
>>>>> >>> OMElement it has not build and Axis2 does not know when
he going to
>>>>> finish
>>>>> >>> with it. So it is responsibility of the user to call the
transport
>>>>> clean up
>>>>> >>> once they have done with the OMElement.
>>>>> >>>
>>>>> >>> But this problem does not occur with the generated data
bind code.
>>>>> There
>>>>> >>> before returning from the stub method it generates the all
the
>>>>> databinding
>>>>> >>> classes. In the generated code finally method calls
>>>>> >>>
>>>>> _messageContext.getTransportOut().getSender().cleanup(_messageContext);
>>>>> >>>
>>>>> >>> to clean up the transport.
>>>>> >>>
>>>>> >>> thanks,
>>>>> >>> Amila.
>>>>> >>>>
>>>>> >>>> Andreas
>>>>> >>>>
>>>>> >>>> On Fri, Feb 27, 2009 at 12:19, Dobri Kitipov
>>>>> >>>> <kdobrik.axis2@googlemail.com> wrote:
>>>>> >>>> > Hi Andreas,
>>>>> >>>> > thank you for the comment. I think you get the
question.
>>>>> >>>> > Quick test shows that setting the following line
of code into
>>>>> the
>>>>> >>>> > client:
>>>>> >>>> >
>>>>> >>>> > options.setCallTransportCleanup(true);
>>>>> >>>> >
>>>>> >>>> > forces the closure of the http connection. It seems
it is not
>>>>> the
>>>>> >>>> > default
>>>>> >>>> > behavior. This is a good and fast solution. I was
a little bit
>>>>> more
>>>>> >>>> > focused
>>>>> >>>> > on wondering why I have such a difference b/n using
SOAP and
>>>>> MIME
>>>>> >>>> > builder.
>>>>> >>>> >
>>>>> >>>> > I need to think about some use cases when we need
to have
>>>>> >>>> > options.setCallTransportCleanup(false). Can we
have this by
>>>>> default in
>>>>> >>>> > some
>>>>> >>>> > cases?
>>>>> >>>> >
>>>>> >>>> > Anyway, it will be worth having a further analysis
of the issue
>>>>> we
>>>>> >>>> > have with
>>>>> >>>> > SOAPBuilder behavior.
>>>>> >>>> >
>>>>> >>>> > Thank you,
>>>>> >>>> > Dobri
>>>>> >>>> >
>>>>> >>>> >
>>>>> >>>> > On Fri, Feb 27, 2009 at 12:46 PM, Andreas Veithen
>>>>> >>>> > <andreas.veithen@gmail.com> wrote:
>>>>> >>>> >>
>>>>> >>>> >> If I understand correctly, Dobri's findings
can be summarized
>>>>> as
>>>>> >>>> >> follows:
>>>>> >>>> >> 1. Once the InputStream is consumed, commons-httpclient
>>>>> automatically
>>>>> >>>> >> releases the connection.
>>>>> >>>> >> 2. SOAPBuilder never completely consumes the
InputStream.
>>>>> >>>> >>
>>>>> >>>> >> The SOAPBuilder behavior is indeed somewhat
questionable, but
>>>>> it is
>>>>> >>>> >> important to understand that because of the
deferred parsing
>>>>> model
>>>>> >>>> >> used by Axiom, there is never a guarantee that
the InputStream
>>>>> will
>>>>> >>>> >> be
>>>>> >>>> >> completely consumed. Normally releasing the
connection is the
>>>>> >>>> >> responsibility of the CommonsHTTPTransportSender#cleanup
method
>>>>> which
>>>>> >>>> >> should be called by ServiceClient#cleanupTransport.
It would be
>>>>> >>>> >> interesting to know if that method is called,
and if it is, why
>>>>> it
>>>>> >>>> >> fails to release the connection.
>>>>> >>>> >>
>>>>> >>>> >> Andreas
>>>>> >>>> >>
>>>>> >>>> >> On Fri, Feb 27, 2009 at 10:10, Dobri Kitipov
>>>>> >>>> >> <kdobrik.axis2@googlemail.com> wrote:
>>>>> >>>> >> > Hi all,
>>>>> >>>> >> > I have observed absolutely the same thing
these days. I need
>>>>> some
>>>>> >>>> >> > more
>>>>> >>>> >> > time
>>>>> >>>> >> > to analyze the whole picture, but here
is my current
>>>>> synthesis of
>>>>> >>>> >> > the
>>>>> >>>> >> > issue.
>>>>> >>>> >> >
>>>>> >>>> >> > It seems that http connection release
is tightly coupled
>>>>> with the
>>>>> >>>> >> > Axis2
>>>>> >>>> >> > builder used to handle and process the
response body and the
>>>>> >>>> >> > corresponding
>>>>> >>>> >> > input streams used. The builder and the
InputStream used are
>>>>> based
>>>>> >>>> >> > on
>>>>> >>>> >> > the
>>>>> >>>> >> > HTTP headers fields like "Content-Type"
and
>>>>> "Transfer-Encoding"
>>>>> >>>> >> > (e.g.
>>>>> >>>> >> > Content-Type: text/xml; charset=UTF-8
 Transfer-Encoding:
>>>>> chunked).
>>>>> >>>> >> > So
>>>>> >>>> >> > if we
>>>>> >>>> >> > have Content-Type: text/xml; then SOAPBuilder
class will be
>>>>> used.
>>>>> >>>> >> > If we
>>>>> >>>> >> > have  type="application/xop+xml" then
MIMEBuilder will be
>>>>> used.
>>>>> >>>> >> >
>>>>> >>>> >> > The successfull story when we have MIMIBuilder:
>>>>> >>>> >> >
>>>>> >>>> >> > When MIMEBuilder is used then the response
Buffered
>>>>> InputStream
>>>>> >>>> >> > (IS) is
>>>>> >>>> >> > wrrapped (I will use "->" sign as substitution
for wrrapped)
>>>>> >>>> >> > ChunkedIS
>>>>> >>>> >> > ->
>>>>> >>>> >> > AutoCloseIS (this has a responseConsumed()
method notified
>>>>> when
>>>>> >>>> >> > IS.read()
>>>>> >>>> >> > returns -1, which means that the response
IS has been
>>>>> completely
>>>>> >>>> >> > read)
>>>>> >>>> >> > ->
>>>>> >>>> >> > PushBackIS ->BounderyPushBackIS. The
BounderyPushBackIS reads
>>>>> the
>>>>> >>>> >> > response
>>>>> >>>> >> > stream (see readFromStream(....)) in a
cycle till it reaches
>>>>> its
>>>>> >>>> >> > end. At
>>>>> >>>> >> > every iteration of this cycle a AutoCloseIS
checkClose(l) is
>>>>> >>>> >> > invoked. So
>>>>> >>>> >> > when the end is reached (-1 is returned)
then this check
>>>>> causes the
>>>>> >>>> >> > invokation of the AutoCloseIS checkClose(...)
 method. This
>>>>> method
>>>>> >>>> >> > invokes
>>>>> >>>> >> > notifyWatcher() that in turn invokes responseBodyConsumed()
>>>>> method
>>>>> >>>> >> > of
>>>>> >>>> >> > the
>>>>> >>>> >> > HttpMethodBase class. This causes the
release of the http
>>>>> >>>> >> > connection
>>>>> >>>> >> > which
>>>>> >>>> >> > is returned back to the connection pool.
So here we have no
>>>>> problem
>>>>> >>>> >> > with
>>>>> >>>> >> > connection reuse.
>>>>> >>>> >> >
>>>>> >>>> >> > The bad story we have with SOAPBuilder:
>>>>> >>>> >> >
>>>>> >>>> >> > When SOAPBuilder is used then the response
Buffered
>>>>> InputStream is
>>>>> >>>> >> > wrrapped
>>>>> >>>> >> > in a ChunkedIS -> AutoCloseIS ->
PushBackIS. May be you has
>>>>> noticed
>>>>> >>>> >> > that
>>>>> >>>> >> > BounderyPushBackIS is not used. As a result
the respose IS is
>>>>> not
>>>>> >>>> >> > completely
>>>>> >>>> >> > read (in fact this is not really correct,
it could be read
>>>>> but the
>>>>> >>>> >> > invokation of the PushBackIS unread(...)
method causes the
>>>>> >>>> >> > AutoCloseIS
>>>>> >>>> >> > checkClose() method never to return -1).
As a result the http
>>>>> >>>> >> > connection
>>>>> >>>> >> > is
>>>>> >>>> >> > not released. And since there is a limit
to have 2 connection
>>>>> per
>>>>> >>>> >> > host
>>>>> >>>> >> > then after the second invokation of the
WS client the thread
>>>>> hangs
>>>>> >>>> >> > waiting
>>>>> >>>> >> > for a free connection.
>>>>> >>>> >> >
>>>>> >>>> >> > Please, provide us with your comments
on this issue.
>>>>> >>>> >> >
>>>>> >>>> >> > Thank you in advance.
>>>>> >>>> >> >
>>>>> >>>> >> > Regards,
>>>>> >>>> >> > Dobri
>>>>> >>>> >> >
>>>>> >>>> >> > On Fri, Feb 27, 2009 at 3:54 AM, Alexis
Midon <
>>>>> midon@intalio.com>
>>>>> >>>> >> > wrote:
>>>>> >>>> >> >>
>>>>> >>>> >> >> no taker for an easy patch?
>>>>> >>>> >> >>
>>>>> >>>> >> >> Alexis
>>>>> >>>> >> >>
>>>>> >>>> >> >>
>>>>> >>>> >> >> On Wed, Feb 25, 2009 at 6:45 PM, Alexis
Midon <
>>>>> midon@intalio.com>
>>>>> >>>> >> >> wrote:
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> Hi everyone,
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> All the issues relatives to AXIS2-935
are really messy,
>>>>> some of
>>>>> >>>> >> >>> them
>>>>> >>>> >> >>> are
>>>>> >>>> >> >>> closed but their clones are not.
Some are flagged as fixed
>>>>> but
>>>>> >>>> >> >>> are
>>>>> >>>> >> >>> obviously
>>>>> >>>> >> >>> not. All these issues are really
old, so I'd like to take a
>>>>> >>>> >> >>> chance to
>>>>> >>>> >> >>> bring
>>>>> >>>> >> >>> them back to your attention, especially
before releasing
>>>>> 1.5.
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> I'll post a description of the
issue in this email as a
>>>>> summary
>>>>> >>>> >> >>> all
>>>>> >>>> >> >>> the
>>>>> >>>> >> >>> jiras.
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> By default, ServiceClient uses
one HttpConnectionManager
>>>>> per
>>>>> >>>> >> >>> invocation
>>>>> >>>> >> >>> [2]. This connection manager will
create and provide one
>>>>> >>>> >> >>> connection to
>>>>> >>>> >> >>> HTTPSender. The first issue is
that by default this
>>>>> connection is
>>>>> >>>> >> >>> never
>>>>> >>>> >> >>> released to the pool [3]. if you
do zillions of
>>>>> invocations, this
>>>>> >>>> >> >>> leak
>>>>> >>>> >> >>> will
>>>>> >>>> >> >>> max out your number of file descriptors.
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> Your investigations in Axis2 options
quickly lead you to
>>>>> the
>>>>> >>>> >> >>> REUSE_HTTP_CLIENT option. But
this first issue has some
>>>>> >>>> >> >>> unfortunate
>>>>> >>>> >> >>> consequences if you activate it.
Actually if you do so, a
>>>>> single
>>>>> >>>> >> >>> connection
>>>>> >>>> >> >>> manager is shared across all invocations.
But because
>>>>> connections
>>>>> >>>> >> >>> are
>>>>> >>>> >> >>> not
>>>>> >>>> >> >>> release, the pool is starved after
two invocations, and the
>>>>> third
>>>>> >>>> >> >>> invocation
>>>>> >>>> >> >>> hangs out indefinitely. :(
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> If you keep digging you will find
the
>>>>> AUTO_RELEASE_CONNECTION
>>>>> >>>> >> >>> option.
>>>>> >>>> >> >>> Its
>>>>> >>>> >> >>> sounds like a good lead! Let's
try it. If you activate this
>>>>> >>>> >> >>> option the
>>>>> >>>> >> >>> connection is properly released
-Yahoooo! the leak is fixed
>>>>> - but
>>>>> >>>> >> >>> unfortunately a new issue shows
up (issue #2, aka
>>>>> AXIS2-3478).
>>>>> >>>> >> >>> AbstractHTTPSender passes the
stream of the connection to
>>>>> the
>>>>> >>>> >> >>> message
>>>>> >>>> >> >>> context [4] , but that the connection
is now properly
>>>>> released,
>>>>> >>>> >> >>> so
>>>>> >>>> >> >>> this
>>>>> >>>> >> >>> stream is closed before the SOAPBuilder
gets a chance to
>>>>> read the
>>>>> >>>> >> >>> response
>>>>> >>>> >> >>> body.
>>>>> >>>> >> >>> Boom! "IOException: Attempted
read on closed stream"
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> These issues are easily reproducible
in versions 1.3, 1.4,
>>>>> 1.5.
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> I submitted and documented a fix
in AXIS2-2931 [5], if you
>>>>> had a
>>>>> >>>> >> >>> chance
>>>>> >>>> >> >>> to look at it that would be much
appreciate.
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> Alexis
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> >>>> >> >>> [1]
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> https://issues.apache.org/jira/browse/AXIS2-935?focusedCommentId=12513543#action_12513543
>>>>> >>>> >> >>> [2] see method getHttpClient in
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> https://svn.apache.org/repos/asf/webservices/commons/trunk/modules/transport/modules/http/src/org/apache/axis2/transport/http/AbstractHTTPSender.java
>>>>> >>>> >> >>> [3] see method cleanup in
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> https://svn.apache.org/repos/asf/webservices/commons/trunk/modules/transport/modules/http/src/org/apache/axis2/transport/http/HTTPSender.java
>>>>> >>>> >> >>> [4] see method processResponse
in AbstractHTTPSender.java
>>>>> >>>> >> >>> [5]
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> >>>> >> >>>
>>>>> https://issues.apache.org/jira/browse/AXIS2-2931?focusedCommentId=12676837#action_12676837
>>>>> >>>> >> >>
>>>>> >>>> >> >
>>>>> >>>> >> >
>>>>> >>>> >
>>>>> >>>> >
>>>>> >>>
>>>>> >>>
>>>>> >>>
>>>>> >>> --
>>>>> >>> Amila Suriarachchi
>>>>> >>> WSO2 Inc.
>>>>> >>> blog: http://amilachinthaka.blogspot.com/
>>>>> >>
>>>>> >
>>>>> >
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Amila Suriarachchi
>>>> WSO2 Inc.
>>>> blog: http://amilachinthaka.blogspot.com/
>>>>
>>>
>>>
>>
>>
>> --
>> Amila Suriarachchi
>> WSO2 Inc.
>> blog: http://amilachinthaka.blogspot.com/
>>
>
>

Mime
View raw message