10 Replies Latest reply on Jan 30, 2009 11:26 AM by jason.greene

    JBoss Cache Options API

    manik

      This has come up in the past, and I want to revisit this for a potential future improvement to the API.

      Options are hugely useful in being able to provide additional per-invocation context and overrides.

      But the current mechanism is nothing short of kludgey:

       cache.getInvocationContext().getOptionOverrides().setXYZ();
       cache.doSomething(); // your invocation
      


      Ugly since it involves thread locals and there is a lot of code to check for leaks, proper cleanup, context leaking to additional calls, nested calls with notifications, etc etc etc.

      So I'm looking for suggestions for a better approach here.

      FYI, the way this was done in JBC 1.4.x (which is still ugly IMO) was to overload all methods on the cache with versions that took in an Option class.

      e.g.,

       cache.get(Fqn f, Object key);
       cache.get(Fqn f, Object key, Option o);
      


      Thoughts and suggestions?

        • 1. Re: JBoss Cache Options API
          galder.zamarreno

          Why not convert all methods that can take Options to varargs?

          Example:

          cache.get(Fqn f, Object key, Option... o);


          You can then call it with or without option, i.e.:

          cache.get("a/b/c", "k");


          And also like this:

          Option myOption = ...
          cache.get("a/b/c", "k", myOption);


          Even better now, you can define individual Option as subsclasses/implementations of Option allowing each to carry extra parameters if needed.

          For example: Let's imagine that for https://jira.jboss.org/jira/browse/JBCACHE-1470 we would like to add the cache listener class to the supress event notification. You could do something like this:

          SupressEvenNotificationOption seno = ....; // SupressEvenNotificationOption implements Option
          seno.setCacheListener(MyListener.class);
          cache.put("a/b/c", "k", "v", seno);


          This looks much cleaner to me and you avoid multiple method definitions in Cache API.

          • 2. Re: JBoss Cache Options API
            galder.zamarreno

            And of course, you could then pass multiple Options in one line, i.e.:

            cache.put("a/b/c", "k", "v", new FailSilently(), new CacheModeLocal());


            Hence, making the put call fail silently and only local.

            • 3. Re: JBoss Cache Options API
              manik

              True. And if they are passed in explicitly via varargs, we get over all of the thread local issues.

              Not too sure about a separate option subclass for each option though - might be a bit heavy.

              Perhaps options could be an enum, given that each option is just a boolean switch (with the exception of lockAcquisitionTimeout and syncReplTimeout, that is). As for lockAcquisitionTimeout and syncReplTimeout, AFAIK lockAcquisitionTimeout is only used to zero the acquisition timeout for putForExternalRead(), so it could be replaced with a boolean zeroLockAcquisutionTimeout.

              I don't see where syncReplTimeout is used, maybe it is unnecessary?

              So, Options may now look like:

              public enum Options {
               FAIL_SILENTLY, CACHE_MODE_LOCAL, SUPPRESS_LOCKING, FORCE_WRITE_LOCK,
               SKIP_CACHE_STATUS_CHECK, FORCE_ASYNC, FORCE_SYNC,
               // ... etc ...
              }
              


              And the CacheInvocationDelegate creates an EnumSet from these and adds this EnumSet to the invocation context before passing up the chain.

              • 4. Re: JBoss Cache Options API
                jason.greene

                When we discussed varargs before, IIRC you didn't like it because of how IDEs handled it at the time. I am not sure if this has improved.

                Another possibility is to define a new interface which contains the overloaded methods. cache.getAdvanced() or something like that. Then you could just define a method with an overloaded EnumSet.

                • 5. Re: JBoss Cache Options API
                  jason.greene

                  There is one other radical approach. Which is that you introduce a context/session for cache access that is bound to that object.

                  Example:

                  CacheSession session = cache.getCacheSession();
                  session.setOption(BLAH_ENUM);
                  session.put(x,y);

                  • 6. Re: JBoss Cache Options API
                    manik

                    yes, I didn't quite like using varargs like that before, not just because of the way IDEs deal with it but also how it becomes misleading on javadocs, since it implies that the varargs are "necessary". That still stands, but at least it provides a cleaner approach to using thread-locals. :)



                    • 7. Re: JBoss Cache Options API
                      manik

                       

                      "jason.greene@jboss.com" wrote:

                      CacheSession session = cache.getCacheSession();
                      session.setOption(BLAH_ENUM);
                      session.put(x,y);


                      That's not a lot different from your getAdvanced(), except that the new interface allows you to set options in a separate method rather than an overloaded method. And then sticks it in a thread-local. :)



                      • 8. Re: JBoss Cache Options API
                        dmlloyd

                        IDEA has never (iirc) had a problem with varargs. But in any case, if you do decide to go with varargs, it might be a good idea to have a varargs-less variation on the method for the no-options case (if it's common) to prevent the empty-array creation on every method invocation. Though now that I think about it, I don't remember if it makes a significant performance-wise...

                        • 9. Re: JBoss Cache Options API
                          manik

                           

                          "david.lloyd@jboss.com" wrote:
                          IDEA has never (iirc) had a problem with varargs. But in any case, if you do decide to go with varargs, it might be a good idea to have a varargs-less variation on the method for the no-options case (if it's common) to prevent the empty-array creation on every method invocation. Though now that I think about it, I don't remember if it makes a significant performance-wise...


                          Not a "problem" as such, but just suggesting that you needed stuff in there. Reading off method signatures, really - the same thing that makes the javadocs misleading. Goes against the principle of varargs, IMO, since we are talking about an optional parameter, not a variable length one. At least, that is the case the way things are with a single Option object (or an EnumSet of options if options become an enum). Less ugly if we have a vararg of Options, where options are an enum and you provide as many options as you want.

                          Not so pretty though if you are implementing an external interface, e.g. Map or javax.cache.Cache, but that does take care of the (more common) no-option use case.


                          • 10. Re: JBoss Cache Options API
                            jason.greene

                             

                            "manik.surtani@jboss.com" wrote:
                            "jason.greene@jboss.com" wrote:

                            CacheSession session = cache.getCacheSession();
                            session.setOption(BLAH_ENUM);
                            session.put(x,y);


                            That's not a lot different from your getAdvanced(), except that the new interface allows you to set options in a separate method rather than an overloaded method. And then sticks it in a thread-local. :)



                            No there is no thread local. The session object in this case would be a normal object, with fields for the current option.