Hi,
I wouldn't keep Oliver from providing a patch wrt. his original request,
if he'd like to ;-)
Though if not, I can possibly also provide something - maybe next week.
I'm only unsure about which of the connection pool settings exactly should
be externalized into properties, and about the isolation level setting (maybe
one
could make this configurable as well, with default READ_COMMITTED?).
Cheers,
Guido
Gesendet: Freitag, 06. Dezember 2013 um 15:38 Uhr
Von: "Oliver Wulff" <owu...@talend.com>
An: "dev@syncope.apache.org" <dev@syncope.apache.org>
Betreff: RE: BasicDataSource usage in persistence.xml
Will do ;-)
Cheers
Oli
________________________________________
From: Francesco Chicchiriccò [ilgro...@apache.org]
Sent: 06 December 2013 13:22
To: dev@syncope.apache.org
Subject: Re: BasicDataSource usage in persistence.xml
On 06/12/2013 12:49, Guido Wimmel wrote:
> Hi Oliver,
>
> FYI: we use such a setup (with dbcp.BasicDataSource) in our project, thus
> avoiding the need to
> configure a data source in the container. This works fine; also with our own
> unit/integration tests.
>
> We also used the standard MySQL isolation level (REPEATABLE_READ), as we had
> problems with our standard
> MySQL configuration related to Binary Logs ("InnoDB is limited to row-logging
> when transaction isolation level
> is READ_COMMITTED or READ_UNCOMMITTED"). This also works fine, but probably
> the explicit setting of
> READ_COMMITTED as a transaction level was chosen on purpose.
>
> We externalized some more properties of BasicDataSource (testOnBorrow,
> testOnReturn, removeAbandoned,
> initialSize, maxActive, minIdle, removeAbandoned,
> timeBetweenEvictionRunsMillis, minEvictableIdleTimeMillis,
> validationQuery). IMO, at least the size parameters (initialSize, maxActive,
> minIdle) and the validation query
> (which can be database specific) should be configurable via properties.
> BasicDataSource should also probably be a direct
> Maven dependency then.
>
> I'd be +1 for this change, as it would spare us the need of overwriting
> persistenceContext.xml ;-)
Well, what are you still waiting for providing a patch, then? ;-)
>> Von: "Francesco Chicchiriccò" <ilgro...@apache.org>
>>> Hi there
>> Hi Oliver,
>>> Wouldn't it make sense to use a BasicDataSource instead of
>>> DriverManagerDataSource as it is also recommended from Spring:
>>> http://docs.spring.io/spring/docs/current/spring-framework-reference/html/jdbc.html#jdbc-connections
>> Short answer: Yes, why not?
>> Long answer follows.
>>
>> The localDataSource bean in persistenceContext.xml is there for
>> providing a DataSource for non-production usage: unit and integration
>> tests, various development profiles [1], embedded mode [2] and
>> standalone distribution [3].
>>
>> As you can see from syncopeContext.xml, things are arranged so that at
>> first a JNDI lookup for a proper DataSource is performed: this becomes
>> of fundamental importance in production environments, as we suggest [4].
>>
>> Moreover, usage of BasicDataSource implies commons-dbcp dependency,
>> which is currently indirectly pulled in by Activiti
>> (org.activiti:activiti-spring, exactly).
>>
>> Hence, I am +-0 for this change.
>>
>>> Instead of this:
>>>
>>> <bean id="localDataSource"
>>> class="org.springframework.jdbc.datasource.IsolationLevelDataSourceAdapter">
>>> <property name="targetDataSource">
>>> <bean class="org.springframework.jdbc.datasource.DriverManagerDataSource">
>>> <property name="driverClassName" value="${jpa.driverClassName}"/>
>>> <property name="url" value="${jpa.url}"/>
>>> <property name="username" value="${jpa.username}"/>
>>> <property name="password" value="${jpa.password}"/>
>>> </bean>
>>> </property>
>>> <property name="isolationLevelName" value="ISOLATION_READ_COMMITTED"/>
>>> </bean>
>>>
>>>
>>> we can configure this bean:
>>>
>>> <bean id="localDataSource"
>>> class="org.springframework.jdbc.datasource.IsolationLevelDataSourceAdapter">
>>> <property name="targetDataSource">
>>> <bean class="org.apache.commons.dbcp.BasicDataSource"
>>> destroy-method="close">
>>> <property name="driverClassName" value="${jpa.driverClassName}"/>
>>> <property name="url" value="${jpa.url}"/>
>>> <property name="username" value="${jpa.username}"/>
>>> <property name="password" value="${jpa.password}"/>
>>> </bean>
>>> </property>
>>> <property name="isolationLevelName" value="ISOLATION_READ_COMMITTED"/>
>>> </bean>
>> If you like such change, please file an issue and attach a patch there.
>>
>> Regards.
>>
>> [1]
>> http://syncope.apache.org/building.html#More_build_profiles[http://syncope.apache.org/building.html#More_build_profiles]
>> [2]
>> https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+in+embedded+mode[https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+in+embedded+mode]
>> [3]
>> https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+standalone+distribution[https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+standalone+distribution]
>> [4]
>> https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+in+real+environments#RunSyncopeinrealenvironments-Usedatasource[https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+in+real+environments#RunSyncopeinrealenvironments-Usedatasource]
--
Francesco Chicchiriccò
Tirasa - Open Source Excellence
http://www.tirasa.net/[http://www.tirasa.net/]
ASF Member, Apache Syncope PMC chair, Apache Cocoon PMC Member
http://people.apache.org/~ilgrosso/[http://people.apache.org/~ilgrosso/]
