Author: fhanik
Date: Fri Feb  6 21:49:36 2009
New Revision: 741747

URL: http://svn.apache.org/viewvc?rev=741747&view=rev
Log:
Make sure thread starvation doesn't exist during reclamation of connections

Added:
    
tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/StarvationTest.java
   (with props)
Modified:
    
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
    
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
    
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java

Modified: 
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java?rev=741747&r1=741746&r2=741747&view=diff
==============================================================================
--- 
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
 (original)
+++ 
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
 Fri Feb  6 21:49:36 2009
@@ -98,7 +98,12 @@
      * reference to mbean
      */
     protected org.apache.tomcat.jdbc.pool.jmx.ConnectionPool jmxPool = null;
-
+    
+    /**
+     * counter to track how many threads are waiting for a connection
+     */
+    protected AtomicInteger waitcount = new AtomicInteger(0);
+    
     
//===============================================================================
     //         PUBLIC METHODS
     
//===============================================================================
@@ -142,6 +147,14 @@
     public String getName() {
         return getPoolProperties().getPoolName();
     }
+    
+    /**
+     * Return the number of threads waiting for a connection
+     * @return number of threads waiting for a connection
+     */
+    public int getWaitCount() {
+        return waitcount.get();
+    }
 
     /**
      * Returns the pool properties associated with this connection pool
@@ -397,6 +410,9 @@
                 
jmxPool.notify(org.apache.tomcat.jdbc.pool.jmx.ConnectionPool.NOTIFY_ABANDON, 
trace);
             }
             con.abandon();
+            //we've asynchronously reduced the number of connections
+            //we could have threads stuck in idle.poll(timeout) that will 
never be notified
+            if (waitcount.get()>0) idle.offer(new 
PooledConnection(poolProperties,this));
         } finally {
             con.unlock();
         }
@@ -457,11 +473,17 @@
                 maxWait = 
(getPoolProperties().getMaxWait()<=0)?Long.MAX_VALUE:getPoolProperties().getMaxWait();
             }
             long timetowait = Math.max(0, maxWait - 
(System.currentTimeMillis() - now));
+            waitcount.incrementAndGet();
             try {
                 //retrieve an existing connection
                 con = idle.poll(timetowait, TimeUnit.MILLISECONDS);
             } catch (InterruptedException ex) {
-                Thread.interrupted();
+                Thread.interrupted();//clear the flag, and bail out
+                SQLException sx = new SQLException("Pool wait interrupted.");
+                sx.initCause(ex);
+                throw sx;
+            } finally {
+                waitcount.decrementAndGet();
             }
             if (maxWait==0 && con == null) { //no wait, return one if we have 
one
                 throw new SQLException("[" + 
Thread.currentThread().getName()+"] " +
@@ -529,6 +551,9 @@
         boolean setToNull = false;
         try {
             con.lock();
+            if (!con.isDiscarded() && !con.isInitialized()) {
+                con.connect();
+            }
             if ((!con.isDiscarded()) && 
con.validate(PooledConnection.VALIDATE_BORROW)) {
                 //set the timestamp
                 con.setTimestamp(now);
@@ -647,7 +672,6 @@
                     if ((now - time) > con.getAbandonTimeout()) {
                         busy.remove(con);
                         abandon(con);
-                        release(con);
                         setToNull = true;
                     } else {
                         //do nothing

Modified: 
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java?rev=741747&r1=741746&r2=741747&view=diff
==============================================================================
--- 
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
 (original)
+++ 
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
 Fri Feb  6 21:49:36 2009
@@ -53,8 +53,8 @@
     protected ConnectionPool parent;
 
     protected WeakReference<JdbcInterceptor> handler = null;
-
-    public PooledConnection(PoolProperties prop, ConnectionPool parent) throws 
SQLException {
+    
+    public PooledConnection(PoolProperties prop, ConnectionPool parent) {
         instanceCount = counter.addAndGet(1);
         poolProperties = prop;
         this.parent = parent;
@@ -112,6 +112,10 @@
         }        
         this.discarded = false;
     }
+    
+    public boolean isInitialized() {
+        return connection!=null;
+    }
 
     protected void reconnect() throws SQLException {
         this.disconnect(false);

Modified: 
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java?rev=741747&r1=741746&r2=741747&view=diff
==============================================================================
--- 
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java
 (original)
+++ 
tomcat/trunk/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java
 Fri Feb  6 21:49:36 2009
@@ -233,5 +233,8 @@
     public String getJdbcInterceptors() {
         return pool.getPoolProperties().getJdbcInterceptors();
     }
+    public int getWaitCount() {
+        return pool.getWaitCount();
+    }
 
 }

Added: 
tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/StarvationTest.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/StarvationTest.java?rev=741747&view=auto
==============================================================================
--- 
tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/StarvationTest.java
 (added)
+++ 
tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/StarvationTest.java
 Fri Feb  6 21:49:36 2009
@@ -0,0 +1,96 @@
+package org.apache.tomcat.jdbc.test;
+
+import java.util.concurrent.CountDownLatch;
+import java.sql.Connection;
+import java.sql.SQLException;
+
+import org.apache.tomcat.jdbc.test.CheckOutThreadTest.TestThread;
+
+/**
+ * If a connection is abandoned and closed, 
+ * then that should free up a spot in the pool, and other threads 
+ * that are waiting should not time out and throw an error but be 
+ * able to acquire a connection, since one was just released.
+ * @author fhanik
+ *
+ */
+public class StarvationTest extends DefaultTestCase {
+
+    public StarvationTest(String name) {
+        super(name);
+    }
+    
+    private void config() {
+        datasource.getPoolProperties().setMaxActive(1);
+        datasource.getPoolProperties().setMaxIdle(1);
+        datasource.getPoolProperties().setInitialSize(1);
+        datasource.getPoolProperties().setRemoveAbandoned(true);
+        datasource.getPoolProperties().setRemoveAbandonedTimeout(5);
+        datasource.getPoolProperties().setTimeBetweenEvictionRunsMillis(500);
+        datasource.getPoolProperties().setMaxWait(10000);
+    }
+    
+    public void testDBCPConnectionStarvation() throws Exception {
+        init();
+        config();
+        this.transferProperties();
+        this.tDatasource.getConnection().close();
+        javax.sql.DataSource datasource = this.tDatasource;
+        Connection con1 = datasource.getConnection(); 
+        Connection con2 = null;
+        try {
+            con2 = datasource.getConnection();
+            try {
+                con2.setCatalog("mysql");//make sure connection is valid
+            }catch (SQLException x) {
+                assertFalse("2nd Connection is not 
valid:"+x.getMessage(),true);
+            }
+            assertTrue("Connection 1 should be closed.",con1.isClosed()); 
//first connection should be closed
+        }catch (Exception x) {
+            assertFalse("Connection got starved:"+x.getMessage(),true);
+        }finally {
+            if (con2!=null) con2.close();
+        }
+    }
+    
+    public void testConnectionStarvation() throws Exception {
+        init();
+        config();
+        Connection con1 = datasource.getConnection(); 
+        Connection con2 = null;
+        try {
+            con2 = datasource.getConnection();
+            try {
+                con2.setCatalog("mysql");//make sure connection is valid
+            }catch (SQLException x) {
+                assertFalse("2nd Connection is not 
valid:"+x.getMessage(),true);
+            }
+            assertTrue("Connection 1 should be closed.",con1.isClosed()); 
//first connection should be closed
+        }catch (Exception x) {
+            assertFalse("Connection got starved:"+x.getMessage(),true);
+        }finally {
+            if (con2!=null) con2.close();
+        }
+    }
+
+    public void testFairConnectionStarvation() throws Exception {
+        init();
+        config();
+        datasource.getPoolProperties().setFairQueue(true);
+        Connection con1 = datasource.getConnection(); 
+        Connection con2 = null;
+        try {
+            con2 = datasource.getConnection();
+            try {
+                con2.setCatalog("mysql");//make sure connection is valid
+            }catch (SQLException x) {
+                assertFalse("2nd Connection is not 
valid:"+x.getMessage(),true);
+            }
+            assertTrue("Connection 1 should be closed.",con1.isClosed()); 
//first connection should be closed
+        }catch (Exception x) {
+            assertFalse("Connection got starved:"+x.getMessage(),true);
+        }finally {
+            if (con2!=null) con2.close();
+        }
+    }
+}

Propchange: 
tomcat/trunk/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/StarvationTest.java
------------------------------------------------------------------------------
    svn:eol-style = native



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to