In any case, here's my "cvs diff -u JDBCStore.java".
Index: JDBCStore.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/session/JDBCStore.java,v
retrieving revision 1.8
diff -u -r1.8 JDBCStore.java
--- JDBCStore.java 9 Dec 2002 15:05:55 -0000 1.8
+++ JDBCStore.java 31 Jan 2003 00:16:22 -0000
@@ -8,7 +8,7 @@
*
* The Apache Software License, Version 1.1
*
- * Copyright (c) 1999 The Apache Software Foundation. All rights
+ * Copyright (c) 1999, 2003 The Apache Software Foundation. All rights
* reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -80,6 +80,9 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
+import java.util.List;
+import java.util.LinkedList;
+import java.util.Collections;
import org.apache.catalina.Container;
import org.apache.catalina.LifecycleException;
import org.apache.catalina.Loader;
@@ -120,9 +123,15 @@
protected String connString = null;
/**
- * The database connection.
+ * The pool of database connections.
*/
- private Connection conn = null;
+ private List connPool = null;
+
+ /**
+ * Keep track of number of connections in pool for debugging
+ */
+ private int numConn = 0;
+
/**
* Driver to use.
@@ -387,6 +396,9 @@
* @exception IOException if an input/output error occurred
*/
public String[] keys() throws IOException {
+
+ if (debug > 2) log("keys()");
+
String keysSql =
"SELECT COUNT(s."+sessionIdCol+"), c."+sessionIdCol+
" FROM "+sessionTable+" s, "+sessionTable+" c"+
@@ -415,8 +427,10 @@
} else {
keys = new String[0];
}
+ release(_conn);
} catch(SQLException e) {
log(sm.getString(getStoreName()+".SQLException", e));
+ dropConnection(_conn);
} finally {
try {
if(rst != null)
@@ -424,9 +438,6 @@
} catch(SQLException e) {
;
}
-
- release(_conn);
- _conn = null;
}
return(keys);
@@ -456,8 +467,11 @@
rst = preparedSizeSql.executeQuery();
if (rst.next())
size = rst.getInt(1);
+
+ release(_conn);
} catch(SQLException e) {
log(sm.getString(getStoreName()+".SQLException", e));
+ dropConnection(_conn);
} finally {
try {
if(rst != null)
@@ -465,9 +479,6 @@
} catch(SQLException e) {
;
}
-
- release(_conn);
- _conn = null;
}
return(size);
@@ -522,8 +533,11 @@
} else if (debug > 0) {
log(getStoreName()+": No persisted data object found");
}
+ if (debug > 1) log("load(" + id + ")");
+ release(_conn);
} catch(SQLException e) {
log(sm.getString(getStoreName()+".SQLException", e));
+ dropConnection(_conn);
} finally {
try {
if(rst != null)
@@ -531,9 +545,6 @@
} catch(SQLException e) {
;
}
-
- release(_conn);
- _conn = null;
}
if(ois != null) {
@@ -552,7 +563,7 @@
}
}
- if (debug > 0)
+ if (debug > 2)
log(sm.getString(getStoreName()+".loading",
id, sessionTable));
}
@@ -574,6 +585,8 @@
String removeSql = "DELETE FROM "+sessionTable+" WHERE "+
sessionIdCol+" = ?";
+ if (debug > 1) log("remove(" + id + ")");
+
if(_conn == null)
return;
@@ -583,15 +596,15 @@
preparedRemoveSql.setString(1, id);
preparedRemoveSql.execute();
+
+ release(_conn);
+
} catch(SQLException e) {
log(sm.getString(getStoreName()+".SQLException", e));
+ dropConnection(_conn);
} finally {
- release(_conn);
- _conn = null;
+ ;
}
-
- if (debug > 0)
- log(sm.getString(getStoreName()+".removing", id, sessionTable));
}
/**
@@ -611,11 +624,12 @@
preparedClearSql = _conn.prepareStatement(clearSql);
preparedClearSql.execute();
+ release(_conn);
} catch(SQLException e) {
log(sm.getString(getStoreName()+".SQLException", e));
+ dropConnection(_conn);
} finally {
- release(_conn);
- _conn = null;
+ ;
}
}
@@ -646,6 +660,7 @@
// * Check if ID exists in database and if so use UPDATE.
remove(session.getId());
+ if (debug > 1) log("save(" + session.getId() + ")");
try {
bos = new ByteArrayOutputStream();
oos = new ObjectOutputStream(new BufferedOutputStream(bos));
@@ -667,8 +682,11 @@
preparedSaveSql.setInt(4, session.getMaxInactiveInterval());
preparedSaveSql.setLong(5, session.getLastAccessedTime());
preparedSaveSql.execute();
+
+ release(_conn);
} catch(SQLException e) {
log(sm.getString(getStoreName()+".SQLException", e));
+ dropConnection(_conn);
} catch (IOException e) {
;
} finally {
@@ -682,13 +700,8 @@
bos = null;
oos = null;
in = null;
-
- release(_conn);
- _conn = null;
}
- if (debug > 0)
- log(sm.getString(getStoreName()+".saving",
- session.getId(), sessionTable));
+
}
// --------------------------------------------------------- Protected Methods
@@ -700,16 +713,25 @@
*
* @return <code>Connection</code> if the connection suceeded
*/
- protected Connection getConnection(){
+ protected synchronized Connection getConnection(){
+ Connection conn = null;
+ if (debug > 3) log("getConnection()");
try {
- if(conn == null || conn.isClosed()) {
+ // if the pool isn't empty, return a connection from the
+ // pool and remove it from the list so that nobody else
+ // can get it
+ if(!connPool.isEmpty()) {
+ conn = (Connection)connPool.remove(0);
+ } else {
Class.forName(driverName);
log(sm.getString(getStoreName()+".checkConnectionDBClosed"));
conn = DriverManager.getConnection(connString);
- conn.setAutoCommit(true);
if(conn == null || conn.isClosed())
log(sm.getString(getStoreName()+".checkConnectionDBReOpenFail"));
+
+ conn.setAutoCommit(true);
+ log("getConnection() = " + conn.toString());
}
} catch (SQLException ex){
log(sm.getString(getStoreName()+".checkConnectionSQLException",
@@ -723,23 +745,47 @@
}
/**
- * Release the connection, not needed here since the
- * connection is not associated with a connection pool.
+ * Release the connection back to the pool
*
* @param conn The connection to be released
*/
protected void release(Connection conn) {
- ;
+ // add it back to the pool of available connections
+ if (debug > 3) log("release(" + conn.toString() + ")");
+ if (conn != null) {
+ connPool.add(conn);
+ if (connPool.size() > numConn) {
+ // output the number of connections
+ numConn = connPool.size();
+ log("Connection pool size increased to " + numConn);
+ }
+ }
+ }
+
+ /**
+ * Drop the connection when it has problems
+ *
+ * @param conn The connection to be dropped
+ */
+ protected void dropConnection(Connection conn) {
+ // do NOT add it back to the pool of available connections (let GC have it)
+ log("dropConnection(" + conn.toString() + ")");
+ // don't close the dropped connection since it can confuse the Driver
+ // (mysql driver doesn't handle it well)
}
/**
* Called once when this Store is first started.
*/
public void start() throws LifecycleException {
+ if (debug > 1) log("start()");
super.start();
- // Open connection to the database
- this.conn = getConnection();
+ // Initialize the connection pool
+ this.connPool = Collections.synchronizedList(new LinkedList());
+ // create and release the first connection in the pool so that
+ // a connection will be there for the first user
+ release(getConnection());
}
/**
@@ -751,12 +797,7 @@
super.stop();
// Close and release everything associated with our db.
- if(conn != null) {
- try {
- conn.commit();
- } catch (SQLException e) {
- ;
- }
+ if(connPool != null) {
if( preparedSizeSql != null ) {
try {
@@ -806,19 +847,14 @@
}
}
- try {
- conn.close();
- } catch (SQLException e) {
- ;
- }
-
this.preparedSizeSql = null;
this.preparedKeysSql = null;
this.preparedSaveSql = null;
this.preparedClearSql = null;
this.preparedRemoveSql = null;
this.preparedLoadSql = null;
- this.conn = null;
+ this.connPool.clear();
+ this.connPool = null;
}
}
}
On Thursday, January 30, 2003, at 02:31 PM, Tom Anderson wrote:
True, that might be a better design but my motivation was for a quick
fix with minimal impact to the design.
On Thursday, January 30, 2003, at 01:25 PM, Glenn Nielsen wrote:
A better solution might be to have a Store which can use a JNDI named
DataSource. Then let the DataSource worry about connection pooling,
etc.
Glenn
Tom Anderson wrote:
<insert newbie disclaimers here>
I have been trying to use the JDBCStore stuff and found it to be a
little unstable especially when the database connections have
problems (or when the database is bounced). So, I decided to make
some modifications that would allow it to drop connections on
SQLExceptions and maintain a small connection pool (LinkedList of
Connections).
Should I post the patch to this forum?
~Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
