Guys, I suggest we finalize the configuration changes in the original ticket then: https://issues.apache.org/jira/browse/IGNITE-6030 and proceed with the changes.
2017-09-23 17:08 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > Can we specify what metrics will look like? I think we should not just > blindly merge them. > > On Fri, Sep 22, 2017 at 11:01 PM, Vladimir Ozerov <voze...@gridgain.com> > wrote: > > > Denis, > > > > Makes sense. Thanks for catching it! > > > > On Sat, Sep 23, 2017 at 8:45 AM, Denis Magda <dma...@apache.org> wrote: > > > > > If we’re taking the consolidation path for Memory and Persistence > > > configurations then it makes sense to merge MemoryMetrics [1] and > > > PersistenceMetrics [2] plus their JMX beans. > > > > > > Agree? > > > > > > [1] https://ignite.apache.org/releases/latest/javadoc/org/ > > > apache/ignite/MemoryMetrics.html <https://ignite.apache.org/ > > > releases/latest/javadoc/org/apache/ignite/MemoryMetrics.html> > > > [2] https://ignite.apache.org/releases/latest/javadoc/org/ > apache/ignite/ > > > PersistenceMetrics.html > > > > > > — > > > Denis > > > > > > > On Sep 22, 2017, at 10:18 PM, Dmitriy Setrakyan < > dsetrak...@apache.org > > > > > > wrote: > > > > > > > > I like it. > > > > > > > > Alexey G, can you please chime in? > > > > > > > > D. > > > > > > > > On Fri, Sep 22, 2017 at 11:33 AM, Vladimir Ozerov < > > voze...@gridgain.com> > > > > wrote: > > > > > > > >> Guys, > > > >> > > > >> Here is my proposal: > > > >> > > > >> 1) MemoryPolicyConfiguration is renamed to > *DataRegionConfiguration*. > > > >> 2) PersistenceConfiguration is merged with MemoryConfiguration and > > > renamed > > > >> to ... *DataStorageConfiguration*! It has: common memory settings > > (e.g. > > > >> default data region), persistence settings (e.g. WAL) and a list of > > > >> DataRegionConfiguration beans. > > > >> > > > >> What we have in the end: > > > >> > > > >> <property name="dataConfiguration"> > > > >> <bean class="o.a.i.DataConfiguration"> > > > >> <property name="pageSize" value="8192" /> > > > >> <property name="persistentStorePath" value="/my/path" /> > > > >> <property name="dataRegions"> > > > >> <list> > > > >> <bean class="o.a.i.DataRegionConfiguration"> > > > >> <property name="name" value="VOLATILE" /> > > > >> <property name="maxSize" value="1_000_000_000" /> > > > >> </bean> > > > >> <bean class="o.a.i.DataRegionConfiguration"> > > > >> <property name="name" value="PERSISTENT" /> > > > >> <property name="maxSize" value="1_000_000_000" /> > > > >> <property name="persistent" value="true" /> > > > >> </bean> > > > >> </list> > > > >> </property> > > > >> </bean> > > > >> </property> > > > >> > > > >> > > > >> Makes sense? > > > >> > > > >> Vladimir. > > > >> > > > >> > > > >> On Thu, Sep 21, 2017 at 7:04 AM, Dmitriy Setrakyan < > > > dsetrak...@apache.org> > > > >> wrote: > > > >> > > > >>> Firstly all, why not call it DataPolicy instead of MemoryPolicy? > > > >> Secondly, > > > >>> why not set data policies directly on IgniteConfiguration. And > > lastly, > > > >> how > > > >>> about we combine memory and disk properties in one bean with clear > > > naming > > > >>> convention? > > > >>> > > > >>> Here is the example. Note that all properties above must start with > > > with > > > >>> "Memory" or "Disk". > > > >>> > > > >>> *IgniteConfiguration cfg = new IgniteConfiguration();* > > > >>> > > > >>> > > > >>> > > > >>> > > > >>>> > > > >>>> > > > >>>> > > > >>>> > > > >>>> *cfg.setDataPolicies( new DataPolicyConfiguration() > > > >>>> .setName("bla"), .setMemoryMaxSize(1024), // must be > greater > > > >> than > > > >>> 0, > > > >>>> since memory always needs to be enabled. > .setDiskMaxSize(0), > > // > > > >> if > > > >>>> greater than 0, then persistence is enabled. );* > > > >>> > > > >>> > > > >>> > > > >>> I think this approach is much more concise and straight forward. > What > > > do > > > >>> you think? > > > >>> > > > >>> D. > > > >>> > > > >>> On Wed, Sep 20, 2017 at 4:55 AM, Vladimir Ozerov < > > voze...@gridgain.com > > > > > > > >>> wrote: > > > >>> > > > >>>> I prefer the second. Composition over inheritance - this is how > all > > > our > > > >>>> configuration is crafted. > > > >>>> > > > >>>> E.g. we do not have "CacheConfiguration" and " > > > >>>> StoreEnabledCacheConfiguration". > > > >>>> Instead, we have "CacheConfiguration.setCacheStoreFactory". > > > >>>> > > > >>>> On Wed, Sep 20, 2017 at 2:46 PM, Alexey Goncharuk < > > > >>>> alexey.goncha...@gmail.com> wrote: > > > >>>> > > > >>>>> Reiterating this based on some feedback from PDS users. > > > >>>>> > > > >>>>> It might be confusing to configure persistence with > "MemoryPolicy", > > > >> so > > > >>>>> another approach is to deprecate the old names and introduce a > new > > > >> name > > > >>>>> "DataRegion" because it reflects the actual state when data is > > stored > > > >>> on > > > >>>>> disk and partially in memory. I have two options in mind, each of > > > >> them > > > >>>>> looks acceptable to me, so I would like to have some feedback > from > > > >> the > > > >>>>> community. Old configuration names will be deprecated (but still > be > > > >>> taken > > > >>>>> if used for backward compatibility). Note, that old names > > deprecation > > > >>>>> handles default configuration compatibility very nicely - current > > PDS > > > >>>> users > > > >>>>> will not need to change anything to keep everything working. The > > two > > > >>>>> options I mentioned are below: > > > >>>>> > > > >>>>> * we have two separate classes for in-memory and persisted data > > > >>> regions, > > > >>>>> so the configuration would look like so: > > > >>>>> > > > >>>>> IgniteConfiguration cfg = new IgniteConfiguration(); > > > >>>>> > > > >>>>> cfg.setDataRegionsConfiguration(new DataRegionsConfiguration() > > > >>>>> .setDataRegions( > > > >>>>> new MemoryDataRegion() > > > >>>>> .setName("volatileCaches") > > > >>>>> .setMaxMemorySize(...), > > > >>>>> new PersistentDataRegion() > > > >>>>> .setName("persistentCaches") > > > >>>>> .setMaxMemorySize(...) > > > >>>>> .setMaxDiskSize())); > > > >>>>> > > > >>>>> cfg.setPersistentStoreConfiguration(new > > > >> PersistentStoreConfiguration() > > > >>> ); > > > >>>>> > > > >>>>> > > > >>>>> * we have one class for data region configuration, but it will > > have a > > > >>>>> sub-bean for persistence configuration: > > > >>>>> > > > >>>>> IgniteConfiguration cfg = new IgniteConfiguration(); > > > >>>>> > > > >>>>> cfg.setDataRegionsConfiguration(new DataRegionsConfiguration() > > > >>>>> .setDataRegions( > > > >>>>> new DataRegion() > > > >>>>> .setName("volatileCaches") > > > >>>>> .setMaxMemorySize(...), > > > >>>>> new DataRegion() > > > >>>>> .setName("persistentCaches") > > > >>>>> .setMaxMemorySize(...), > > > >>>>> .setPersistenceConfiguration( > > > >>>>> new DataRegionPersistenceConfiguration() > > > >>>>> .setMaxDiskSize(...)))); > > > >>>>> > > > >>>>> cfg.setPersistentStoreConfiguration(new > > > >> PersistentStoreConfiguration() > > > >>> ); > > > >>>>> > > > >>>> > > > >>> > > > >> > > > > > > > > >
