On Wed, Aug 20, 2014 at 3:40 PM, Radim Vansa <rva...@redhat.com> wrote:
> On 08/20/2014 12:16 PM, Dan Berindei wrote: > > > > > On Wed, Aug 20, 2014 at 12:27 PM, Galder Zamarreño <gal...@redhat.com> > wrote: > >> >> On 15 Aug 2014, at 12:41, Dan Berindei <dan.berin...@gmail.com> wrote: >> >> > >> > >> > >> > On Fri, Aug 15, 2014 at 11:37 AM, Galder Zamarreño <gal...@redhat.com> >> wrote: >> > >> > On 12 Aug 2014, at 22:41, Dan Berindei <dan.berin...@gmail.com> wrote: >> > >> >> >> >> >> >> >> >> On Tue, Aug 5, 2014 at 11:27 AM, Galder Zamarreño <gal...@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/main/resources/infinispan-defaults.xml >> >> > 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> > > Yes, that can be done, my point is that moving the defaults to XML doesn't necessarily simplify things. > > > >> >> 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. > > That sounds interesting, I guess I should track RadarGun changes more closely. > > >> > 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. > Pretty good idea, but I think we want to postpone the next configuration overhaul for 8.0 :) > My 2c > > > Radim > > -- > Radim Vansa <rva...@redhat.com> <rva...@redhat.com> > JBoss DataGrid QA > > > _______________________________________________ > 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
