On 22/08/2012 13:51, Phil Steitz wrote:
> Any feedback on the patch? Should I go ahead and apply the [pool]
> patch?
Sorry for the very late review. Some comments/questions on the pool2
part (I haven't looked at DBCP):
- Why not add abandoned support to GKOP as well as GOP?
- I wasn't convinced at first about separating standard and abandoned
config. It is growing on me but I'd be interested in hearing your
reasons for this.
- What is the reasoning behind the tests:
- getNumIdle() < 2
- getNumActive() > getMaxTotal() - 3
in borrowObject()? It would be good to document these reasons in a
comment.
- I think an explicit state of PooledObjectState.ABANDONED would be
clearer / easier to work with (see below).
- There are various timing issues/questions in removeAbandoned()
- what is the lock on allObjects meant to achieve?
- an object may be returned and re-borrowed after being added to
"remove" and before it is invalidated
- abandonedConfig should be in the config section not the JMX section
My instinct is that a checkAbandoned() method on PooledObject that
changes the state to PooledObjectState.ABANDONED and returns true if it
does this is likely to be an easier starting point for a thread-safe
solution rather than separating the timing check and the state change.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
