tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1131263 - in /tomcat/trunk: java/org/apache/catalina/session/JDBCStore.java java/org/apache/catalina/session/LocalStrings.properties webapps/docs/changelog.xml webapps/docs/config/manager.xml
Date Tue, 07 Jun 2011 09:34:47 GMT
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  <markt@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-unsubscribe@tomcat.apache.org
>>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 


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


Mime
View raw message