You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by Claus Ibsen <cl...@gmail.com> on 2013/11/09 15:50:59 UTC
Re: git commit: CAMEL-6948: Releasing a non-singleton producer by
ProducerCache should not only stop the producer but also shutdown it as well
if it is a ShutdownableService.
Hi
There is a stopAndShutdownService method on ServiceHelper in the util
package. Its maybe nicer to use to than the type cast. Then you can do
that in one command.
And this is also how we stop/start/shutdown services in a cleaner way.
And if you enabled TRACE/DEBUG logging the service helper can log what
happens as well.
On Fri, Nov 8, 2013 at 9:46 PM, <bv...@apache.org> wrote:
> Updated Branches:
> refs/heads/master f74ff2cf6 -> f744afd9c
>
>
> CAMEL-6948: Releasing a non-singleton producer by ProducerCache should not only stop the producer but also shutdown it as well if it is a ShutdownableService.
>
> Project: http://git-wip-us.apache.org/repos/asf/camel/repo
> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/f744afd9
> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/f744afd9
> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/f744afd9
>
> Branch: refs/heads/master
> Commit: f744afd9c39a1fc9261ad68dabe06a51a238fac9
> Parents: f74ff2c
> Author: Babak Vahdat <bv...@apache.org>
> Authored: Fri Nov 8 21:45:59 2013 +0100
> Committer: Babak Vahdat <bv...@apache.org>
> Committed: Fri Nov 8 21:45:59 2013 +0100
>
> ----------------------------------------------------------------------
> .../org/apache/camel/impl/ProducerCache.java | 6 +++
> .../camel/impl/DefaultProducerCacheTest.java | 53 +++++++++++++++++---
> 2 files changed, 51 insertions(+), 8 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/camel/blob/f744afd9/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
> ----------------------------------------------------------------------
> diff --git a/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java b/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
> index 6b292c0..b35eca5 100644
> --- a/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
> +++ b/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
> @@ -30,6 +30,7 @@ import org.apache.camel.Processor;
> import org.apache.camel.Producer;
> import org.apache.camel.ProducerCallback;
> import org.apache.camel.ServicePoolAware;
> +import org.apache.camel.ShutdownableService;
> import org.apache.camel.processor.UnitOfWorkProducer;
> import org.apache.camel.spi.ServicePool;
> import org.apache.camel.support.ServiceSupport;
> @@ -137,6 +138,11 @@ public class ProducerCache extends ServiceSupport {
> } else if (!producer.isSingleton()) {
> // stop non singleton producers as we should not leak resources
> producer.stop();
> +
> + // shutdown as well in case the producer is shutdownable
> + if (producer instanceof ShutdownableService) {
> + ShutdownableService.class.cast(producer).shutdown();
> + }
> }
> }
>
>
> http://git-wip-us.apache.org/repos/asf/camel/blob/f744afd9/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
> ----------------------------------------------------------------------
> diff --git a/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java b/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
> index 5ad0f42..803302c 100644
> --- a/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
> +++ b/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
> @@ -30,7 +30,8 @@ import org.apache.camel.Producer;
> */
> public class DefaultProducerCacheTest extends ContextTestSupport {
>
> - private static final AtomicInteger COUNTER = new AtomicInteger();
> + private final AtomicInteger stopCounter = new AtomicInteger();
> + private final AtomicInteger shutdownCounter = new AtomicInteger();
>
> public void testCacheProducerAcquireAndRelease() throws Exception {
> ProducerCache cache = new ProducerCache(this, context);
> @@ -56,7 +57,7 @@ public class DefaultProducerCacheTest extends ContextTestSupport {
> assertEquals("Size should be 0", 0, cache.size());
>
> for (int i = 0; i < 8; i++) {
> - Endpoint e = new MyEndpoint(i);
> + Endpoint e = new MyEndpoint(true, i);
> Producer p = cache.acquireProducer(e);
> cache.releaseProducer(e, p);
> }
> @@ -64,19 +65,50 @@ public class DefaultProducerCacheTest extends ContextTestSupport {
> assertEquals("Size should be 5", 5, cache.size());
>
> // should have stopped the 3 evicted
> - assertEquals(3, COUNTER.get());
> + assertEquals(3, stopCounter.get());
>
> cache.stop();
>
> // should have stopped all 8
> - assertEquals(8, COUNTER.get());
> + assertEquals(8, stopCounter.get());
> + }
> +
> + public void testReleaseProducerInvokesStopAndShutdownByNonSingeltonProducers() throws Exception {
> + ProducerCache cache = new ProducerCache(this, context, 1);
> + cache.start();
> +
> + assertEquals("Size should be 0", 0, cache.size());
> +
> + for (int i = 0; i < 3; i++) {
> + Endpoint e = new MyEndpoint(false, i);
> + Producer p = cache.acquireProducer(e);
> + cache.releaseProducer(e, p);
> + }
> +
> + assertEquals("Size should be 0", 0, cache.size());
> +
> + // should have stopped all 3
> + assertEquals(3, stopCounter.get());
> +
> + // should have shutdown all 3
> + assertEquals(3, shutdownCounter.get());
> +
> + cache.stop();
> +
> + // no more stop after stopping the cache
> + assertEquals(3, stopCounter.get());
> +
> + // no more shutdown after stopping the cache
> + assertEquals(3, shutdownCounter.get());
> }
>
> private final class MyEndpoint extends DefaultEndpoint {
>
> - private int number;
> + private final boolean isSingleton;
> + private final int number;
>
> - private MyEndpoint(int number) {
> + private MyEndpoint(boolean isSingleton, int number) {
> + this.isSingleton = isSingleton;
> this.number = number;
> }
>
> @@ -92,7 +124,7 @@ public class DefaultProducerCacheTest extends ContextTestSupport {
>
> @Override
> public boolean isSingleton() {
> - return true;
> + return isSingleton;
> }
>
> @Override
> @@ -114,7 +146,12 @@ public class DefaultProducerCacheTest extends ContextTestSupport {
>
> @Override
> protected void doStop() throws Exception {
> - COUNTER.incrementAndGet();
> + stopCounter.incrementAndGet();
> + }
> +
> + @Override
> + protected void doShutdown() throws Exception {
> + shutdownCounter.incrementAndGet();
> }
> }
>
>
--
Claus Ibsen
-----------------
Red Hat, Inc.
Email: cibsen@redhat.com
Twitter: davsclaus
Blog: http://davsclaus.com
Author of Camel in Action: http://www.manning.com/ibsen
Re: git commit: CAMEL-6948: Releasing a non-singleton producer by
ProducerCache should not only stop the producer but also shutdown it as
well if it is a ShutdownableService.
Posted by Babak Vahdat <ba...@swissonline.ch>.
Hi
I just spotted bunch of flaws in that utility class and logged a ticket for
it:
https://issues.apache.org/jira/browse/CAMEL-6951
Babak
Claus Ibsen-2 wrote
> Hi
>
> There is a stopAndShutdownService method on ServiceHelper in the util
> package. Its maybe nicer to use to than the type cast. Then you can do
> that in one command.
>
> And this is also how we stop/start/shutdown services in a cleaner way.
> And if you enabled TRACE/DEBUG logging the service helper can log what
> happens as well.
>
> On Fri, Nov 8, 2013 at 9:46 PM, <
> bvahdat@
> > wrote:
>> Updated Branches:
>> refs/heads/master f74ff2cf6 -> f744afd9c
>>
>>
>> CAMEL-6948: Releasing a non-singleton producer by ProducerCache should
>> not only stop the producer but also shutdown it as well if it is a
>> ShutdownableService.
>>
>> Project: http://git-wip-us.apache.org/repos/asf/camel/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/f744afd9
>> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/f744afd9
>> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/f744afd9
>>
>> Branch: refs/heads/master
>> Commit: f744afd9c39a1fc9261ad68dabe06a51a238fac9
>> Parents: f74ff2c
>> Author: Babak Vahdat <
> bvahdat@
> >
>> Authored: Fri Nov 8 21:45:59 2013 +0100
>> Committer: Babak Vahdat <
> bvahdat@
> >
>> Committed: Fri Nov 8 21:45:59 2013 +0100
>>
>> ----------------------------------------------------------------------
>> .../org/apache/camel/impl/ProducerCache.java | 6 +++
>> .../camel/impl/DefaultProducerCacheTest.java | 53
>> +++++++++++++++++---
>> 2 files changed, 51 insertions(+), 8 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/camel/blob/f744afd9/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> b/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> index 6b292c0..b35eca5 100644
>> --- a/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> +++ b/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> @@ -30,6 +30,7 @@ import org.apache.camel.Processor;
>> import org.apache.camel.Producer;
>> import org.apache.camel.ProducerCallback;
>> import org.apache.camel.ServicePoolAware;
>> +import org.apache.camel.ShutdownableService;
>> import org.apache.camel.processor.UnitOfWorkProducer;
>> import org.apache.camel.spi.ServicePool;
>> import org.apache.camel.support.ServiceSupport;
>> @@ -137,6 +138,11 @@ public class ProducerCache extends ServiceSupport {
>> } else if (!producer.isSingleton()) {
>> // stop non singleton producers as we should not leak
>> resources
>> producer.stop();
>> +
>> + // shutdown as well in case the producer is shutdownable
>> + if (producer instanceof ShutdownableService) {
>> + ShutdownableService.class.cast(producer).shutdown();
>> + }
>> }
>> }
>>
>>
>> http://git-wip-us.apache.org/repos/asf/camel/blob/f744afd9/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> b/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> index 5ad0f42..803302c 100644
>> ---
>> a/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> +++
>> b/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> @@ -30,7 +30,8 @@ import org.apache.camel.Producer;
>> */
>> public class DefaultProducerCacheTest extends ContextTestSupport {
>>
>> - private static final AtomicInteger COUNTER = new AtomicInteger();
>> + private final AtomicInteger stopCounter = new AtomicInteger();
>> + private final AtomicInteger shutdownCounter = new AtomicInteger();
>>
>> public void testCacheProducerAcquireAndRelease() throws Exception {
>> ProducerCache cache = new ProducerCache(this, context);
>> @@ -56,7 +57,7 @@ public class DefaultProducerCacheTest extends
>> ContextTestSupport {
>> assertEquals("Size should be 0", 0, cache.size());
>>
>> for (int i = 0; i < 8; i++) {
>> - Endpoint e = new MyEndpoint(i);
>> + Endpoint e = new MyEndpoint(true, i);
>> Producer p = cache.acquireProducer(e);
>> cache.releaseProducer(e, p);
>> }
>> @@ -64,19 +65,50 @@ public class DefaultProducerCacheTest extends
>> ContextTestSupport {
>> assertEquals("Size should be 5", 5, cache.size());
>>
>> // should have stopped the 3 evicted
>> - assertEquals(3, COUNTER.get());
>> + assertEquals(3, stopCounter.get());
>>
>> cache.stop();
>>
>> // should have stopped all 8
>> - assertEquals(8, COUNTER.get());
>> + assertEquals(8, stopCounter.get());
>> + }
>> +
>> + public void
>> testReleaseProducerInvokesStopAndShutdownByNonSingeltonProducers() throws
>> Exception {
>> + ProducerCache cache = new ProducerCache(this, context, 1);
>> + cache.start();
>> +
>> + assertEquals("Size should be 0", 0, cache.size());
>> +
>> + for (int i = 0; i < 3; i++) {
>> + Endpoint e = new MyEndpoint(false, i);
>> + Producer p = cache.acquireProducer(e);
>> + cache.releaseProducer(e, p);
>> + }
>> +
>> + assertEquals("Size should be 0", 0, cache.size());
>> +
>> + // should have stopped all 3
>> + assertEquals(3, stopCounter.get());
>> +
>> + // should have shutdown all 3
>> + assertEquals(3, shutdownCounter.get());
>> +
>> + cache.stop();
>> +
>> + // no more stop after stopping the cache
>> + assertEquals(3, stopCounter.get());
>> +
>> + // no more shutdown after stopping the cache
>> + assertEquals(3, shutdownCounter.get());
>> }
>>
>> private final class MyEndpoint extends DefaultEndpoint {
>>
>> - private int number;
>> + private final boolean isSingleton;
>> + private final int number;
>>
>> - private MyEndpoint(int number) {
>> + private MyEndpoint(boolean isSingleton, int number) {
>> + this.isSingleton = isSingleton;
>> this.number = number;
>> }
>>
>> @@ -92,7 +124,7 @@ public class DefaultProducerCacheTest extends
>> ContextTestSupport {
>>
>> @Override
>> public boolean isSingleton() {
>> - return true;
>> + return isSingleton;
>> }
>>
>> @Override
>> @@ -114,7 +146,12 @@ public class DefaultProducerCacheTest extends
>> ContextTestSupport {
>>
>> @Override
>> protected void doStop() throws Exception {
>> - COUNTER.incrementAndGet();
>> + stopCounter.incrementAndGet();
>> + }
>> +
>> + @Override
>> + protected void doShutdown() throws Exception {
>> + shutdownCounter.incrementAndGet();
>> }
>> }
>>
>>
>
>
>
> --
> Claus Ibsen
> -----------------
> Red Hat, Inc.
> Email:
> cibsen@
> Twitter: davsclaus
> Blog: http://davsclaus.com
> Author of Camel in Action: http://www.manning.com/ibsen
--
View this message in context: http://camel.465427.n5.nabble.com/Re-git-commit-CAMEL-6948-Releasing-a-non-singleton-producer-by-ProducerCache-should-not-only-stop-th-tp5742926p5742957.html
Sent from the Camel Development mailing list archive at Nabble.com.
Re: git commit: CAMEL-6948: Releasing a non-singleton producer by
ProducerCache should not only stop the producer but also shutdown it as
well if it is a ShutdownableService.
Posted by Babak Vahdat <ba...@swissonline.ch>.
Thanks for the hint but that was exactly the reason why I reopened the ticket
to get that done better.
Right now running tests of camel-core to make sure everything is fine.
Babak
Claus Ibsen-2 wrote
> Hi
>
> There is a stopAndShutdownService method on ServiceHelper in the util
> package. Its maybe nicer to use to than the type cast. Then you can do
> that in one command.
>
> And this is also how we stop/start/shutdown services in a cleaner way.
> And if you enabled TRACE/DEBUG logging the service helper can log what
> happens as well.
>
> On Fri, Nov 8, 2013 at 9:46 PM, <
> bvahdat@
> > wrote:
>> Updated Branches:
>> refs/heads/master f74ff2cf6 -> f744afd9c
>>
>>
>> CAMEL-6948: Releasing a non-singleton producer by ProducerCache should
>> not only stop the producer but also shutdown it as well if it is a
>> ShutdownableService.
>>
>> Project: http://git-wip-us.apache.org/repos/asf/camel/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/f744afd9
>> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/f744afd9
>> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/f744afd9
>>
>> Branch: refs/heads/master
>> Commit: f744afd9c39a1fc9261ad68dabe06a51a238fac9
>> Parents: f74ff2c
>> Author: Babak Vahdat <
> bvahdat@
> >
>> Authored: Fri Nov 8 21:45:59 2013 +0100
>> Committer: Babak Vahdat <
> bvahdat@
> >
>> Committed: Fri Nov 8 21:45:59 2013 +0100
>>
>> ----------------------------------------------------------------------
>> .../org/apache/camel/impl/ProducerCache.java | 6 +++
>> .../camel/impl/DefaultProducerCacheTest.java | 53
>> +++++++++++++++++---
>> 2 files changed, 51 insertions(+), 8 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/camel/blob/f744afd9/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> b/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> index 6b292c0..b35eca5 100644
>> --- a/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> +++ b/camel-core/src/main/java/org/apache/camel/impl/ProducerCache.java
>> @@ -30,6 +30,7 @@ import org.apache.camel.Processor;
>> import org.apache.camel.Producer;
>> import org.apache.camel.ProducerCallback;
>> import org.apache.camel.ServicePoolAware;
>> +import org.apache.camel.ShutdownableService;
>> import org.apache.camel.processor.UnitOfWorkProducer;
>> import org.apache.camel.spi.ServicePool;
>> import org.apache.camel.support.ServiceSupport;
>> @@ -137,6 +138,11 @@ public class ProducerCache extends ServiceSupport {
>> } else if (!producer.isSingleton()) {
>> // stop non singleton producers as we should not leak
>> resources
>> producer.stop();
>> +
>> + // shutdown as well in case the producer is shutdownable
>> + if (producer instanceof ShutdownableService) {
>> + ShutdownableService.class.cast(producer).shutdown();
>> + }
>> }
>> }
>>
>>
>> http://git-wip-us.apache.org/repos/asf/camel/blob/f744afd9/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> b/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> index 5ad0f42..803302c 100644
>> ---
>> a/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> +++
>> b/camel-core/src/test/java/org/apache/camel/impl/DefaultProducerCacheTest.java
>> @@ -30,7 +30,8 @@ import org.apache.camel.Producer;
>> */
>> public class DefaultProducerCacheTest extends ContextTestSupport {
>>
>> - private static final AtomicInteger COUNTER = new AtomicInteger();
>> + private final AtomicInteger stopCounter = new AtomicInteger();
>> + private final AtomicInteger shutdownCounter = new AtomicInteger();
>>
>> public void testCacheProducerAcquireAndRelease() throws Exception {
>> ProducerCache cache = new ProducerCache(this, context);
>> @@ -56,7 +57,7 @@ public class DefaultProducerCacheTest extends
>> ContextTestSupport {
>> assertEquals("Size should be 0", 0, cache.size());
>>
>> for (int i = 0; i < 8; i++) {
>> - Endpoint e = new MyEndpoint(i);
>> + Endpoint e = new MyEndpoint(true, i);
>> Producer p = cache.acquireProducer(e);
>> cache.releaseProducer(e, p);
>> }
>> @@ -64,19 +65,50 @@ public class DefaultProducerCacheTest extends
>> ContextTestSupport {
>> assertEquals("Size should be 5", 5, cache.size());
>>
>> // should have stopped the 3 evicted
>> - assertEquals(3, COUNTER.get());
>> + assertEquals(3, stopCounter.get());
>>
>> cache.stop();
>>
>> // should have stopped all 8
>> - assertEquals(8, COUNTER.get());
>> + assertEquals(8, stopCounter.get());
>> + }
>> +
>> + public void
>> testReleaseProducerInvokesStopAndShutdownByNonSingeltonProducers() throws
>> Exception {
>> + ProducerCache cache = new ProducerCache(this, context, 1);
>> + cache.start();
>> +
>> + assertEquals("Size should be 0", 0, cache.size());
>> +
>> + for (int i = 0; i < 3; i++) {
>> + Endpoint e = new MyEndpoint(false, i);
>> + Producer p = cache.acquireProducer(e);
>> + cache.releaseProducer(e, p);
>> + }
>> +
>> + assertEquals("Size should be 0", 0, cache.size());
>> +
>> + // should have stopped all 3
>> + assertEquals(3, stopCounter.get());
>> +
>> + // should have shutdown all 3
>> + assertEquals(3, shutdownCounter.get());
>> +
>> + cache.stop();
>> +
>> + // no more stop after stopping the cache
>> + assertEquals(3, stopCounter.get());
>> +
>> + // no more shutdown after stopping the cache
>> + assertEquals(3, shutdownCounter.get());
>> }
>>
>> private final class MyEndpoint extends DefaultEndpoint {
>>
>> - private int number;
>> + private final boolean isSingleton;
>> + private final int number;
>>
>> - private MyEndpoint(int number) {
>> + private MyEndpoint(boolean isSingleton, int number) {
>> + this.isSingleton = isSingleton;
>> this.number = number;
>> }
>>
>> @@ -92,7 +124,7 @@ public class DefaultProducerCacheTest extends
>> ContextTestSupport {
>>
>> @Override
>> public boolean isSingleton() {
>> - return true;
>> + return isSingleton;
>> }
>>
>> @Override
>> @@ -114,7 +146,12 @@ public class DefaultProducerCacheTest extends
>> ContextTestSupport {
>>
>> @Override
>> protected void doStop() throws Exception {
>> - COUNTER.incrementAndGet();
>> + stopCounter.incrementAndGet();
>> + }
>> +
>> + @Override
>> + protected void doShutdown() throws Exception {
>> + shutdownCounter.incrementAndGet();
>> }
>> }
>>
>>
>
>
>
> --
> Claus Ibsen
> -----------------
> Red Hat, Inc.
> Email:
> cibsen@
> Twitter: davsclaus
> Blog: http://davsclaus.com
> Author of Camel in Action: http://www.manning.com/ibsen
--
View this message in context: http://camel.465427.n5.nabble.com/Re-git-commit-CAMEL-6948-Releasing-a-non-singleton-producer-by-ProducerCache-should-not-only-stop-th-tp5742926p5742932.html
Sent from the Camel Development mailing list archive at Nabble.com.