commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [pool][dbcp] Ongoing saga of setFactory
Date Sun, 03 Jul 2011 21:43:26 GMT
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.

Phil



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


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


Mime
View raw message