commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: [pool][dbcp] POOL-229 (move AbandonedObjectPool to pool)
Date Fri, 24 Aug 2012 14:42:42 GMT
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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message