From 8bb486073f5f434103e8e8ee3e512629c56c2e79 Mon Sep 17 00:00:00 2001 From: fhanik Date: Sun, 3 May 2009 00:18:33 +0000 Subject: [PATCH] Make connection objects non reusable. Once release has been called, it can't be reused. This makes the sizing algorithm easier C3P0 leaks connections during the fairness test, reaches 20 connections even max is set to 10 git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@771006 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/tomcat/jdbc/pool/ConnectionPool.java | 24 ++++++++++++---------- .../apache/tomcat/jdbc/pool/PooledConnection.java | 7 ++++++- .../apache/tomcat/jdbc/test/DefaultTestCase.java | 2 +- .../org/apache/tomcat/jdbc/test/FairnessTest.java | 13 ++++++------ 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java b/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java index d090aa78b..ece2480d3 100644 --- a/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java +++ b/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java @@ -60,39 +60,39 @@ public class ConnectionPool { /** * All the information about the connection pool */ - protected PoolProperties poolProperties; + private PoolProperties poolProperties; /** * Contains all the connections that are in use * TODO - this shouldn't be a blocking queue, simply a list to hold our objects */ - protected BlockingQueue busy; + private BlockingQueue busy; /** * Contains all the idle connections */ - protected BlockingQueue idle; + private BlockingQueue idle; /** * The thread that is responsible for checking abandoned and idle threads */ - protected PoolCleaner poolCleaner; + private PoolCleaner poolCleaner; /** * Pool closed flag */ - protected boolean closed = false; + private boolean closed = false; /** * Since newProxyInstance performs the same operation, over and over * again, it is much more optimized if we simply store the constructor ourselves. */ - protected Constructor proxyClassConstructor; + private Constructor proxyClassConstructor; /** * Executor service used to cancel Futures */ - protected ThreadPoolExecutor cancellator = new ThreadPoolExecutor(0,1,1000,TimeUnit.MILLISECONDS,new LinkedBlockingQueue()); + private ThreadPoolExecutor cancellator = new ThreadPoolExecutor(0,1,1000,TimeUnit.MILLISECONDS,new LinkedBlockingQueue()); /** * reference to mbean @@ -102,7 +102,7 @@ public class ConnectionPool { /** * counter to track how many threads are waiting for a connection */ - protected AtomicInteger waitcount = new AtomicInteger(0); + private AtomicInteger waitcount = new AtomicInteger(0); //=============================================================================== // PUBLIC METHODS @@ -427,9 +427,11 @@ public class ConnectionPool { return; try { con.lock(); - con.release(); + if (con.release()) { + size.addAndGet(-1); + } } finally { - size.addAndGet(-1); + con.unlock(); } } @@ -441,7 +443,7 @@ public class ConnectionPool { * @return PooledConnection * @throws SQLException */ - protected PooledConnection borrowConnection(int wait) throws SQLException { + private PooledConnection borrowConnection(int wait) throws SQLException { if (isClosed()) { throw new SQLException("Connection pool closed."); diff --git a/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java b/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java index 5351fb7cb..ddbd71f80 100644 --- a/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java +++ b/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java @@ -26,6 +26,7 @@ import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.jdbc.pool.interceptor.ConnectionState; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; /** @@ -109,6 +110,8 @@ public class PooledConnection { */ private WeakReference handler = null; + private AtomicBoolean released = new AtomicBoolean(false); + public PooledConnection(PoolProperties prop, ConnectionPool parent) { instanceCount = counter.addAndGet(1); @@ -117,6 +120,7 @@ public class PooledConnection { } public void connect() throws SQLException { + if (released.get()) throw new SQLException("A connection once released, can't be reestablished."); if (connection != null) { try { this.disconnect(false); @@ -292,7 +296,7 @@ public class PooledConnection { /** * This method is called if (Now - timeCheckedIn > getReleaseTime()) */ - public void release() { + public boolean release() { try { disconnect(true); } catch (Exception x) { @@ -300,6 +304,7 @@ public class PooledConnection { log.debug("Unable to close SQL connection",x); } } + return released.compareAndSet(false, true); } diff --git a/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/DefaultTestCase.java b/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/DefaultTestCase.java index 10ed11095..f24036d68 100644 --- a/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/DefaultTestCase.java +++ b/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/DefaultTestCase.java @@ -39,7 +39,7 @@ import com.mchange.v2.log.MLog; public class DefaultTestCase extends TestCase { protected org.apache.tomcat.jdbc.pool.DataSource datasource; protected BasicDataSource tDatasource; - protected DataSource c3p0Datasource; + protected ComboPooledDataSource c3p0Datasource; protected int threadcount = 10; protected int iterations = 100000; public DefaultTestCase(String name) { diff --git a/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/FairnessTest.java b/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/FairnessTest.java index 1f2b95b64..5f87fea89 100644 --- a/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/FairnessTest.java +++ b/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/FairnessTest.java @@ -41,7 +41,7 @@ public class FairnessTest extends DefaultTestCase { protected long complete = Long.getLong("complete",20000); protected boolean printthread = Boolean.getBoolean("printthread"); CountDownLatch latch = null; - protected void printThreadResults(TestThread[] threads, String name) { + protected void printThreadResults(TestThread[] threads, String name, int active, int expected) { long minfetch = Long.MAX_VALUE, maxfetch = Long.MIN_VALUE, totalfetch = 0; long maxwait = 0, minwait = Long.MAX_VALUE, averagewait = 0, totalwait = 0; float avgfetch = 0; @@ -59,6 +59,7 @@ public class FairnessTest extends DefaultTestCase { System.out.println("["+name+"] Max fetch:"+(maxfetch)+" Min fetch:"+(minfetch)+" Average fetch:"+ (((float)totalfetch))/(float)threads.length); System.out.println("["+name+"] Max wait:"+(((float)maxwait)/1000000f)+"ms. Min wait:"+(((float)minwait)/1000000f)+"ms. Average wait:"+(((((float)totalwait))/(float)totalfetch)/1000000f)+" ms."); + System.out.println("["+name+"] Max active:"+active+" Expected Active:"+expected); } @@ -87,7 +88,7 @@ public class FairnessTest extends DefaultTestCase { } this.run = false; long delta = System.currentTimeMillis() - start; - printThreadResults(threads,"testDBCPThreads20Connections10"); + printThreadResults(threads,"testDBCPThreads20Connections10",this.tDatasource.getNumActive(),10); tearDown(); } @@ -116,7 +117,7 @@ public class FairnessTest extends DefaultTestCase { } this.run = false; long delta = System.currentTimeMillis() - start; - printThreadResults(threads,"testPoolThreads20Connections10"); + printThreadResults(threads,"testPoolThreads20Connections10",this.datasource.getSize(),10); tearDown(); } @@ -146,7 +147,7 @@ public class FairnessTest extends DefaultTestCase { } this.run = false; long delta = System.currentTimeMillis() - start; - printThreadResults(threads,"testPoolThreads20Connections10Fair"); + printThreadResults(threads,"testPoolThreads20Connections10Fair",this.datasource.getSize(),10); tearDown(); } @@ -176,7 +177,7 @@ public class FairnessTest extends DefaultTestCase { } this.run = false; long delta = System.currentTimeMillis() - start; - printThreadResults(threads,"testPoolThreads20Connections10FairAsync"); + printThreadResults(threads,"testPoolThreads20Connections10FairAsync",this.datasource.getSize(),10); tearDown(); } @@ -205,7 +206,7 @@ public class FairnessTest extends DefaultTestCase { } this.run = false; long delta = System.currentTimeMillis() - start; - printThreadResults(threads,"testC3P0Threads20Connections10"); + printThreadResults(threads,"testC3P0Threads20Connections10",c3p0Datasource.getNumConnectionsAllUsers(),10); tearDown(); } -- 2.11.0