On 23/09/2020 01:29, Phil Steitz wrote: > > On 9/14/20 10:10 AM, Gary Gregory wrote: >> On Mon, Sep 14, 2020 at 1:07 PM Phil Steitz <phil.ste...@gmail.com> >> wrote: >>> >>> On 9/14/20 9:36 AM, Gary Gregory wrote: >>>> This feature is now in Pool master. I will prepare an RC soon if you >>>> all think we are good to go so we can then move on to DBCP. >>> I am still working on testing this in the DBCP use case. Probably best >>> to wait a little for others to review and for me to get the DBCP change >>> tested against current pool sources. I should be able to finish that >>> this weekend. > > I implemented changes in DBCP, based on recently committed changes in > pool. I made a few decisions that I would appreciate feedback on. > > 1. The JDBC abort method requires an Executor as actual parameter. I > added a FixedThreadPool executor with a max of 3 threads to > PoolableConnectionFactory for this purpose. Given that this operation > might hang sometimes, I thought it best to allow more than a single > thread. I guess it could be configurable, but 3 seemed a reasonable > choice. I copied the custom thread factory used by pool's EvictionTimer > to source daemon threads based on ccl for this. I then added a close() > method to PCF that closes the executor and modified BasicDataSource > close to call this. > > 2. I used p.getObject().getInnermostDelegate().abort(executor) in PCF > destroyObject when abandoned mode is passed in rather than just > getObject().abort as destroy invokes close (I wonder if that is a bug?) > > 3. I changed JdbcBridge#abort to remove the checkOpen() check. I was > getting exceptions in my concurrency tests where connections were being > closed but then considered abandoned. This is a legit race that abort > will generally handle fine and I don't see the need to throw.
All looks reasonable to me. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
