commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <>
Subject Re: [pool][dbcp] Ongoing saga of setFactory
Date Thu, 07 Jul 2011 19:07:37 GMT
On 03/07/2011 22:43, Phil Steitz wrote:
> On 7/3/11 12:32 PM, Mark Thomas wrote:
>> On 26/06/2011 01:05, Phil Steitz wrote:
>>> On 6/25/11 4:28 PM, Mark Thomas wrote:
>>>> On 17/06/2011 09:02, Mark Thomas wrote:
>>>>> On 17/06/2011 00:32, Gary Gregory wrote:
>>>>>> I think 2.0 is the opportunity to do this right. Almost like we were
>>>>>> designing this from scratch.
>>>>>> Making the factory an invariant of the pool sounds good.
>>>>>> Otoh If a setFactory method exists it should be implemented fully.
>>>>>> throw an exception impl is pretty "smelly".
>>>>> I agree that it is smelly. I'm not sure we can change this without a
>>>>> huge amount of work in DBCP.
>>>> I think largely due to the clean-up Phil has already done in DBCP, this
>>>> is now manageable. I have just done this for GOP and will do the same
>>>> for GKOP next (exact timing TBD).
>>> +1 - definitely better.  Will test and review and if I get to if
>>> before you, handle GKOP.   Thanks for doing this.
>> Looking at this, I can't see a way of doing this without introducing a
>> new interface StatementPoolFactory that extends KeyedObjectPoolFactory
>> and adds a setPoolingConnection() method (or something along those lines
>> anyway).
>> The names may well change as the refactoring evolves and we add generics
>> support to DBCP but this should do for now.
> Sorry I have not gotten to this.  I was thinking something a little
> different.  What needs to be set is the GKOP factory, which is part
> of its config.  The current BDS setup is arguably smelly, since it
> creates a partial config for the KeyedObjectPoolFactory (omitting
> the factory).  Extending as you have above works around this split. 
> Setting the pooling connection is really setting the factory.  That
> will work, but so might the following:
> 0) Get rid of the partially configured KeyedObjectPoolFactory in BDS
> altogether.  The only variable part of the config is
> maxOpenPreparedStatements.  Pass this along to PCF (and a
> poolStatements flag).
> 1) Create the full config for the statement pool GKOP in PCF makeObject.
> 2) Add a setStatementPool method to PoolingConnection, so this can
> be constructed without the statement pool.
> 3) Include the factory ( = the newly constructed PC) in the config
> used to create the statement pool in PCF makeObject
> So makeObject in PCF looks something like:
> Connection conn = _connFactory.createConnection();
> if (conn == null) {
>     throw new IllegalStateException("Connection factory returned
> null from createConnection");
> }
> initializeConnection(conn);
> if(poolStatements) {
>     conn = new PoolingConnection(conn);
>     GenericKeyedObjectPoolConfig config =
>                     new GenericKeyedObjectPoolConfig();
>     config.setMaxTotalPerKey(-1);
>     config.setWhenExhaustedAction(WhenExhaustedAction.FAIL);
>     config.setMaxWait(0);
>     config.setMaxIdlePerKey(1);
>     config.setMaxTotal(maxOpenPreparedStatements);
>     config.setFactory((PoolingConnection)conn);
>     KeyedObjectPool stmtPool = new GenericKeyedObjectPool(config);
>     conn.setStatementPool(stmtPool);
>  }
> return new PoolableConnection(conn,_pool,_config);
> I have not tested the above setup, but it has the advantage that the
> pool configuration is defined fully in one place and it is similar
> to what we now have for PCs vis a vis the GOP.

Yep, that works. Changes committed.


To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message