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() > >>> ); > >>>>> > >>>> > >>> > >> > >
