magpiebrain

Sam Newman's site, a Consultant at ThoughtWorks

I’ve been writing some caching code recently at work and it occurred to me that I should really look into some caching API’s. Whilst talking to Matt at the last London-Java meetup, the discussion of caching came up. He said he’d looked at “OSCache”:http://www.opensymphony.com/oscache/, but decided against using it due to a complaint with its API. Apparently when you get an entry from the cache, if it doesn’t exist it throws an exception. Not fulling believing this to be true, I looked through the API docs (note to the OpenSymphony guys – a simple tutorial would be nice) to find that he is completely correct. This from the cache.getFromCache javadoc:

Throws:
NeedsRefreshException – Thrown when the object either doesn’t exist, or exists but is stale. When this exception occurs, the CacheEntry corresponding to the supplied key will be locked and other threads requesting this entry will potentially be blocked until the caller repopulates the cache. If the caller choses not to repopulate the cache, they must instead call cancelUpdate(String).

This strikes me has bizare to say the least. First off exceptions are being used to control logic flow, which is a bad thing. Exceptions are for errors in your code – be they user driven, programatic or due to system disruption (please don’t quote the whole NumberFormatException being thrown from the various number parse methods – thats just wrong). Secondly, the obvious (to me) way to design a cache is to allow a pluggable load strategy to the cache, and if the content doesn’t exist the cache itself goes and gets the content. Then if there is a problem loading the content you would get an error thrown, which would of course be an unchecked exception as it will be a programmatic or system environment error. Here the cache is saying “I’m stupid, fix me” and will then perhaps even lock your thread until you do. To be of any use to me I’m going to have to wrap OSCache with my own Cache class to do this for me.

I am struggling to think of a single advantage to this approach. Everywhere you want to use the getFromCache() method you are now forced to wrap it with a try-catch block, and then have to repopulate the cache if it fails. A pluggable content-loading system wouldn’t be hard to write. I have come to the conclusion that I must be missing something. Either the javadoc is wrong or there is a very good reason for getFromCache() to work like this. Can someone put me right?

Advertisements

9 Responses to “Strange design decisions of our time – OSCache”

  1. Chris Miller

    I couldn’t agree more, and I see this behaviour as one of the bigger flaws in the design of OSCache.

    However if it’s any consolation I think I know why OSCache was designed this way. It’s because (in the case of stale content being requested) the NeedsRefreshException holds a copy of the stale content. This allows the taglibs to be used to output the old content if it’s not possible to render new content (due to eg a broken database connection).

    Obviously this could have been handled differently, and without requiring an exception to be thrown. As far as I know this mechanism has been in place since day one and the API is now heavily dependent on it, so reworking it this far down the track would break a lot of people’s code.

    Reply
  2. R.J.

    Dittos from me; wierd indeed.
    I usually like to apply a thin wrapper to the caching mechanism I am using anyways, and that usually obscures this wierd detail from the bulk of my code… but last time I was at that level it struck me as odd.

    Reply
  3. Sam Newman

    Well, if you wanted you could add a new getCacheEntry method (how about getEntry? Its a method on a cache so calling it a cacheEntry is overkill anyway) and deprecate the old method. As for the taglib wanting to get old content if new content cannot be retrieved (e.g. due to a server being down) that could be part of the pluggable refresh framework. A CacheEntryRefresher would be registered with specified CacheEntry objects inside the cache itself. Such a class could define one method:

    public Object refreshCache(Object entry, Object staleEntry)

    Passing in entry allows the CacheEntryRefresher to be registered against multiple entries, the stale object can then be returned if new content cannot be returned. Its pretty much what my CacheWrapper will end up looking like in any case…

    Reply
  4. Sam Pullara

    Not speaking to the exception part, but to the cache population part. Cache loaders are ok in some situations but often the only place where the information exists to populate the cache is in the caller. Only when the cache content is based purely on the key is it possible to reconstruct it in a loader. Additionally, putting the cache population code in the loader separates it logically from the caller which can be a maintenance nightmare.

    Reply
  5. Sam Newman

    I can understand your argument, but to be honest I still think this applies a small percentage of people using caching code. The fact that only the calling code knows how to populate the cache is certainly not guaranteed , and an abstract loader framework doesn’t prohibit this taking place – instead of a try-catch block with the entry being loaded in the catch block, you first register the loader, then try and get the content (although to be I’m struggling to think of a reason why the loader needs to be assigned more than once, whereas the exception always needs to be caught). As for seperating out the loader from the caller being an implementation issue, I don’t think this is a big problem at all. Its almost like saying putting Swing ActionListener listener code in a seperate class makes a button hard to maintain. Code that uses a cache is made very simple by assuming the content is there – let the cache seemlessly get the content. If you really want to know if the content is dirty, add a simple boolean method.

    Reply
  6. Paul

    I heard someone say once that “using exceptions to report failures is like using telegraph poles to stop your car”. It suck with me. 😉

    Reply
  7. Kirk

    I’ve had this argument with some people on collections in general. The agrument for is that if you try to get something from a collection and it’s not there, then it’s an exception. I’ve always countered that, it’s an exception if I expect to find it and I don’t. But, the collection doesn’t understand the symantics of my call and therfore can not know if an exception is warrented. As such, I don’t want the author of collections making that decision for me. I assume the same uneccesary assumption was made by the author of OSCache.

    When we design or write code, we often need to make assumptions. The question is, do we go back and evaluate those assumptions. How do we support our assumptions? Is it with dogma, or can we be pragmatic and just recognize the ones for which there is no basis for and eliminate them. It is these assumptions that make frameworks less useful.

    In this case, if needing to refresh the cache an exceptional situation? Is not finding an object in the cache an exception situation? I’m not sure about the first which leads me to believe that it should be handled in some logic other than exception code. I’m sure that the second situation should not throw an exception.

    Reply
  8. Sam Newman

    Kirk, I tend to favour the idea that if you make assumptions that turn out to be incorrect, then exceptions get thrown. I object to code that throws exceptions when all I want to do is test those assumptions. In the case of the Cache though this is a bit of a different issue. I assume I should be able to get the content, but I have also assumed that the cache should be capable of refreshing it. If it attempts to refresh and cannot then throw an (unchecked) exception. So my real complaint is not that an exception is being thrown, more that the cache’s functionality falls short of my expectations. This results in code using the cache being far more verbose and messy than it needs to be.

    Reply

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Basic HTML is allowed. Your email address will not be published.

Subscribe to this comment feed via RSS

%d bloggers like this: