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 <mi...@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 < > amilasuriarach...@gmail.com> wrote: > >> >> >> On Tue, Mar 24, 2009 at 4:16 AM, Alexis Midon <mi...@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 < >>> amilasuriarach...@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.veit...@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 <mi...@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 <mi...@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 >>>>> >> <amilasuriarach...@gmail.com> wrote: >>>>> >>> >>>>> >>> hi all, >>>>> >>> >>>>> >>> On Fri, Feb 27, 2009 at 6:35 PM, Andreas Veithen >>>>> >>> <andreas.veit...@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.ax...@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.veit...@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.ax...@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 < >>>>> mi...@intalio.com> >>>>> >>>> >> > wrote: >>>>> >>>> >> >> >>>>> >>>> >> >> no taker for an easy patch? >>>>> >>>> >> >> >>>>> >>>> >> >> Alexis >>>>> >>>> >> >> >>>>> >>>> >> >> >>>>> >>>> >> >> On Wed, Feb 25, 2009 at 6:45 PM, Alexis Midon < >>>>> mi...@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/ >> > >