camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Babak Vahdat <babak.vah...@swissonline.ch>
Subject 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.
Date Sun, 10 Nov 2013 11:59:39 GMT
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,  &lt;

> bvahdat@

> &gt; 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 &lt;

> bvahdat@

> &gt;
>> Authored: Fri Nov 8 21:45:59 2013 +0100
>> Committer: Babak Vahdat &lt;

> bvahdat@

> &gt;
>> 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.

Mime
View raw message