You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacopo Cappellato <ja...@hotwaxmedia.com> on 2013/01/10 10:06:07 UTC
Re: svn commit: r1431191 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java
Jacques,
please see inline:
On Jan 10, 2013, at 6:32 AM, jleroux@apache.org wrote:
> Author: jleroux
> Date: Thu Jan 10 05:32:48 2013
> New Revision: 1431191
>
> URL: http://svn.apache.org/viewvc?rev=1431191&view=rev
> Log:
> In "Memory leak due to transaction management using DBCP and MySQL" https://issues.apache.org/jira/browse/OFBIZ-5122 Jose Manuel Vivó Arnal reported that I missed to apply the patch from OFBIZ-2599, just handled the DBPC part, and Philippe Mouawad did not notice.
>
> OK, so it was just a matter of replacing
> PoolableConnectionFactory factory = new PoolableConnectionFactory(xacf, pool, null, null, true, true);
> by
> PoolableManagedConnectionFactory factory = new PoolableManagedConnectionFactory(xacf, pool, null, null, true, true);
This is minor but it would be better to write:
PoolableConnectionFactory factory = new PoolableManagedConnectionFactory(xacf, pool, null, null, true, true);
>
> The rest are only imports cleaning.
>
> Too bad we did not notice that for 3.5 years. It seems it does not affect servers with plenty of memory, or updated often enough. But I recommend to upgrade of course...
Are we sure we can backport this fix to all the release branches as you did? From the history of comments in OFBIZ-2599 I see that there are some prerequisites for the version of DBCP: did you check if they are satisfied by the jars in 10.04 and 11.04?
Regards,
Jacopo
>
> Modified:
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java
>
> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java?rev=1431191&r1=1431190&r2=1431191&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java (original)
> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java Thu Jan 10 05:32:48 2013
> @@ -18,11 +18,22 @@
> *******************************************************************************/
> package org.ofbiz.entity.connection;
>
> +import java.sql.Connection;
> +import java.sql.Driver;
> +import java.sql.SQLException;
> +import java.util.HashMap;
> +import java.util.Map;
> +import java.util.Properties;
> +
> +import javax.transaction.TransactionManager;
> +
> +import javolution.util.FastMap;
> +
> import org.apache.commons.dbcp.ConnectionFactory;
> import org.apache.commons.dbcp.DriverConnectionFactory;
> -import org.apache.commons.dbcp.PoolableConnectionFactory;
> import org.apache.commons.dbcp.managed.LocalXAConnectionFactory;
> import org.apache.commons.dbcp.managed.ManagedDataSource;
> +import org.apache.commons.dbcp.managed.PoolableManagedConnectionFactory;
> import org.apache.commons.dbcp.managed.XAConnectionFactory;
> import org.apache.commons.pool.impl.GenericObjectPool;
> import org.ofbiz.base.util.Debug;
> @@ -32,16 +43,6 @@ import org.ofbiz.entity.datasource.Gener
> import org.ofbiz.entity.transaction.TransactionFactory;
> import org.w3c.dom.Element;
>
> -import javax.transaction.TransactionManager;
> -import java.sql.Connection;
> -import java.sql.Driver;
> -import java.sql.SQLException;
> -import java.util.HashMap;
> -import java.util.Map;
> -import java.util.Properties;
> -
> -import javolution.util.FastMap;
> -
> /**
> * DBCPConnectionFactory
> */
> @@ -146,7 +147,7 @@ public class DBCPConnectionFactory imple
>
>
> // create the pool object factory
> - PoolableConnectionFactory factory = new PoolableConnectionFactory(xacf, pool, null, null, true, true);
> + PoolableManagedConnectionFactory factory = new PoolableManagedConnectionFactory(xacf, pool, null, null, true, true);
> factory.setValidationQuery("select 1 from entity_key_store where key_name = ''");
> factory.setDefaultReadOnly(false);
>
>
>
Re: svn commit: r1431191 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectio
nFactory.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Jacopo Cappellato wrote:
> Jacques,
>
> please see inline:
>
> On Jan 10, 2013, at 6:32 AM, jleroux@apache.org wrote:
>
>> Author: jleroux
>> Date: Thu Jan 10 05:32:48 2013
>> New Revision: 1431191
>>
>> URL: http://svn.apache.org/viewvc?rev=1431191&view=rev
>> Log:
>> In "Memory leak due to transaction management using DBCP and MySQL" https://issues.apache.org/jira/browse/OFBIZ-5122 Jose Manuel
>> Vivó Arnal reported that I missed to apply the patch from OFBIZ-2599, just handled the DBPC part, and Philippe Mouawad did not
>> notice.
>>
>> OK, so it was just a matter of replacing
>> PoolableConnectionFactory factory = new PoolableConnectionFactory(xacf, pool, null, null, true, true);
>> by
>> PoolableManagedConnectionFactory factory = new PoolableManagedConnectionFactory(xacf, pool, null, null, true, true);
>
> This is minor but it would be better to write:
>
> PoolableConnectionFactory factory = new PoolableManagedConnectionFactory(xacf, pool, null, null, true, true);
OK, I can change that in trunk
>>
>> The rest are only imports cleaning.
>>
>> Too bad we did not notice that for 3.5 years. It seems it does not affect servers with plenty of memory, or updated often
>> enough. But I recommend to upgrade of course...
>
> Are we sure we can backport this fix to all the release branches as you did? From the history of comments in OFBIZ-2599 I see
> that there are some prerequisites for the version of DBCP: did you check if they are satisfied by the jars in 10.04 and 11.04?
Yes, commons-dbcp-1.4.jar is present since R10.04 (also else it would not have compiled ;o)
The change has been introduced since 1.3: https://issues.apache.org/jira/browse/DBCP-294
Jacques
> Regards,
>
> Jacopo
>
>>
>> Modified:
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java
>>
>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java?rev=1431191&r1=1431190&r2=1431191&view=diff
>> ============================================================================== ---
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java (original) +++
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java Thu Jan 10 05:32:48 2013 @@ -18,11
>> +18,22 @@ *******************************************************************************/
>> package org.ofbiz.entity.connection;
>>
>> +import java.sql.Connection;
>> +import java.sql.Driver;
>> +import java.sql.SQLException;
>> +import java.util.HashMap;
>> +import java.util.Map;
>> +import java.util.Properties;
>> +
>> +import javax.transaction.TransactionManager;
>> +
>> +import javolution.util.FastMap;
>> +
>> import org.apache.commons.dbcp.ConnectionFactory;
>> import org.apache.commons.dbcp.DriverConnectionFactory;
>> -import org.apache.commons.dbcp.PoolableConnectionFactory;
>> import org.apache.commons.dbcp.managed.LocalXAConnectionFactory;
>> import org.apache.commons.dbcp.managed.ManagedDataSource;
>> +import org.apache.commons.dbcp.managed.PoolableManagedConnectionFactory;
>> import org.apache.commons.dbcp.managed.XAConnectionFactory;
>> import org.apache.commons.pool.impl.GenericObjectPool;
>> import org.ofbiz.base.util.Debug;
>> @@ -32,16 +43,6 @@ import org.ofbiz.entity.datasource.Gener
>> import org.ofbiz.entity.transaction.TransactionFactory;
>> import org.w3c.dom.Element;
>>
>> -import javax.transaction.TransactionManager;
>> -import java.sql.Connection;
>> -import java.sql.Driver;
>> -import java.sql.SQLException;
>> -import java.util.HashMap;
>> -import java.util.Map;
>> -import java.util.Properties;
>> -
>> -import javolution.util.FastMap;
>> -
>> /**
>> * DBCPConnectionFactory
>> */
>> @@ -146,7 +147,7 @@ public class DBCPConnectionFactory imple
>>
>>
>> // create the pool object factory
>> - PoolableConnectionFactory factory = new PoolableConnectionFactory(xacf, pool, null, null, true, true);
>> + PoolableManagedConnectionFactory factory = new PoolableManagedConnectionFactory(xacf, pool, null, null, true, true);
>> factory.setValidationQuery("select 1 from entity_key_store where key_name = ''");
>> factory.setDefaultReadOnly(false);