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.
Gary On Tue, Sep 8, 2020 at 9:16 AM Gary Gregory <garydgreg...@gmail.com> wrote: > > FWIW, I like the name "DestroyMode" because it matches the "destroy" in the > method name. > > Gary > > On Mon, Sep 7, 2020, 19:08 Gary Gregory <garydgreg...@gmail.com> wrote: >> >> On Mon, Sep 7, 2020 at 6:02 PM Phil Steitz <phil.ste...@gmail.com> wrote: >> > >> > >> > On 9/3/20 2:44 AM, Mark Thomas wrote: >> > > On 31/08/2020 01:05, Phil Steitz wrote: >> > > >> > > <snip/> >> > > >> > >> If others agree it is a good idea for dbcp, I can do it. I can see the >> > >> argument that its better to stay with close() even for abandoned and I >> > >> have not been able to get the deadlock to happen, so I would like to >> > >> wait a bit and allow others to weigh in. Similarly for pool, I would >> > >> like to get more community feedback before adding to the interface. >> > > Hmm. >> > > >> > > On one hand if the driver deadlocks I don't see how that can be anything >> > > other than a driver bug. If multiple code paths obtain multiple locks >> > > then those code paths must always obtain those locks in the same order. >> > > Anything else is a bug that is likely to result in a deadlock in a >> > > multithreaded environment. >> > > >> > > On the other hand, it could be argued that the situation only arises >> > > when an application doesn't correctly return connections to the pool >> > > and/or keeps them for too long and/or doesn't configure the pool >> > > correctly for their usage pattern. >> > > >> > > The approach of adding >> > > >> > > PooledObjectFactory.destroyObject(PooledObject,CloseMode) >> > > >> > > where CloseMode is an Enum with two values looks reasonable to me. >> > >> > >> > I have started to work on the [pool] changes for this. I want to check >> > two things before completing the PR: >> > >> > 1. "Close" is a [dbcp] concept which does not make sense for all pool >> > factories, so I am going to name the enum "DestroyMode" and the two >> > modes, "Default" and "Abandoned". That leaves room for other modes like >> > "Evicted" or "Invalid" later. >> > >> > 2. Speaking of later, technically adding modes will not break binary >> > compatibility. Are we going to be OK adding outside a major release? >> > If the answer is no, I might argue to include the other natural modes now. >> >> Yes, IMO, it is OK to add enum values in a minor release since it does >> not break binary (or source) compatibility. >> >> Gary >> >> > >> > Phil >> > >> > > >> > > I do agree that abort() should only be used in the case of abandoned >> > > connections. >> > > >> > > Mark >> > > >> > > --------------------------------------------------------------------- >> > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> > > For additional commands, e-mail: dev-h...@commons.apache.org >> > > >> > >> > --------------------------------------------------------------------- >> > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> > For additional commands, e-mail: dev-h...@commons.apache.org >> > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
