Hi Phil, Starting a new mailing thread is a good idea at this point. I'll just mention: I really do not want to roll back code changes in master. If you want to pick a commit in the past and create a branch 2.x from that, it's fine with me, or tell me, and I'll be happy to create the branch. Then I can switch master to 3.0.
Gary On Mon, Jul 3, 2023, 11:31 Phil Steitz <phil.ste...@gmail.com> wrote: > On Mon, Jul 3, 2023 at 6:41 AM Gary Gregory <garydgreg...@gmail.com> > wrote: > > > On Thu, Jun 29, 2023 at 5:08 PM Phil Steitz <phil.ste...@gmail.com> > wrote: > > > > > > On Thu, Jun 29, 2023 at 11:39 AM Gary Gregory <garydgreg...@gmail.com> > > > wrote: > > > > > > > Great presentation in the video Elliotte. Thanks for sharing the > link. > > > > > > > > > > +1 many thanks. > > > > > > Now back to our hero. Let me pretend to be one of the people in the > > > audience of the video. > > > > > > We have this library that is used by all kinds of "programs" [1] and we > > are > > > not sure how best to a) type our own exceptions (the ones we generate) > > and > > > b) propagate the ones generated by user-supplied object factories. > > Broadly > > > speaking, we have four kinds of exceptions to deal with: > > > > > > 0) User-supplied object factories that the pool uses to perform pooled > > > object lifecycle operations may throw checked or unchecked exceptions. > > > Many of these will be the classic variety you mention in the video - > > > database is dead, etc when the factory is trying to open a physical > > > connection. The smelly "throws Exception" in place now lets us just > pass > > > these all up the stack, but essentially untyped. > > > 1) Pool clients want to do something, but the pool can't do it without > > > violating one of its invariants (most common is a client wants to > borrow > > an > > > object and, after waiting the configured interval, there is no capacity > > to > > > serve). We are inconsistent here. In the out of capacity case, we > throw > > > NoSuchElementException (probably bogus according to your taxonomy), but > > in > > > some cases - e.g. addObject - we silently no-op. > > > 2) An API is abused (e.g. return an object that did not come from the > > pool > > > or try to start borrowing objects before providing a factory) > > > 3) Something happens that should not be possible (indicating some kind > of > > > concurrency bug in [pool] itself or unanticipated craziness from a > > factory > > > or client code) > > > > > > There is a lot of complexity in the 0)-1) cases because some failures > are > > > more important to both the pool and clients than others (for example, > > > factory exceptions thrown in object validation or destruction are > > different > > > from those thrown in object creation). > > > > > > If I understand your advice, it would be: > > > 0) checked if that is what comes from the factory; just propagate > > otherwise > > > 1) checked? Maybe actually no exception - handle via return values or > > > somesuch? By far the most common here is NoSuchElementException. I > > guess > > > it's OK to see pool exhaustion as "environmental" [2], so probably > > checked > > > is actually right > > > 2) unchecked > > > 3) unchecked > > > > > > Assuming I got 0) right at least, the follow-up is how do we get the > > right > > > exceptions passed up the stack? What Gary is proposing is adding a > type > > > parameter at the pool level. Could be that is the best solution, but > > that > > > adds to complexity of the API and I keep wanting it to be defined at > the > > > factory level somehow and ideally have it default to something so users > > who > > > don't want to think about it (maybe because their factories only throw > > > unchecked exceptions) don't have to think about it. > > > > > > I would be remiss not to add a closing (at this point probably > annoying) > > > comment to the effect that this should be a 3.0 discussion :) > > > > I am OK with all of this being for 3.0, which can be as soon as we want. > > > > Thanks, Gary. We should probably end this thread then and start a new one > on 3.0 planning. As a side note, I discovered that the failures reported > to the console but not causing test failure that I saw were real, due to > problems in GKOP reuseCapacity. The OP test case for POOL-391 is > demanding, but we should be able to handle it. It was not causing the test > to fail because the runner does not see failures in spawned threads. My > bad not knowing that. Once I set it up so the failures would fail the > test, they did consistently. I have been working on and off on this over > the last week and I think I am close to a fix. Once I have this, I think > it would be good to roll a 2.12 without the compat break because there are > quite a few bug fixes in the RC. Would you be OK either rolling back the > added type parameter change or moving it to a new 3.0 branch? Or I guess > alternatively, creating a 2.x branch without that change? I can help with > either of these. > > Phil > > > > > Gary > > > > > > > > Phil > > > > > > [1] It is hard for us to understand what is and is not in "our program" > > as > > > library developers. Our code is always running inside someone else's > > > program, often using yet another third party's code. So for example, > in > > a > > > common use case, [pool] is used by [dbcp] which is used by tomcat, with > > > factories specified in a user webapp running in tomcat, wrapped into > > [dbcp] > > > and managed by [pool]. > > > [2] Though many/most actual cases of this in production code end up > being > > > the result of self-DOS due to programming errors. > > > > > > > > > > > Gary > > > > > > > > On Thu, Jun 29, 2023, 10:33 Elliotte Rusty Harold < > elh...@ibiblio.org> > > > > wrote: > > > > > > > > > On Thu, Jun 29, 2023 at 10:10 AM Gilles Sadowski < > > gillese...@gmail.com> > > > > > wrote: > > > > > > > > > > > > Le jeu. 29 juin 2023 à 15:22, Elliotte Rusty Harold > > > > > > <elh...@ibiblio.org> a écrit : > > > > > > > > > > > > > > On Thu, Jun 29, 2023 at 9:07 AM Gilles Sadowski < > > > > gillese...@gmail.com> > > > > > wrote: > > > > > > > > > > > > > > > > Hello. > > > > > > > > > > > > > > > > Le jeu. 29 juin 2023 à 14:44, Gary Gregory < > > garydgreg...@gmail.com > > > > > > > > > > a écrit : > > > > > > > > > > > > > > > I agree with the second part assuming the *current* Java > > > > > > > > best practices, not because of old APIs that are here to stay > > > > > > > > only because of infinite backwards compatibility, and not > > > > > > > > because they are deemed perfect. > > > > > > > > > > > > > > > > > > > > > Best practices on this haven't changed since Java 1.0 (Possibly > > Java > > > > > > > 1.0 alpha 3 if I recall versions correctly). > > > > > > > > > > > > > > Checked exceptions: all errors arising from external input to > the > > > > > program. > > > > > > > Runtime exceptions: program bugs > > > > > > > > > > > > A pile of arguments (tally is largely against *any* use of > checked > > > > > exceptions): > > > > > > > > > > > > > > > > > > https://ohadshai.medium.com/its-almost-2020-and-yet-checked-exceptions-are-still-a-thing-bfaa24c8997e > > > > > > > > > > tldr; he's wrong. More details: > > > > > https://www.youtube.com/watch?v=rJ-Ihh7RNao&t=419s > > > > > > > > > > > > > > > > > > > I don't know which applies here, but 99% of the time this is > all > > you > > > > > > > need to consider to figure out whether a checked or runtime > > exception > > > > > > > is called for. There are indeed a lot of old broken APIs that > > don't > > > > > > > follow these simple rules, > > > > > > > > > > > > The rules which you stated are far from simple because "external" > > > > > > depends on the POV (more below). > > > > > > > > > > > > Commons components like [RNG], [Geometry], [Numbers], [Math] > > > > > > only throw unchecked exceptions. > > > > > > > > > > Libraries you link to are not external to the program. Runtime > > > > > exceptions might be appropriate here. > > > > > > > > > > > The only rule that ever made sense to me is from J. Bloch's > > "Effective > > > > > > Java": > > > > > > ---CUT--- > > > > > > Use checked exceptions for recoverable conditions and runtime > > > > > > exceptions for programming errors > > > > > > ---CUT--- > > > > > > > > > > This is about the only thing in that book where I disagree with > > Bloch. > > > > > At the very least this is not said as well as it needs to be. > > > > > Recoverable vs unrecoverable distinguishes Errors from Exceptions, > > not > > > > > runtime exceptions from checked exceptions. You can interpret that > > > > > sentence to mean "bugs are not recoverable short of fixing the > code" > > > > > in which case sure, runtime exceptions are not recoverable. > > > > > > > > > > > For example, *all* precondition failures are, from the POV of the > > > > > > called function, a programming error (akin to NPE or IAE). > > > > > > > > > > This is correct. NPEs and IAEs are programming errors. > > > > > > > > > > > Whether the root cause is an "internal" or "external" error is > > > > irrelevant > > > > > > at the point where the issue is detected (where the runtime > > exception > > > > > > just conveys that "the call was inappropriate"). > > > > > > > > > > It's not about the root cause. It's about the immediate cause. > > Passing > > > > > null or another illegal argument to a method that isn't ready to > > > > > receive it is a programming error, i.e. a bug, and therefore these > > are > > > > > properly runtime exceptions. > > > > > > > > > > > > and it's never fun when a system goes down > > > > > > > in production because some random JSON yahoo thought checked > > > > > > > exceptions were "icky". > > > > > > > > > > > > A bug of the application, by the "absent-minded" developer > > mentioned > > > > > > in the previous mail. > > > > > > In Java, nothing prevents the throwing of a runtime > > > > > > exception; the developer *always* must enclose "foreign" code in > a > > > > > > "try/catch" block, to protect the system. There is no need for > > checked > > > > > > exceptions just to remember this (really) simple rule. > > > > > > > > > > Had the JSON library properly used checked exceptions, the > programmer > > > > > would not have been able to compile code that contained this > mistake. > > > > > The reason we have strong static typing, unit tests, and checked > > > > > exceptions is that programmers are imperfect humans. Let the > machines > > > > > do the work that machines are good at, including checking whether > > > > > errors are properly handled. > > > > > > > > > > > > Lately I've been working in Python, and error handling practice > > in > > > > > > > this community is abominable. The lack of checked exceptions > is a > > > > > > > large reason why Python code is expected to be unreliable and > > Python > > > > > > > programs routinely dump stack. > > > > > > > > > > > > I've heard that it's rather the result of "script"-oriented > > design, or > > > > > lack > > > > > > thereof... :-) > > > > > > > > > > There's more than one reason. :-) > > > > > > > > > > > > > > > -- > > > > > Elliotte Rusty Harold > > > > > elh...@ibiblio.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 > > > > >
