--
Galder Zamarreño
Infinispan, Red Hat
On 1 Mar 2017, at 07:01, Dan Berindei <dan.berindei@gmail.com> wrote:
On Wed, Mar 1, 2017 at 10:18 AM, Radim Vansa <rvansa@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],
maybe with a different intention.
Strange and complex combinations don't help. We have made a clear
separation between templates and cache configurations; you should not
use regular cache configuration as a template for programmatically
defined cache anymore, and if you really want to, there are means to
that (load, undefine, define).
Btw., the javadoc is out of date, too, since it mentions default cache
which has been removed recently.
That defineConfiguration javadoc is just weird,
^ Most likely my fault! :$ ;)
it says what the
Configuration returned by the method will be but it doesn't say what
the configuration associated with that cache name in the cache manager
will be...
I agree with throwing an exception in defineConfiguration(...) when
that cache is already defined. I would not throw an exception from
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).
Cheers
Dan
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@gmail.com
<mailto:mudokonman@gmail.com>> wrote:
On Mon, Feb 27, 2017 at 9:55 AM Dan Berindei
<dan.berindei@gmail.com <mailto:dan.berindei@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@gmail.com <mailto:mudokonman@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@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa@redhat.com>
JBoss Performance Team
_______________________________________________
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev