On 08/20/2014 12:16 PM, Dan Berindei wrote:
On Wed, Aug 20, 2014 at 12:27 PM, Galder Zamarreño <galder(a)redhat.com
<mailto:galder@redhat.com>> wrote:
On 15 Aug 2014, at 12:41, Dan Berindei <dan.berindei(a)gmail.com
<mailto:dan.berindei@gmail.com>> wrote:
>
>
>
> On Fri, Aug 15, 2014 at 11:37 AM, Galder Zamarreño
<galder(a)redhat.com <mailto:galder@redhat.com>> wrote:
>
> On 12 Aug 2014, at 22:41, Dan Berindei <dan.berindei(a)gmail.com
<mailto:dan.berindei@gmail.com>> wrote:
>
>>
>>
>>
>> On Tue, Aug 5, 2014 at 11:27 AM, Galder Zamarreño
<galder(a)redhat.com <mailto:galder@redhat.com>> wrote:
>> Can't comment on the document, so here are my thoughts:
>>
>> Re: "Get rid of lazy cache starting...all the caches run on all
nodes...it should still be possible to start a cache at runtime,
but it will be run on all nodes as well"
>>
>> ^ Though I like the idea, it might change a crucial aspect of
how default cache configuration works (if we leave the concept of
default cache at all). Say you start a cache named "a" for which
there's no config. Up until now we'd use the default cache
configuration and create a cache "a" with that config. However, if
caches are started cluster wide now, before you can do that, you'd
have to check that there's no cache "a" configuration anywhere in
the cluster. If there is, I guess the configuration would be
shipped to the node that starts the cache (if it does not have it)
and create the cache with it? Or are you assuming all nodes in the
cluster must have all configurations defined?
>>
>> +1 to remove the default cache as a default configuration.
>>
>> I like the idea of shipping the cache configuration to all the
nodes. We will have to require any user-provided objects in the
configuration to be serializable/externalizable, but I don't see a
big problem with that.
>>
>> In fact, it would also allow us to send the entire
configuration to the coordinator on join, so we could verify that
the configuration on all nodes is compatible (not exactly the
same, since things like capacityFactor can be different). And it
would remove the need for the CacheJoinInfo class...
>>
>> A more limited alternative, not requiring config serialization,
would be to disallow getCache(name) when a configuration doesn't
exist but add a method createCache(name, configurationName) that
only requires configurationName to be defined everywhere.
>>
>>
>> Re: "Revisiting Configuration elements..."
>>
>> If we're going to do another round of updates in this area, I
think we should consider what to do with unconfigured values. Back
in the 4.x days, the JAXB XML parsing allowed us to know which
configuration elements the user had not configured, which helped
us tweak configuration and do validation more easily. Now, when we
look at a Configuration builder object, we see default values but
we do not that a value is the one it is because the user has
specifically defined it, or because it's unconfigured. One way to
do so is by separating the default values, say to an XML file
which is reference (I think WF does something along these lines)
and leave the builder object with all null values. This would make
it easy to figure out which elements have been touched and for
that those that have not, use default values. This has popped up
in the forums before but can't find a link right now...
>>
>> I was also thinking of doing something like that, but instead
of having a separate XML with the defaults I was going to propose
creating a layer of indirection: every configuration value would
be a ConfigurationProperty<T>, with a default value, an override
value, and an actual value. We already do something similar for
e.g. StateTransferConfiguration.awaitInitialTransfer and
originalAwaitInitialTransfer).
>
> ^ What's the problem with a separate XML file?
>
> I really like the idea of externalizing default values from a
documentation perspective and ease of change down the line, both
for us and for users.
>
> On top of that, it could be validated and be presented as a
reference XML file, getting rid of the sample XML file that we
currently have which is half done and no one really updates it.
>
> First of all, how would that XML look? Like a regular
configuration file, with one cache of each type?
Yeah, could do. Wildfly guys already doing it:
https://github.com/wildfly/wildfly/blob/master/clustering/infinispan/src/...
> One store of each type? In every cache? How would we handle
defaults for custom stores?
The defaults for custom stores are the same as for any other cache
store. The only thing you cannot default is the custom store
specific stuff, which is specific to the custom store :)
Except you can't include them in the default XML, because the default
XML is in core (I assume) and the custom stores are not.
Would it be a problem (from XML tech perspective) to have default store
configuration without all the infinispan shell (<persistence /> and
above), just like
<?xml ... ?>
<myCustomStore>
...
</myCustomStore>
You could have a JDBC_CACHE_STORE cache with the defaults for JDBC
cache stores...etc.
In what XML file?
> We already have an XML file with default values:
infinispan-config-7.0.xsd. It would be nice if we could parse that
and keep the defaults in a single place, but if we need to
duplicate the defaults anyway, I'd rather keep them in code.
An XSD file is not an XML file. By having the defaults in an XML
file, we can validate it and confirm that it's a valid XML file
that we can parse it. Users don't load Infinispan with XSD files :)
I'm pretty sure infinispan-config-7.0.xsd is a valid XML file, it even
starts with a standard XML declaration: <?xml version="1.0"
encoding="UTF-8" standalone="yes"?>
To avoid duplication, I'd be tempted to remove all default values
from the XSD file and keep them only in the reference XML file.
It would definitely be harder to look up the reference XML and check
the defaults, compared to a Ctrl+click on the element/attribute name
with the XSD.
Of course, the XSD only allows one default value for each attribute,
and even duplicating the element types for each cache mode sounds
pretty daunting.
Cool apps (read: RadarGun) generate XSD from source code :) I am
generally fan of having just single place for the default value,
propagated automatically.
> I also think with a separate XML file, we'd still need to keep
some not-quite-defaults in the various builder.build() methods (or
Configurations methods).
^ What defaults are you talking about? Can you provide an example
of such default options?
With an XML, you could even have different defaults depending on
the other attributes of the cache. E.g. say you have an OL cache,
you could say that the default value for writeSkew with OL is
true, whereas with PL, the default value is false.
Yeah, that would be a good example of what I was thinking about :)
But I was thinking we shouldn't just change the default value, we
should also throw an exception when the user tries to enable write
skew in a PL cache. That check would have to stay in the builder class
- not a default, but still related.
Isn't that a mark that the configuration is not designed well? I am not
sure how doable it is, but can we have syntactically correct
configuration implying semantically correct documentation? In the OL/PL
case, if PL implies not enabling WSC, we should make PL element instead
of attribute and not include the WSC attribute at all. If want to keep
WSC with different default, we could have different attribute for that
(with same name) so that user can look it up.
My 2c
Radim
--
Radim Vansa <rvansa(a)redhat.com>
JBoss DataGrid QA