Hello, A common case we saw are missing keep alive with the oracle driver and a hanging read (due to destroyed connection state of firewall) or vip failover of sql server. Besides close we also tried rollback which is also affected of most drivers synchronized blocks.
-- https://Bernd.eckenfels.net ________________________________ Von: Phil Steitz <phil.ste...@gmail.com> Gesendet: Sunday, August 30, 2020 8:27:04 PM An: dev@commons.apache.org <dev@commons.apache.org> Betreff: Re: [dbcp][pool] Use abort instead of close for abandoned connections? On 8/29/20 9:58 PM, Bernd Eckenfels wrote: > We have a pool implementation where we had to call abort especially for > revoked long running transactions, since they regularly have been locked up > in a connection wide synchronized in the close. However we only use it for > that case and try to close the objects after the abort anyway. We don’t use > it to close connections when they are only idle or too old - owned by the > pool. Right. I was thinking only of the abandoned case. Interestingly, I have not been able to replicate the deadlock that started me thinking about this. I have always worried though about problems with close hanging or deadlocking in the abandoned use case where what is going on is a transaction is stuck or the abandoned timeout is just set too low. Phil > > It seems to be however still a good idea to run the abort/close in a separate > task as it can also take ages for example when TLS close handshakes hang (we > did try to reduce the connection timeout before but that also can be blocked > before the abort) > > Gruss > Bernd > -- > https://Bernd.eckenfels.net > ________________________________ > Von: Gary Gregory <garydgreg...@gmail.com> > Gesendet: Saturday, August 29, 2020 10:02:42 PM > An: Commons Developers List <dev@commons.apache.org> > Betreff: Re: [dbcp][pool] Use abort instead of close for abandoned > connections? > > On Sat, Aug 29, 2020 at 2:41 PM Phil Steitz <phil.ste...@gmail.com> wrote: > >> On 8/29/20 11:03 AM, Gary Gregory wrote: >>> On Sat, Aug 29, 2020 at 1:35 PM Phil Steitz <phil.ste...@gmail.com> >> wrote: >>>> A pool-related deadlock was reported recently in [1] to tomcat-user. >>>> The OP was using a different pool, but it looks to me like the same >>>> deadlock could happen with dbcp. The source is arguably a driver bug, >>>> but in [2], the driver maintainer makes the good point that to avoid the >>>> problem in [1] (and others like it) it would be better if pool >>>> maintainers used abort instead of close to disconnect "stuck" or >>>> abandoned connections. That makes sense to me. >>>> >>>> To implement this in dbcp, we would need to either always use abort >>>> instead of close when closing physical connections (a bad idea, IMO, as >>>> it could mess up counters due to async execution and add overhead) >>> I agree that changing the happy path from close() to abort() is a bad >> idea; >>> especially since the Connection#close() Javadoc contract releases >> database >>> resources while abort() does not or is vague about it (at least in the >> Java >>> 8 Javadoc). >> Yes, bad idea to do that, I think. >> >>> >>>> or >>>> modify pool to call a different factory method than destroy when >>>> triggering close-abandoned. I think the latter makes sense and it could >>>> be useful for other pool clients to be able to this kind of thing as >> well. >>> This means that we would treat abandoned connections like a real resource >>> leak in the sense that (if) abort() leaks database resources, then the >> DBA >>> would have to deal with that. >> Yeah, unless the driver does it cleanly. The other thing to note here >> is that abort is async and is supposed to allow ops in progress to >> complete, etc. That is kinder to clients but technically allows >> maxActive to exceed the cap while the abort is in progress. >> >>> I think formally surfacing an abort path vs. the current close/destroy is >>> OK since it reflects what JDBC allows. If that means trickling the >> concept >>> down to Pool, then that's OK as well IMO. This could be generically >>> surfaced with a "close mode" enum passed to close/destroy APIs. This is >>> similar to what we did in HttpCore's CloseMode(IMMEDIATE vs GRACEFUL): >>> >> https://javadoc.io/static/org.apache.httpcomponents.core5/httpcore5/5.0.1/org/apache/hc/core5/io/CloseMode.html >> >> Exactly. I think the abandoned-destroy is legitimately different from >> the other destroy use cases and I could see other kinds of pools >> (sockets, jms, etc) benefiting from having the ability to handle destroy >> differently for abandoned (or other) cases. >> >>> >>>> What I propose is that we make these change to pool: >>>> >>>> 1. Add a new method to PooledObjectFactory called "abandonObject" with >>>> default implementation delegating to destroyObject. >>>> >>> That or overload destroyObject with a destroyObject(CloseMode) would be >>> easier to find. >> Yes, I think that would be better. That raises the interesting question >> of what are the CloseModes. All we need for this issue is normal vs >> abandoned, but I can see value in providing fuller context to >> factories. So if we are doing it, why not something like: >> >> invalidated >> abandoned >> evicted >> excess idle >> failed validation >> failed activation >> failed passivation >> clearing pool >> > I think a YAGNI-style approach would work well here by starting with 2 > modes: one to support the current behavior (default) and another to support > the new behavior backed by Connection#abort() in DBCP. > A third "escape hatch" mode could see passing in a lamba I suppose. I > really think we should start by just focusing on the one issue at hand > before we enlarge the solution. > > 2c, > Gary > > >> Phil >> >> >> >>> Gary >>> >>> >>>> 2. Change the logic in removeAbandoned to call the new factory method >>>> instead of destroyObject >>>> >>>> And then in dbcp: >>>> >>>> * Add implementation of abandonObject in PoolableConnectionFactory to >>>> call abort. >>>> >>>> wdyt? >>>> >>>> Phil >>>> >>>> [1] >>>> >>>> >> https://lists.apache.org/thread.html/r7095d641bbc8db0a526d2d9a18684202347a747cf0e315a4a50d2001%40%3Cusers.tomcat.apache.org%3E >>>> [2] https://bugs.mysql.com/bug.php?id=64954 >>>> >>>> >>>> >> --------------------------------------------------------------------- >> 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
