Permalink
Browse files

Change async checkout behavior to be more like sync checkout: only c…

…reate resources (connections) when they are needed.

    Now, connections should only be created on demand. The initial code created connections until the max limit of the pool was

    Some minor tweaks to test that confirm the desired behavior at a unit test level.
  • Loading branch information...
1 parent adc3eb9 commit 2b30b333a2a5e8003c7e32431fcc162002467505 @jayjwylie jayjwylie committed Nov 8, 2012
@@ -124,14 +124,12 @@ public V checkout(K key) throws Exception {
long startNs = System.nanoTime();
Pool<V> resourcePool = getResourcePoolForKey(key);
- // Always attempt to grow. This protects against running out of
- // resources because they were destroyed.
- attemptGrow(key, resourcePool);
-
V resource = null;
try {
checkNotClosed();
- resource = attemptCheckout(resourcePool);
+ // Must attempt a non blocking checkout before blockingGet to ensure
+ // resources are created for the pool.
+ resource = attemptNonBlockingCheckout(key, resourcePool);
if(resource == null) {
long timeRemainingNs = resourcePoolConfig.getTimeout(TimeUnit.NANOSECONDS)
@@ -155,25 +153,25 @@ public V checkout(K key) throws Exception {
}
/**
- * Get a free resource if one exists. This method does not block. It either
- * returns null or a resource.
+ * Get a free resource if one exists. If there are no free resources,
+ * attempt to create a new resource (up to the max allowed for the pool).
+ * This method does not block per se. However, creating a resource may be
+ * (relatively) expensive. This method either returns null or a resource.
+ *
+ * This method is the only way in which new resources are created for the
+ * pool. So, non blocking checkouts must be attempted to populate the
+ * resource pool.
*/
- protected V attemptCheckout(Pool<V> pool) throws Exception {
+ protected V attemptNonBlockingCheckout(K key, Pool<V> pool) throws Exception {
V resource = pool.nonBlockingGet();
+ if(resource == null) {
+ if(pool.attemptGrow(key, this.objectFactory)) {
+ resource = pool.nonBlockingGet();
+ }
+ }
return resource;
}
- /**
- * Attempt to create a new object and add it to the pool--this only happens
- * if there is room for the new object. This method does not block. This
- * method returns true if it adds a resource to the pool. (Returning true
- * does not guarantee subsequent checkout will succeed because concurrent
- * checkouts may occur.)
- */
- protected boolean attemptGrow(K key, Pool<V> pool) throws Exception {
- return pool.attemptGrow(key, this.objectFactory);
- }
-
/**
* Get the pool for the given key. If no pool exists, create one.
*/
@@ -103,24 +103,17 @@ public void registerResourceRequest(K key, AsyncResourceRequest<V> resourceReque
Queue<AsyncResourceRequest<V>> requestQueue = getRequestQueueForKey(key);
if(requestQueue.isEmpty()) {
- // Optimistically attempt non-blocking checkout iff requestQueue is
- // empty.
- Pool<V> resourcePool = getResourcePoolForKey(key);
- try {
- attemptGrow(key, resourcePool);
- } catch(Exception e) {
- resourceRequest.handleException(e);
- return;
- }
+ // Attempt non-blocking checkout iff requestQueue is empty.
+ Pool<V> resourcePool = getResourcePoolForKey(key);
V resource = null;
-
try {
- resource = attemptCheckout(resourcePool);
+ resource = attemptNonBlockingCheckout(key, resourcePool);
} catch(Exception e) {
destroyResource(key, resourcePool, resource);
resource = null;
resourceRequest.handleException(e);
+ return;
}
if(resource != null) {
resourceRequest.useResource(resource);
@@ -173,9 +166,9 @@ private boolean processQueue(K key) {
V resource = null;
try {
- // Always attempt to grow to deal with destroyed resources.
- attemptGrow(key, resourcePool);
- resource = attemptCheckout(resourcePool);
+ // Must attempt non-blocking checkout to ensure resources are
+ // created for the pool.
+ resource = attemptNonBlockingCheckout(key, resourcePool);
} catch(Exception e) {
destroyResource(key, resourcePool, resource);
resource = null;
@@ -101,9 +101,9 @@ public void testExceptionOnDestroy() throws Exception {
assertEquals(1, this.pool.getCheckedInResourceCount());
this.pool.checkout("a");
- assertEquals(2, this.factory.getCreated());
- assertEquals(2, this.pool.getTotalResourceCount());
- assertEquals(1, this.pool.getCheckedInResourceCount());
+ assertEquals(1, this.factory.getCreated());
+ assertEquals(1, this.pool.getTotalResourceCount());
+ assertEquals(0, this.pool.getCheckedInResourceCount());
for(int i = 0; i < POOL_SIZE - 1; i++) {
checkedOut = this.pool.checkout("a");
@@ -296,8 +296,8 @@ public void contendForResources() throws Exception {
try {
waitForCheckers.await();
- assertEquals(POOL_SIZE, this.pool.getTotalResourceCount());
- assertEquals(POOL_SIZE, this.pool.getCheckedInResourceCount());
+ assertEquals(this.pool.getCheckedInResourceCount(), this.pool.getTotalResourceCount());
+ // assertEquals(POOL_SIZE, this.pool.getCheckedInResourceCount());
} catch(InterruptedException e) {
e.printStackTrace();
}
@@ -335,8 +335,8 @@ public void contendForQueueAndPool() throws Exception {
try {
waitForThreadsEnd.await();
- assertEquals(POOL_SIZE, this.queuedPool.getTotalResourceCount());
- assertEquals(POOL_SIZE, this.queuedPool.getCheckedInResourceCount());
+ assertEquals(this.queuedPool.getCheckedInResourceCount(),
+ this.queuedPool.getTotalResourceCount());
assertEquals(0, this.queuedPool.getRegisteredResourceRequestCount());
assertEquals(numEnqueuers * numEnqueues, TestResourceRequest.usedResourceCount.get());

0 comments on commit 2b30b33

Please sign in to comment.