Re: HttpProducer endpoint with "unlimited/limited" redelivery - memory leak

2014-12-09 Thread Claus Ibsen
Hi

Thanks for reporting. Willem logged and fixed this
https://issues.apache.org/jira/browse/CAMEL-8134

On Mon, Dec 8, 2014 at 11:53 AM, Radek Kraus  wrote:
> Hello,
> I have a suspicion on (or better I faced to) memory leak in the following
> situation. I use HttpProducer endpoint with "unlimited" redelivery
> (configuration problem), but the problem is present for "limited" redelivery
> too. Here is simplified route, which I used for problem simulation (finally
> I got OOM in this simulation):
>
> return new RouteBuilder() {
>   public void configure() {
> errorHandler(
>
> defaultErrorHandler().maximumRedeliveries(-1).redeliveryDelay(5).maximumRedeliveryDelay(5)
> );
> from("direct:start").setHeader(Exchange.HTTP_METHOD,
> POST).to("http://localhost:8899/test";);
>   }
> };
>
> The problem occurs, when HTTP server returns 404 as response code (for
> example when the target "web application" is undeployed from web application
> container). In this situation the next/new "synchronization" callback is
> registered in each HttpProducer endpoint processing, which caused that we
> have a lot of "close" synchronization actions registered on UnitOfWork in
> some time (OOM problem in extreme situation) - see
> org.apache.camel.converter.stream.CachedOutputStream(Exchange exchange,
> final boolean closedOnCompletion, which is invoked from HttpProducer, method
> doExtractResponseBodyAsStream(...))
>
> Maybe the problem relates to CAMEL-7055, where the synchronization callback
> is registered even if closedOnCompletion parameter is false.
>
> I see, that "stream close synchronization" is necessary, mainly for
> situation, where we got the valid body response, but maybe for
> "populateHttpOperationFailedException" isn't necessary to register the
> synchronization action at all, because the response body is only converted
> into java.lang.String.
>
> I would like to ask, if my solution with using redelivery, is based on wrong
> idea or I discovered a bug, which should be reported to JIRA.
>
> Thank you for any feedback.
> Radek Kraus.
>
>
>
> --
> View this message in context: 
> http://camel.465427.n5.nabble.com/HttpProducer-endpoint-with-unlimited-limited-redelivery-memory-leak-tp5760285.html
> Sent from the Camel - Users mailing list archive at Nabble.com.



-- 
Claus Ibsen
-
Red Hat, Inc.
Email: cib...@redhat.com
Twitter: davsclaus
Blog: http://davsclaus.com
Author of Camel in Action: http://www.manning.com/ibsen
hawtio: http://hawt.io/
fabric8: http://fabric8.io/


Re: HttpProducer endpoint with "unlimited/limited" redelivery - memory leak

2014-12-09 Thread Willem Jiang
Hi Radek,

I revisit the code twice and found out we should not add the synchronisation if 
the closedOnCompletion option is false. As it’s the user responsibility to 
clean up the stream once it close the stream. In you case the exchange 
synchronisation is keeping adding even the under layer input stream is consumed.

I created a JIRA[1] for it and committed a quick patch for it. 

[1]https://issues.apache.org/jira/browse/CAMEL-8134 

--  
Willem Jiang

Red Hat, Inc.
Web: http://www.redhat.com
Blog: http://willemjiang.blogspot.com (English)
http://jnn.iteye.com (Chinese)
Twitter: willemjiang  
Weibo: 姜宁willem



On December 8, 2014 at 6:55:17 PM, Radek Kraus (radek_kr...@centrum.cz) wrote:
> Hello,
> I have a suspicion on (or better I faced to) memory leak in the following
> situation. I use HttpProducer endpoint with "unlimited" redelivery
> (configuration problem), but the problem is present for "limited" redelivery
> too. Here is simplified route, which I used for problem simulation (finally
> I got OOM in this simulation):
>  
> return new RouteBuilder() {
> public void configure() {
> errorHandler(
>  
> defaultErrorHandler().maximumRedeliveries(-1).redeliveryDelay(5).maximumRedeliveryDelay(5)
>   
> );
> from("direct:start").setHeader(Exchange.HTTP_METHOD,
> POST).to("http://localhost:8899/test";);
> }
> };
>  
> The problem occurs, when HTTP server returns 404 as response code (for
> example when the target "web application" is undeployed from web application
> container). In this situation the next/new "synchronization" callback is
> registered in each HttpProducer endpoint processing, which caused that we
> have a lot of "close" synchronization actions registered on UnitOfWork in
> some time (OOM problem in extreme situation) - see
> org.apache.camel.converter.stream.CachedOutputStream(Exchange exchange,
> final boolean closedOnCompletion, which is invoked from HttpProducer, method
> doExtractResponseBodyAsStream(...))
>  
> Maybe the problem relates to CAMEL-7055, where the synchronization callback
> is registered even if closedOnCompletion parameter is false.
>  
> I see, that "stream close synchronization" is necessary, mainly for
> situation, where we got the valid body response, but maybe for
> "populateHttpOperationFailedException" isn't necessary to register the
> synchronization action at all, because the response body is only converted
> into java.lang.String.
>  
> I would like to ask, if my solution with using redelivery, is based on wrong
> idea or I discovered a bug, which should be reported to JIRA.
>  
> Thank you for any feedback.
> Radek Kraus.
>  
>  
>  
> --
> View this message in context: 
> http://camel.465427.n5.nabble.com/HttpProducer-endpoint-with-unlimited-limited-redelivery-memory-leak-tp5760285.html
>   
> Sent from the Camel - Users mailing list archive at Nabble.com.
>  



Re: HttpProducer endpoint with "unlimited/limited" redelivery - memory leak

2014-12-09 Thread Radek Kraus
Hi Willem and Claus,
many thank you for fix and JIRA issue too. 

I felt that one possibility, was to don't register synchronization at all
(like was in version 2.12.2), but I wasn't sure with this option (mainly in
context of CAMEL-7055).



--
View this message in context: 
http://camel.465427.n5.nabble.com/HttpProducer-endpoint-with-unlimited-limited-redelivery-memory-leak-tp5760285p5760424.html
Sent from the Camel - Users mailing list archive at Nabble.com.


Re: HttpProducer endpoint with "unlimited/limited" redelivery - memory leak

2014-12-09 Thread Willem Jiang
Hi Radek,

It’s more complex than you thought. We need to clean up the cached the file in 
some place.
I just added some unit tests to make sure we don’t introduce regress issue 
again.
You can check out the patch if you are interesting about the whole context :)

--  
Willem Jiang

Red Hat, Inc.
Web: http://www.redhat.com
Blog: http://willemjiang.blogspot.com (English)
http://jnn.iteye.com (Chinese)
Twitter: willemjiang  
Weibo: 姜宁willem



On December 10, 2014 at 3:29:57 AM, Radek Kraus (radek_kr...@centrum.cz) wrote:
> Hi Willem and Claus,
> many thank you for fix and JIRA issue too.
>  
> I felt that one possibility, was to don't register synchronization at all
> (like was in version 2.12.2), but I wasn't sure with this option (mainly in
> context of CAMEL-7055).
>  
>  
>  
> --
> View this message in context: 
> http://camel.465427.n5.nabble.com/HttpProducer-endpoint-with-unlimited-limited-redelivery-memory-leak-tp5760285p5760424.html
>   
> Sent from the Camel - Users mailing list archive at Nabble.com.
>