On 06/06/2011 21:40, Felix Schumacher wrote: > Am Montag, den 06.06.2011, 17:56 +0100 schrieb Mark Thomas: >> On 06/06/2011 11:59, Keiichi Fujino wrote: >>> When jdbc-pool or DBCP is used as DB connection pool and >>> removeAbandoned is "true", >>> dbConnection is closed because dbConnection is not returned to the >>> connection pool. >>> And then, dbConnection is acquired again because dbConnection has been >>> already closed. >>> >>> Should I return the connection to the connection pool? >>> Or, is the DB connection cached until removeAbandoned works? >> >> The connection should be returned to the pool. If you could take care of >> that, that would be great. Sorry for missing that when I applied the patch. > It was a conscious decision not to return the connection to the pool for > the following reasons: > 1. the original code didn't do it
The expected usage of pooled connections is that they are obtained from the pool, used and then immediately returned. Removing a connection from the pool and just keeping it defeats the point of having a pool. > 2. we would have to close the prepared statements in case of pooled > connections Yes, although DBCP can pool those too (if appropriately configured). You may wish to add something on that topic to the docs, if only a link the the relevant DBCP config docs. > 3. the connection is not really abandoned, so one could argue, that you > should run the pool with removeAbandoned set to 'false' Yep, but as per 1, it isn't really following the expected conventions when using a connection pool. > But I would gladly provide a patch where connections are returned to the > pool. That would be great. Thanks. Mark > > Felix >> >> Mark >> >>> >>> >>> 2011/6/4 <ma...@apache.org>: >>>> Author: markt >>>> Date: Fri Jun 3 22:13:09 2011 >>>> New Revision: 1131263 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=1131263&view=rev >>>> Log: >>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51264 >>>> Allow the JDBC persistent session store to use a JNDI datasource to define >>>> the database in which sessions are persisted. >>>> Patch provided by Felix Schumacher. >>>> >>>> Modified: >>>> tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java >>>> tomcat/trunk/java/org/apache/catalina/session/LocalStrings.properties >>>> tomcat/trunk/webapps/docs/changelog.xml >>>> tomcat/trunk/webapps/docs/config/manager.xml >>>> >>>> Modified: tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java >>>> URL: >>>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java?rev=1131263&r1=1131262&r2=1131263&view=diff >>>> ============================================================================== >>>> --- tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java (original) >>>> +++ tomcat/trunk/java/org/apache/catalina/session/JDBCStore.java Fri Jun >>>> 3 22:13:09 2011 >>>> @@ -33,6 +33,11 @@ import java.sql.SQLException; >>>> import java.util.ArrayList; >>>> import java.util.Properties; >>>> >>>> +import javax.naming.Context; >>>> +import javax.naming.InitialContext; >>>> +import javax.naming.NamingException; >>>> +import javax.sql.DataSource; >>>> + >>>> import org.apache.catalina.Container; >>>> import org.apache.catalina.LifecycleException; >>>> import org.apache.catalina.Loader; >>>> @@ -102,6 +107,16 @@ public class JDBCStore extends StoreBase >>>> */ >>>> protected String driverName = null; >>>> >>>> + /** >>>> + * name of the JNDI resource >>>> + */ >>>> + protected String dataSourceName = null; >>>> + >>>> + /** >>>> + * DataSource to use >>>> + */ >>>> + protected DataSource dataSource = null; >>>> + >>>> // ------------------------------------------------------------- Table >>>> & cols >>>> >>>> /** >>>> @@ -436,6 +451,27 @@ public class JDBCStore extends StoreBase >>>> return (this.sessionLastAccessedCol); >>>> } >>>> >>>> + /** >>>> + * Set the JNDI name of a DataSource-factory to use for db access >>>> + * >>>> + * @param dataSourceName The JNDI name of the DataSource-factory >>>> + */ >>>> + public void setDataSourceName(String dataSourceName) { >>>> + if (dataSourceName == null || "".equals(dataSourceName.trim())) { >>>> + manager.getContainer().getLogger().warn( >>>> + sm.getString(getStoreName() + >>>> ".missingDataSourceName")); >>>> + return; >>>> + } >>>> + this.dataSourceName = dataSourceName; >>>> + } >>>> + >>>> + /** >>>> + * Return the name of the JNDI DataSource-factory >>>> + */ >>>> + public String getDataSourceName() { >>>> + return this.dataSourceName; >>>> + } >>>> + >>>> // --------------------------------------------------------- Public >>>> Methods >>>> >>>> /** >>>> @@ -866,6 +902,24 @@ public class JDBCStore extends StoreBase >>>> if (dbConnection != null) >>>> return (dbConnection); >>>> >>>> + if (dataSourceName != null && dataSource == null) { >>>> + Context initCtx; >>>> + try { >>>> + initCtx = new InitialContext(); >>>> + Context envCtx = (Context) >>>> initCtx.lookup("java:comp/env"); >>>> + this.dataSource = (DataSource) >>>> envCtx.lookup(this.dataSourceName); >>>> + } catch (NamingException e) { >>>> + manager.getContainer().getLogger().error( >>>> + sm.getString(getStoreName() + ".wrongDataSource", >>>> + this.dataSourceName), e); >>>> + } >>>> + } >>>> + >>>> + if (dataSource != null) { >>>> + dbConnection = dataSource.getConnection(); >>>> + return dbConnection; >>>> + } >>>> + >>>> // Instantiate our database driver if necessary >>>> if (driver == null) { >>>> try { >>>> >>>> Modified: >>>> tomcat/trunk/java/org/apache/catalina/session/LocalStrings.properties >>>> URL: >>>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/LocalStrings.properties?rev=1131263&r1=1131262&r2=1131263&view=diff >>>> ============================================================================== >>>> --- tomcat/trunk/java/org/apache/catalina/session/LocalStrings.properties >>>> (original) >>>> +++ tomcat/trunk/java/org/apache/catalina/session/LocalStrings.properties >>>> Fri Jun 3 22:13:09 2011 >>>> @@ -27,6 +27,8 @@ JDBCStore.checkConnectionDBClosed=The da >>>> JDBCStore.checkConnectionDBReOpenFail=The re-open on the database failed. >>>> The database could be down. >>>> JDBCStore.checkConnectionSQLException=A SQL exception occurred {0} >>>> JDBCStore.checkConnectionClassNotFoundException=JDBC driver class not >>>> found {0} >>>> +JDBCStore.wrongDataSource=Can't open JNDI DataSource [{0}] >>>> +JDBCStore.missingDataSourceName=No valid JNDI name was given. >>>> managerBase.createRandom=Created random number generator for session ID >>>> generation in {0}ms. >>>> managerBase.createSession.ise=createSession: Too many active sessions >>>> managerBase.sessionTimeout=Invalid session timeout setting {0} >>>> >>>> Modified: tomcat/trunk/webapps/docs/changelog.xml >>>> URL: >>>> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1131263&r1=1131262&r2=1131263&view=diff >>>> ============================================================================== >>>> --- tomcat/trunk/webapps/docs/changelog.xml (original) >>>> +++ tomcat/trunk/webapps/docs/changelog.xml Fri Jun 3 22:13:09 2011 >>>> @@ -69,6 +69,11 @@ >>>> without error. (markt) >>>> </fix> >>>> <fix> >>>> + <bug>51264</bug>: Allow the JDBC persistent session store to use a >>>> + JNDI datasource to define the database in which sessions are >>>> persisted. >>>> + Patch provided by Felix Schumacher. (markt) >>>> + </fix> >>>> + <fix> >>>> <bug>51274</bug>: Add missing i18n strings in >>>> PersistentManagerBase. >>>> Patch provided by Eiji Takahashi. (markt) >>>> </fix> >>>> >>>> Modified: tomcat/trunk/webapps/docs/config/manager.xml >>>> URL: >>>> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/manager.xml?rev=1131263&r1=1131262&r2=1131263&view=diff >>>> ============================================================================== >>>> --- tomcat/trunk/webapps/docs/config/manager.xml (original) >>>> +++ tomcat/trunk/webapps/docs/config/manager.xml Fri Jun 3 22:13:09 2011 >>>> @@ -356,6 +356,13 @@ >>>> session table.</p> >>>> </attribute> >>>> >>>> + <attribute name="dataSourceName" required="false"> >>>> + <p>Name of the JNDI resource for a JDBC DataSource-factory. If this >>>> option >>>> + is given and a valid JDBC resource can be found, it will be used >>>> and any >>>> + direct configuration of a JDBC connection via >>>> <code>connectionURL</code> >>>> + and <code>driverName</code> will be ignored.</p> >>>> + </attribute> >>>> + >>>> <attribute name="driverName" required="true"> >>>> <p>Java class name of the JDBC driver to be used.</p> >>>> </attribute> >>>> >>>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >>>> For additional commands, e-mail: dev-h...@tomcat.apache.org >>>> >>>> >>> >>> >>> >> >> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
