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. The
>>>>>> 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.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
