<html><head></head><body class="ApplePlainTextBody" dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br>--<br>Galder Zamarreño<br>Infinispan, Red Hat<br><br><blockquote type="cite">On 1 Mar 2017, at 07:01, Dan Berindei <dan.berindei@gmail.com> wrote:<br><br>On Wed, Mar 1, 2017 at 10:18 AM, Radim Vansa <rvansa@redhat.com> wrote:<br><blockquote type="cite">I still think that if the cache is already defined, defineConfiguration<br>should throw an exception. This javadoc was written 7 years ago [1],<br>maybe with a different intention.<br>Strange and complex combinations don't help. We have made a clear<br>separation between templates and cache configurations; you should not<br>use regular cache configuration as a template for programmatically<br>defined cache anymore, and if you really want to, there are means to<br>that (load, undefine, define).<br><br>Btw., the javadoc is out of date, too, since it mentions default cache<br>which has been removed recently.<br><br></blockquote><br>That defineConfiguration javadoc is just weird,<br></blockquote><br>^ Most likely my fault! :$ ;)<br><br><blockquote type="cite">it says what the<br>Configuration returned by the method will be but it doesn't say what<br>the configuration associated with that cache name in the cache manager<br>will be...<br><br>I agree with throwing an exception in defineConfiguration(...) when<br>that cache is already defined. I would not throw an exception from<br>getCache(cache, configurationName) when the cache is already defined,<br>I'd just ignore the new configuration (as we already ignore it when<br>the cache is runninng) and maybe log a warning telling people to use<br>defineConfiguration(cacheName, configurationName, new<br>ConfigurationBuilder().build()) + getCache(cacheName).<br><br>Cheers<br>Dan<br><br><br><blockquote type="cite">R.<br><br>[1]<br>https://github.com/infinispan/infinispan/commit/73d99d37ebfb8af6b64df6a7579a2448deacbde7#diff-e2b618b97769a45ec42eb5910a8c2119R62<br><br>On 02/28/2017 10:51 PM, William Burns wrote:<br><blockquote type="cite">So while I was trying to work on this, I have to admit I am even more<br>torn in regards to what to do. Looking at [1] it looks like the<br>template should only be applied if the cache configuration is not<br>currently defined. Unfortunately it doesn't work this way and always<br>applies this template to any existing configuration. So I am thinking<br>an alternative is to instead make it work as the documentation states,<br>only using the template if the cache is not currently defined. This<br>seems more logical to me at least.<br><br>With that change the getCache(String, String) could stay as long as it<br>is documented that a template is only applied if no cache<br>configuration exists.<br><br>What do you guys think?<br><br>[1]<br>https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/manager/EmbeddedCacheManager.java#L84<br><br><br>On Mon, Feb 27, 2017 at 10:09 AM William Burns <mudokonman@gmail.com<br><mailto:mudokonman@gmail.com>> wrote:<br><br> On Mon, Feb 27, 2017 at 9:55 AM Dan Berindei<br> <dan.berindei@gmail.com <mailto:dan.berindei@gmail.com>> wrote:<br><br> I would go for option 2.<br><br><br> Do you think a WARN message will be enough? I am a bit weary about<br> this option myself.<br><br><br> We already started disconnecting the cache definition and<br> retrieval,<br> at least `getCache(name)` doesn't define a new cache based on the<br> default configuration any more. So I don't think it would be<br> too much,<br> even at this point, to deprecate all the overloads of<br> `getCache` that<br> can define a new cache and advise users to use<br> `defineConfiguration`<br> instead.<br><br><br> Hrmm I like the idea of deprecating the overloads :)<br><br><br><br><br> Cheers<br> Dan<br><br><br> On Mon, Feb 27, 2017 at 4:31 PM, William Burns<br> <mudokonman@gmail.com <mailto:mudokonman@gmail.com>> wrote:<br><blockquote type="cite">When working on another project using Infinispan the code<br></blockquote> being used was a<br><blockquote type="cite">bit interesting and I don't think our template configuration<br></blockquote> handling was<br><blockquote type="cite">expecting it do so in such a way.<br><br>Essentially the code defined a template for a distributed<br></blockquote> cache as well as<br><blockquote type="cite">some named caches. Then whenever a cache is retrieved it<br></blockquote> would pass the<br><blockquote type="cite">given name and always the distributed cache template.<br></blockquote> Unfortunately with<br><blockquote type="cite">the way templates work they essentially redefine a cache<br></blockquote> first so the actual<br><blockquote type="cite">cache configuration was wiped out. In this example I was<br></blockquote> able to get the<br><blockquote type="cite">code to change to using a default cache instead, which is<br></blockquote> the behavior that<br><blockquote type="cite">is needed.<br><br>The issue though at hand is whether we should allow a user<br></blockquote> to call getCache<br><blockquote type="cite">in such a way. My initial thought is to have it throw some<br></blockquote> sort of<br><blockquote type="cite">configuration exception when this is invoked. But there are<br></blockquote> some possible<br><blockquote type="cite">options.<br><br>1. Throw a configuration exception not allowing a user to<br></blockquote> use a template<br><blockquote type="cite">with an already defined cache. This has a slight disconnect<br></blockquote> between<br><blockquote type="cite">configuration and runtime, since if a user adds a new<br></blockquote> definition it could<br><blockquote type="cite">cause runtime issues.<br>2. Log an error/warning message when this occurs. Is this<br></blockquote> enough though?<br><blockquote type="cite">Still could have runtime issues that are possibly undetected.<br>3. Merge the configurations together applying the template<br></blockquote> first. This<br><blockquote type="cite">would be akin to how default cache works currently, but you<br></blockquote> would get to<br><blockquote type="cite">define your default template configuration at runtime. This<br></blockquote> sounded like the<br><blockquote type="cite">best option to me, but the problem is what if someone calls<br></blockquote> getCache using<br><blockquote type="cite">the same cache name but a different template. This could get<br></blockquote> hairy as well.<br><blockquote type="cite"><br>Really thinking about the future, disconnecting the cache<br></blockquote> definition and<br><blockquote type="cite">retrieval would be the best option, but we can't do that<br></blockquote> this late in the<br><blockquote type="cite">game.<br><br>What do you guys think?<br><br>- Will<br><br>_______________________________________________<br>infinispan-dev mailing list<br>infinispan-dev@lists.jboss.org<br></blockquote> <mailto:infinispan-dev@lists.jboss.org><br><blockquote type="cite">https://lists.jboss.org/mailman/listinfo/infinispan-dev<br></blockquote> _______________________________________________<br> infinispan-dev mailing list<br> infinispan-dev@lists.jboss.org<br> <mailto:infinispan-dev@lists.jboss.org><br> https://lists.jboss.org/mailman/listinfo/infinispan-dev<br><br><br><br>_______________________________________________<br>infinispan-dev mailing list<br>infinispan-dev@lists.jboss.org<br>https://lists.jboss.org/mailman/listinfo/infinispan-dev<br></blockquote><br><br>--<br>Radim Vansa <rvansa@redhat.com><br>JBoss Performance Team<br><br>_______________________________________________<br>infinispan-dev mailing list<br>infinispan-dev@lists.jboss.org<br>https://lists.jboss.org/mailman/listinfo/infinispan-dev<br></blockquote>_______________________________________________<br>infinispan-dev mailing list<br>infinispan-dev@lists.jboss.org<br>https://lists.jboss.org/mailman/listinfo/infinispan-dev<br></blockquote><br></body></html>