You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/06/04 00:13:10 UTC

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

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


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

Posted by Mark Thomas <ma...@apache.org>.
On 03/06/2011 23:41, Tim Funk wrote:
> If NamingExceptionOccurs - should this instead rethrow a SqlException
> instead of letting the logic keep going? Otherwise wouldn't a SqlException
> be thrown later in the method?

Don't think so. If the lookup is invalid dataSource will be null so it
should fall back to using the non-pooled settings (which may also fail)

Mark

> 
> -Tim
> 
> On Fri, Jun 3, 2011 at 6:13 PM, <ma...@apache.org> wrote:
> 
>> 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
>> @@ -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;
>> +        }
>> +
>>
> 




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


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

Posted by Tim Funk <fu...@apache.org>.
If NamingExceptionOccurs - should this instead rethrow a SqlException
instead of letting the logic keep going? Otherwise wouldn't a SqlException
be thrown later in the method?

-Tim

On Fri, Jun 3, 2011 at 6:13 PM, <ma...@apache.org> wrote:

> 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
> @@ -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;
> +        }
> +
>

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

Posted by Mark Thomas <ma...@apache.org>.
On 07/06/2011 13:22, Felix Schumacher wrote:
> Am Dienstag, den 07.06.2011, 10:34 +0100 schrieb Mark Thomas:
>> On 06/06/2011 21:40, Felix Schumacher wrote:

<snip/>

>>> But I would gladly provide a patch where connections are returned to the
>>> pool. 
>>
>> That would be great. Thanks.
> I have attached it, but I could also upload it to the original bugzilla
> entry.

Please do. It is easy to forget stuff attached to messages on the dev
list. Also, please re-open the issue - that way it won't get forgotten.

> I also corrected one of my previous error messages, which contained a
> single-tick, which was not so good.

Great.

Mark

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


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

Posted by Felix Schumacher <fe...@internetallee.de>.
Am Dienstag, den 07.06.2011, 10:34 +0100 schrieb Mark Thomas:
> 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.
There would be one other usage of jndi-resources, that would be to hide
password and db-user from the context.

> 
> >  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.
done.

> 
> >  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.
I have attached it, but I could also upload it to the original bugzilla
entry.

I also corrected one of my previous error messages, which contained a
single-tick, which was not so good.

Felix
> 
> 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-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
> 


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

Posted by Mark Thomas <ma...@apache.org>.
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-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


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

Posted by Felix Schumacher <fe...@internetallee.de>.
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
 2. we would have to close the prepared statements in case of pooled
connections
 3. the connection is not really abandoned, so one could argue, that you
should run the pool with removeAbandoned set to 'false'

But I would gladly provide a patch where connections are returned to the
pool. 

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-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


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

Posted by Mark Thomas <ma...@apache.org>.
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.

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-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


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

Posted by Keiichi Fujino <kf...@apache.org>.
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?


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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>



-- 
Keiichi.Fujino

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