Refactor db connection pooling: Use connection names instead of connection properties as has keys, introduce new DbConnection wrapper class and a serial-id flag in DbSource to validate connections.

src/helma/framework/core/Application.java

@@ -1526,6 +1526,13 @@ public final class Application implements Runnable {
         getEventLog().info(msg);
     }
 
+    /**
+     * Log a generic application debug message
+     */
+    public void logDebug(String msg) {
+        getEventLog().debug(msg);
+    }
+
     /**
      * Log an application access
      */

src/helma/main/Server.java

@@ -417,7 +417,6 @@ public class Server implements Runnable {
         dbProps = new ResourceProperties();
         dbProps.setIgnoreCase(false);
         dbProps.addResource(new FileResource(file));
-        DbSource.setDefaultProps(dbProps);
 
         // read apps.properties file
         String appsPropfile = sysProps.getProperty("appsPropFile");

src/helma/objectmodel/db/DbConnection.java

@@ -0,0 +1,60 @@
+package helma.objectmodel.db;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+/**
+ * A thin wrapper around a java.sql.Connection, providing utility methods
+ * for connection validation and closing.
+ */
+public class DbConnection {
+
+    private final Connection connection;
+    private final int serialId;
+
+    public DbConnection(Connection connection, int serialId) {
+        if(connection == null) {
+            throw new NullPointerException("connection parameter null in DbConnection");
+        }
+        this.connection = connection;
+        this.serialId = serialId;
+    }
+
+    public Connection getConnection() {
+        return connection;
+    }
+
+    public int getSerialId() {
+        return serialId;
+    }
+
+    public void close() {
+        try {
+            if (!connection.isClosed()) {
+                connection.close();
+            }
+        } catch (SQLException x) {
+            System.err.println("Error closing DB connection: " + x);
+        }
+    }
+
+    public boolean isValid(int id) {
+        if (id != serialId) {
+            return false;
+        }
+        // test if connection is still ok
+        try {
+            Statement stmt = connection.createStatement();
+            stmt.execute("SELECT 1");
+            stmt.close();
+        } catch (SQLException sx) {
+            return false;
+        }
+        return true;
+    }
+
+    public String toString() {
+        return "DbConnection[" + connection.toString() + "]";
+    }
+}

src/helma/objectmodel/db/DbSource.java

@@ -27,20 +27,18 @@ import java.util.Properties;
 import java.util.Hashtable;
 
 /**
- *  This class describes a releational data source (URL, driver, user and password).
+ *  This class describes a relational data source (URL, driver, user and password).
  */
 public class DbSource {
-    private static ResourceProperties defaultProps = null;
-    private Properties conProps;
     private String name;
-    private ResourceProperties props, subProps;
+    private int serialId = 0;
+    private ResourceProperties props;
     protected String url;
     private String driver;
+    private Properties conProps;
     private boolean isOracle, isMySQL, isPostgreSQL, isH2;
     private long lastRead = 0L;
     private Hashtable dbmappings = new Hashtable();
-    // compute hashcode statically because it's expensive and we need it often
-    private int hashcode;
     // thread local connection holder for non-transactor threads
     private ThreadLocal connection;
 
@@ -68,56 +66,47 @@ public class DbSource {
      */
     public synchronized Connection getConnection()
             throws ClassNotFoundException, SQLException {
-        Connection con;
+        DbConnection con;
         Transactor tx = Transactor.getInstance();
+        if (props.lastModified() != lastRead) {
+            init();
+        }
         if (tx != null) {
-            con = tx.getConnection(this);
+            con = tx.getDbConnection(name, serialId);
         } else {
-            con = getThreadLocalConnection();
+            con = getThreadLocalDbConnection();
         }
 
-        boolean fileUpdated = props.lastModified() > lastRead ||
+        if (con == null) {
-                (defaultProps != null && defaultProps.lastModified() > lastRead);
+            con = new DbConnection(DriverManager.getConnection(url, conProps), serialId);
-
-        if (con == null || con.isClosed() || fileUpdated) {
-            init();
-            con = DriverManager.getConnection(url, conProps);
 
             // If we wanted to use SQL transactions, we'd set autoCommit to
             // false here and make commit/rollback invocations in Transactor methods;
             // System.err.println ("Created new Connection to "+url);
             if (tx != null) {
-                tx.registerConnection(this, con);
+                tx.registerConnection(name, con);
             } else {
                 connection.set(con);
             }
         }
 
-        return con;
+        return con.getConnection();
     }
 
     /**
      * Used for connections not managed by a Helma transactor
      * @return a thread local tested connection, or null
      */
-    private Connection getThreadLocalConnection() {
+    private DbConnection getThreadLocalDbConnection() {
         if (connection == null) {
             connection = new ThreadLocal();
             return null;
         }
-        Connection con = (Connection) connection.get();
+        DbConnection con = (DbConnection) connection.get();
-        if (con != null) {
+        if (con != null && !con.isValid(serialId)) {            
-            // test if connection is still ok
+            con.close();
-            try {
+            connection.remove();
-                Statement stmt = con.createStatement();
+            return null;
-                stmt.execute("SELECT 1");
-                stmt.close();
-            } catch (SQLException sx) {
-                try {
-                    con.close();
-                } catch (SQLException ignore) {/* nothing to do */}
-                return null;
-            }
         }
         return con;
     }
@@ -142,13 +131,10 @@ public class DbSource {
      * @throws ClassNotFoundException if the JDBC driver couldn't be loaded
      */
     private synchronized void init() throws ClassNotFoundException {
-        lastRead = (defaultProps == null) ? props.lastModified()
+        lastRead = props.lastModified();
-                                          : Math.max(props.lastModified(),
+        serialId ++;
-                                                     defaultProps.lastModified());
         // refresh sub-properties for this DbSource
-        subProps = props.getSubProperties(name + '.');
+        ResourceProperties subProps = props.getSubProperties(name + '.');
-        // use properties hashcode for ourselves
-        hashcode = subProps.hashCode();
         // get JDBC URL and driver class name
         url = subProps.getProperty("url");
         driver = subProps.getProperty("driver");
@@ -212,15 +198,6 @@ public class DbSource {
         return name;
     }
 
-    /**
-     * Set the default (server-wide) properties
-     *
-     * @param props server default db.properties
-     */
-    public static void setDefaultProps(ResourceProperties props) {
-        defaultProps = props;
-    }
-
     /**
      * Check if this DbSource represents an Oracle database
      *
@@ -278,17 +255,4 @@ public class DbSource {
         return (DbMapping) dbmappings.get(tablename.toUpperCase());
     }
 
-    /**
-     * Returns a hash code value for the object.
-     */
-    public int hashCode() {
-        return hashcode;
-    }
-
-    /**
-     * Indicates whether some other object is "equal to" this one.
-     */
-    public boolean equals(Object obj) {
-        return obj instanceof DbSource && subProps.equals(((DbSource) obj).subProps);
-    }
 }

src/helma/objectmodel/db/Transactor.java

@@ -234,37 +234,38 @@ public class Transactor {
 
     /**
      * Register a db connection with this transactor thread.
-     * @param src the db source
+     * @param name the db source name
      * @param con the connection
      */
-    public void registerConnection(DbSource src, Connection con) {
+    public void registerConnection(String name, DbConnection con) {
-        sqlConnections.put(src, con);
+        DbConnection previous = (DbConnection) sqlConnections.put(name, con);
+        if (previous != null) {
+            nmgr.app.logEvent("Closing previous connection " + con);
+            previous.close();
+        }
         // we assume a freshly created connection is ok.
-        testedConnections.put(src, new Long(System.currentTimeMillis()));
+        testedConnections.put(name, new Long(System.currentTimeMillis()));
     }
 
     /**
      * Get a db connection that was previously registered with this transactor thread.
-     * @param src the db source
+     * @param name the db source name
-     * @return the connection
+     * @param serialId the current serial id of the db source definition, used for validation
+     * @return the connection, or null if no valid connection is available
      */
-    public Connection getConnection(DbSource src) {
+    public DbConnection getDbConnection(String name, int serialId) {
-        Connection con = (Connection) sqlConnections.get(src);
+        DbConnection con = (DbConnection) sqlConnections.get(name);
-        Long tested = (Long) testedConnections.get(src);
+        Long tested = (Long) testedConnections.get(name);
         long now = System.currentTimeMillis();
-        if (con != null && (tested == null || now - tested.longValue() > 60000)) {
+        // Check if the connection is still valid
-            // Check if the connection is still alive by executing a simple statement.
+        if (con != null
-            try {
+                && (tested == null || now - tested.longValue() > 60000) 
-                Statement stmt = con.createStatement();
+                && !con.isValid(serialId)) {
-                stmt.execute("SELECT 1");
+            nmgr.app.logEvent("Closing cached connection " + con);
-                stmt.close();
+            sqlConnections.remove(name);
-                testedConnections.put(src, new Long(now));
+            testedConnections.remove(name);
-            } catch (SQLException sx) {
+            con.close();
-                try {
+            return null;
-                    con.close();
-                } catch (SQLException ignore) {/* nothing to do */}
-                return null;
-            }
         }
         return con;
     }
@@ -529,12 +530,11 @@ public class Transactor {
         if (sqlConnections != null) {
             for (Iterator i = sqlConnections.values().iterator(); i.hasNext();) {
                 try {
-                    Connection con = (Connection) i.next();
+                    DbConnection con = (DbConnection) i.next();
-
                     con.close();
                     nmgr.app.logEvent("Closing DB connection: " + con);
-                } catch (Exception ignore) {
+                } catch (Exception x) {
-                    // exception closing db connection, ignore
+                    nmgr.app.logEvent("Error closing DB connection: " + x);
                 }
             }