[infinispan-dev] Calling getCache with a template and defined configuration
William Burns
mudokonman at gmail.com
Wed Mar 1 20:31:02 EST 2017
Alright seems fine to me.
On Wed, Mar 1, 2017, 9:27 AM Tristan Tarrant <ttarrant at redhat.com> wrote:
> On 01/03/17 13:01, Dan Berindei wrote:
> > On Wed, Mar 1, 2017 at 10:18 AM, Radim Vansa <rvansa at redhat.com> wrote:
> >> I still think that if the cache is already defined, defineConfiguration
> >> should throw an exception. This javadoc was written 7 years ago [1],
>
> > I agree with throwing an exception in defineConfiguration(...) when
> > that cache is already defined.
>
> +1
>
I get a lot of tests to change and remove now 😁
> > getCache(cache, configurationName) when the cache is already defined,
> > I'd just ignore the new configuration (as we already ignore it when
> > the cache is runninng) and maybe log a warning telling people to use
> > defineConfiguration(cacheName, configurationName, new
> > ConfigurationBuilder().build()) + getCache(cacheName).
>
> +1
>
> Tristan
>
> >> R.
> >>
> >> [1]
> >>
> https://github.com/infinispan/infinispan/commit/73d99d37ebfb8af6b64df6a7579a2448deacbde7#diff-e2b618b97769a45ec42eb5910a8c2119R62
> >>
> >> On 02/28/2017 10:51 PM, William Burns wrote:
> >>> So while I was trying to work on this, I have to admit I am even more
> >>> torn in regards to what to do. Looking at [1] it looks like the
> >>> template should only be applied if the cache configuration is not
> >>> currently defined. Unfortunately it doesn't work this way and always
> >>> applies this template to any existing configuration. So I am thinking
> >>> an alternative is to instead make it work as the documentation states,
> >>> only using the template if the cache is not currently defined. This
> >>> seems more logical to me at least.
> >>>
> >>> With that change the getCache(String, String) could stay as long as it
> >>> is documented that a template is only applied if no cache
> >>> configuration exists.
> >>>
> >>> What do you guys think?
> >>>
> >>> [1]
> >>>
> https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/manager/EmbeddedCacheManager.java#L84
> >>>
> >>>
> >>> On Mon, Feb 27, 2017 at 10:09 AM William Burns <mudokonman at gmail.com
> >>> <mailto:mudokonman at gmail.com>> wrote:
> >>>
> >>> On Mon, Feb 27, 2017 at 9:55 AM Dan Berindei
> >>> <dan.berindei at gmail.com <mailto:dan.berindei at gmail.com>> wrote:
> >>>
> >>> I would go for option 2.
> >>>
> >>>
> >>> Do you think a WARN message will be enough? I am a bit weary about
> >>> this option myself.
> >>>
> >>>
> >>> We already started disconnecting the cache definition and
> >>> retrieval,
> >>> at least `getCache(name)` doesn't define a new cache based on
> the
> >>> default configuration any more. So I don't think it would be
> >>> too much,
> >>> even at this point, to deprecate all the overloads of
> >>> `getCache` that
> >>> can define a new cache and advise users to use
> >>> `defineConfiguration`
> >>> instead.
> >>>
> >>>
> >>> Hrmm I like the idea of deprecating the overloads :)
> >>>
> >>>
> >>>
> >>>
> >>> Cheers
> >>> Dan
> >>>
> >>>
> >>> On Mon, Feb 27, 2017 at 4:31 PM, William Burns
> >>> <mudokonman at gmail.com <mailto:mudokonman at gmail.com>> wrote:
> >>> > When working on another project using Infinispan the code
> >>> being used was a
> >>> > bit interesting and I don't think our template configuration
> >>> handling was
> >>> > expecting it do so in such a way.
> >>> >
> >>> > Essentially the code defined a template for a distributed
> >>> cache as well as
> >>> > some named caches. Then whenever a cache is retrieved it
> >>> would pass the
> >>> > given name and always the distributed cache template.
> >>> Unfortunately with
> >>> > the way templates work they essentially redefine a cache
> >>> first so the actual
> >>> > cache configuration was wiped out. In this example I was
> >>> able to get the
> >>> > code to change to using a default cache instead, which is
> >>> the behavior that
> >>> > is needed.
> >>> >
> >>> > The issue though at hand is whether we should allow a user
> >>> to call getCache
> >>> > in such a way. My initial thought is to have it throw some
> >>> sort of
> >>> > configuration exception when this is invoked. But there are
> >>> some possible
> >>> > options.
> >>> >
> >>> > 1. Throw a configuration exception not allowing a user to
> >>> use a template
> >>> > with an already defined cache. This has a slight disconnect
> >>> between
> >>> > configuration and runtime, since if a user adds a new
> >>> definition it could
> >>> > cause runtime issues.
> >>> > 2. Log an error/warning message when this occurs. Is this
> >>> enough though?
> >>> > Still could have runtime issues that are possibly
> undetected.
> >>> > 3. Merge the configurations together applying the template
> >>> first. This
> >>> > would be akin to how default cache works currently, but you
> >>> would get to
> >>> > define your default template configuration at runtime. This
> >>> sounded like the
> >>> > best option to me, but the problem is what if someone calls
> >>> getCache using
> >>> > the same cache name but a different template. This could get
> >>> hairy as well.
> >>> >
> >>> > Really thinking about the future, disconnecting the cache
> >>> definition and
> >>> > retrieval would be the best option, but we can't do that
> >>> this late in the
> >>> > game.
> >>> >
> >>> > What do you guys think?
> >>> >
> >>> > - Will
> >>> >
> >>> > _______________________________________________
> >>> > infinispan-dev mailing list
> >>> > infinispan-dev at lists.jboss.org
> >>> <mailto:infinispan-dev at lists.jboss.org>
> >>> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >>> _______________________________________________
> >>> infinispan-dev mailing list
> >>> infinispan-dev at lists.jboss.org
> >>> <mailto:infinispan-dev at lists.jboss.org>
> >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> infinispan-dev mailing list
> >>> infinispan-dev at lists.jboss.org
> >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >>
> >>
> >> --
> >> Radim Vansa <rvansa at redhat.com>
> >> JBoss Performance Team
> >>
> >> _______________________________________________
> >> infinispan-dev mailing list
> >> infinispan-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >
>
> --
> Tristan Tarrant
> Infinispan Lead
> JBoss, a division of Red Hat
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/infinispan-dev/attachments/20170302/1d332380/attachment-0001.html
More information about the infinispan-dev
mailing list