CRUD API throws Exception?

[Alex Snaps]

Hey,

We're getting to the meat of the API slowly. I think the ground work is now in a state where we should get that moving. That's why, according to the prioritized board at 

https://waffle.io/ehcache/ehcache3 the next task up is issue #23: Add CRUD API 

I'd be in favor of having get/put/remove et al declare to throw checked exceptions. We'd then need to figure out what kinda exceptions. There is `PermanentPartialCacheAccessException extends CacheAccessException` in the repo for now. It comes from the initial POC we, Chris & I, did years back. It'd be nice to if we keep that approach or not... 

Also, this would dictate pretty much what the `Store<K, Value<V>>` interface would look like.

Alex

--

Alex Snaps <alex....@gmail.com>
Principal Software Engineer - Terracotta
http://twitter.com/alexsnaps
http://about.me/alexsnaps

http://www.devoxx4kids.org/quebec/ 

[Chris Dennis]

Right, so yes I'm very pro having two interfaces: <Cache> and <UncheckedCache>, where everything normally returns <Cache> which throws checked exceptions, and then have a static method: UncheckedCache unchecked(Cache cache) to get a version that throws only runtime exceptions.  That way if you want no enforced error checking you can have it, but most explicitly choose it.  As regards the "PermanentPartialCacheAccessException" the idea was to use these top level exception types to indicate something about the scope of the failure:

Permanent == This failure is eternal (the system will not recover)

Partial == This failure is not generalized to the whole cache (other operations might still work).

This seemed like it might be useful information on which recovery/handling behavior could be based.  Basically I want the type system to naturally map to the sets of reasonable handling behavior.

How much sense does this make?

Chris

[Aurelien Broszniowski]

Just to understand, what is the advantage of having two interfaces ? Shouldn't a single interface throw both unchecked and checked exceptions? Checked exceptions for problems that are possible to occur (thus forcing the user to catch the likely erroneous states) and unchecked exceptions in certain cases where the API is wrongly used, e.g. passing a null argument and then throwing a NullPointerException?
In other words, shouldn't an API follow this rule : Permanent error -> Unchecked exception, Partial/Recoverable error -> checked exception ?

[Mathilde Rigabert Lemée]

I agree with Aurelien. That's KISS.

The idea behind the UncheckedCache unchecked(Cache cache) may look like a nice feature because it will simplify the life of the user who do not want to think about all the partial cases but that's on the user plate, not our and it complicates the API which should stay the simplest.

Not a big fan of having checked exception on a get, put or remove either. If the cache is not accessible, it might be an unchecked exception. Have to think a little more about that, but ... I can see why this checked exceptions will be useful (when everything is distributed I guess ?) but it depends what you have in mind for ehcache. I think it really complicates the API to have all this checked exceptions when 99% of the use case will be a local cache. I may miss a case.

On Mon, Sep 8, 2014 at 10:12 PM, Aurelien Broszniowski <aur...@gmail.com> wrote:

Just to understand, what is the advantage of having two interfaces ? Shouldn't a single interface throw both unchecked and checked exceptions? Checked exceptions for problems that are possible to occur (thus forcing the user to catch the likely erroneous states) and unchecked exceptions in certain cases where the API is wrongly used, e.g. passing a null argument and then throwing a NullPointerException?
In other words, shouldn't an API follow this rule :  Permanent error -> Unchecked exception, Partial/Recoverable error -> checked exception ?

--
You received this message because you are subscribed to the Google Groups "ehcache-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ehcache-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ehcache-dev/8d6a5641-5af5-4f28-95e3-ec3674ece909%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Alex Snaps]

Not really sure I follow you here, Aurelien:

If a permanent error throws an unchecked exception, do you imply it results from a user error? 

Or do you mean that both "user error" & "permanent error" should throw _some_ unchecked exception... 

Also unsure about what really "keeping it simple, *s'il vous plait*" means here: while I think keeping it simple to the user is desirable, what's simple really? i.e. yes, for a heap only storage, nothing can really go wrong (though arguably that's not even really true), but for any other topology, it gets much more complex... and very quickly:

What when the diskstore fails (even more so if it's async on put)? if the persisted serialized representation doesn't match the class definition on the runtime's classpath? if the remote node, authority over the given Key, isn't accessible? 

While providing a "simple" API to the user seem desirable, entirely hiding the (possible) complexity of the underlying topology would bring it's very own share of trouble. 

It does seem like we have two opposing forces here... though I also think, luckily enough, there is no choice to be made, other than maybe what is the default abstraction to expose to end-users... It seems to me like both are desirable (i.e. an explicit one, as well as a "simple" one). Do we at least agree on that? And if so, what would you argue is the way to get from one to the other? If not, I seem to read the way to do it is to hide all complexity from the user, but how would one wanting to get complete control, get it? ... catch random runtime exceptions? 

To view this discussion on the web visit https://groups.google.com/d/msgid/ehcache-dev/CAOOJetiejMdiyHKqmJfGjpEzfewT1RdOjVjWGnQn%2BuRw%2BAcOOg%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Mathilde Rigabert Lemée]

"While providing a "simple" API to the user seem desirable, entirely hiding the (possible) complexity of the underlying topology would bring it's very own share of trouble. "

I totally agree on the fact that bad things can happen when you add complexity everywhere (async, distributed ...).It just depends for whom you're designing this API at first, because for me, the principal case is a simple heap cache. That's why the default should be a simpler Cache and CheckedCache. The way to go from one to the other will be so costly [for the end users]. It will also complicated a lot the code base too. It brings more problems than help.  I'm not saying here that having a checked cache is not a good feature - just the does it worth the price ? Maybe yes. What I do not want is to see somewhere duplication of methods, one which the check and the other the uncheck for the end-user.

So I'm in favor (by order) :

 having only one unchecked + "but how would one wanting to get complete control, get it? ... catch random runtime exceptions?" I assume that choice. Will imply to have a perfect documentation for the exceptions.

 having 2 - one cache (unchecked) and on CheckedCache . The way to go from one to another (from Chris) is OK. Reminds me synchronisation on collections so that's easy - except for the return params but no choice here. Same than previous for the unchecked cache (documentation/exception).

Regarding the exceptions :

permanent + total =  CacheAccessException ?

permanent + partial = PermanentPartialCacheAccessException ?

no permanent + total = CacheAccessException ()

no permanent + partial = CacheAccessException

I'm OK with the idea of PermanentPartialCacheAccessException but I'm not sure the name to be clear for the end-users. Have nothing to propose right now so ...Something around stability (fatal/unstable/you can try again). Don't know. Can you maybe add a use case of a PermanentPartialCacheAccessException ? 

To view this discussion on the web visit https://groups.google.com/d/msgid/ehcache-dev/CAKux6pY0uP2O7pfVVTAUWUxQPXJsQjSNPU6_T9sE8fOoxO4-Aw%40mail.gmail.com.

[Chris Dennis]

I’d like to rewind things here a little and try and navigate us to the “correct” solution in a more systematic way, top down.  Lets ignore all discussion of the actual exception type hierarchy (checked or not) since I think that’s something we can consider/design later once we have the basic stuff nailed down.  At the highest level there are three solutions open to us:

1. One cache interface that throws checked exceptions on failures.
2. One cache interface that throws unchecked exceptions on failures.
3. Two cache interfaces, one that throws checked exceptions on failures, and the other that throws unchecked exceptions on failures.

I think we are all agreed that solution #1 is unfair to simple cache users as it forces them to handle a bunch of exception cases which they do not (for whatever reason) care about – for me that rules that solution out.

Solution #2 relies on two things:

1. It assumes that we can write and maintain perfect documentation of the “interesting" set of runtime exceptions that can be thrown by the cache.  For interesting you can read “worth handling by the user” aka “would be checked exceptions if we were using checked exceptions”.  In order to maintain this set of exceptions we would in all likelihood use checked exceptions internally so that we can easily track how the useful exceptions flow through our code, and thereby avoid missing any when documenting them.  This seems a little strange in the what’s good for the goose is good for the gander sense.
2. It also relies on users remembering to read the javadoc perfectly, otherwise all our documentation effort is for naught.  The maintenance side of this is particularly onerous, since users will have to reread the documentation with every release, as there is no way of knowing whether new runtime exception throws have been introduced, since the compiler will not warn users about them.

To me these short comings rule out this solution – the task of trying to reliably handle exceptions in this kind of an environment is simply too complex for us to foist on users.

This is why I think solution #3 is the only thing that makes sense.  Rather than trying to find a single interface approach that works for all users, we split the population in half, provide two interfaces and ask users to choose: checked or runtime exceptions depending on their use case.

If this makes sense to all, then we can move on to discuss how we expose these two interfaces, and which if any of the two should  be more prominent in the API.  If not lets continue the debate, but trying to focus on this first decision.

Make sense?

Chris

To view this discussion on the web visit https://groups.google.com/d/msgid/ehcache-dev/CAOOJethJAF7Z9tzU7v-bMH-TEtz8C%3Dv8cjq%3DKrtCjbrKMaGkXw%40mail.gmail.com.

[Mathilde Rigabert Lemée]

Totally agree.

Still #2 and not far away #3 for me. Even if I agree about what you said for the #2 and #3.

To view this discussion on the web visit https://groups.google.com/d/msgid/ehcache-dev/D0347D7D.2868D%25chris.w.dennis%40gmail.com.

[Chris Schanck]

Well, for me, #1 is the actual preferred 'Java way', and for distributed cases (and probably most non-trivial real world use cases) checked exceptions are necessary.

If you want/need a thin layer on the outermost API that eats and transforms checked into unchecked, that's easy to do (and I suppose we could ship one). But it seems wrong not to emphasize the checked exception error cases if the cache interface is meant to be used for anything but simple heap caches.

The check/unchecked argument sometimes feels like it is really an argument against Java's way of handling exceptions; try/catch/finally blocks are wordy, ugly, and unloved. But it's a Java implementation, and checked exceptions are the proper way to notify the user of errors. Besides, modern IDE's make it easy to generate the try/catch blocks for prototyping, like most other Java boilerplate.

Just $0.02.

[Alex Snaps]

It seems to me like we should play it on the safe side and use the type that throws checked exceptions as the default. 

My rational is as follow: this is a Cache. Unlike when your system of record being down, you should be able to proceed operations (even if maybe slightly degraded) even when some cache operation couldn't succeed. 

The PlayingItSafe.java example illustrates this here: https://gist.github.com/anonymous/2368b094793b02864b69#file-playingitsafe-java

Now I understand that someone starting to the Ehcache API will be initially somewhat put off by this, but given these exceptions not only should, but could (in the sense that 99% of use cases for a cache can actually deal w/ it being unresponsive) be handled by the user, I'd expect the developer to pause and least think for bit about it... 

If he knows really better (using a on-heap only topology) or is just prototyping something quickly, he can then switch to deliberately ignore the exceptions somehow, as here: https://gist.github.com/anonymous/2368b094793b02864b69#file-consciouslylivingontheedge-java

Now that's merely a proposal, I'm not saying that's how one should do it. 

Also, this approach allows for finer tuning of what to do in the case of an exception, i.e. not wrap it in an unchecked one so it gets out of my way while I code. A variation of the that is here: https://gist.github.com/anonymous/2368b094793b02864b69#file-resilientcache-java

Basically using a decorator, wire a different strategy in... This particular example may be good enough in certain usages, not in others... but one can actually decide: fine grained control using the Cache interface, go live on the edge and get the exceptions out of the way using the something along UncheckedCache or choose a custom tailored strategy.

So, yes, try/catch are ugly... but I'd rather use the type-system explicitly here and get the compiler's help when say bumping up my ehcache version that introduced this new Exception or condition or whatnot, rather than hope for people rereading the Javadoc of all APIs they use on every minor release. 

Also, given this is a Cache, my webapp serving a 500 because I forgot to catch the RuntimeException in something like: https://gist.github.com/anonymous/2368b094793b02864b69#file-livingontheedge-java 

doesn't seem like something I'd desire... only because that given value, persisted to disk, can't be deserialized on my recently redeployed app, that slightly changed a class here or there... 

To view this discussion on the web visit https://groups.google.com/d/msgid/ehcache-dev/2295340c-2fb3-4d93-85d3-84a22590f99d%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Aurelien Broszniowski]

It's good that you wrote code, it brings clarity on how things may look like, now I realize that if it's difficult to agree on an answer, it's probably because it's not clear what kind of exception can happen.

E.g. a cache.get(key) returning no result is not an exception, it can happen, it's a cache, so I wouldn't like to write code following the PlayingItSafe example.

Thinking about the lifecycle of a cache, I can think of cases where exceptions would be needed (e.g. can't init a cache), but regarding CRUD ops, what error cases really needs to force the user to handle it?

Since you guys know in depth the codebase of Ehcache, can you list examples of exceptions that are thrown when doing CRUD ops and you know they're important enough to be propagated to the user instead of failing silently (just being logged)?  

[Alex Snaps]

Aurelien, sadly on CRUD ops many things can go wrong, I hinted to a couple of them in other posts:

deserialization failures when loading data from disk, disk failing, remote node being unavailable... most failures apply to non-heap caches though. 

Thinking about this further, as I had a hunch ever since I wrote this `ResilientCache` example, I think we may well be looking at this wrong. 

All the `Cache` failures on CRUD are actually `Store` (the SPI that backs the Cache up and let it store data) failures. Caches can fail for other reasons besides these: like `CacheLoader` or `CacheWriter` failures... 

My main point behind wanting to have checked exceptions, is to force someone to handle these... Because I think a cache failure shouldn't bring the entire app down (or that thread, request, ... whatever). Instead, I'd like the Cache to be resilient. So why not have `Cache` be resilient _always_... And not throw exception (except if the user really wants it). 

Basically come up with an API that naturally leads to doing the right thing, without being restrictive nor "straight in your face", just like implicit CacheLoaders aim at doing: implementing a "cache aside" pattern isn't easy and forces the user to understand much about the cache's internals; otoh the "cache through" pattern is much more easy to think about: register a loader & a writer and the cache will load when data is missing and update the SoR on writes, and all that in a thread safe way, without races. 

I'm thinking we should try to do the same here: have the Cache be resilient, according to some definition, by default. You could specify what "resilience" mean depending on your usecase as well as deployment topology, maybe even providing your own `ResilienceStrategy` or something... But recoverable exceptions would then be handled internally, while unrecoverable ones, i.e. unchecked _and_ documented ones, would be the only ones "leaking" up. 

Does this make sense? I'm thinking about giving this a shot, and doing a short spike on trying to come up with a concrete proposal in implementing some prototype around that idea... So for now, I'd argue ignore exceptions at the `Cache` level... internal SPI types though (like the `Store` interface, would declare explicit checked exception... but the `Cache` would be layer responsible to handle these "accordingly").

Thoughts?

To view this discussion on the web visit https://groups.google.com/d/msgid/ehcache-dev/b1632468-40eb-40b8-9bb7-5d5744e3124d%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.