Dmitriy, we are not renaming classes, we are refactoring them. Prior to this design, it was impossible to set persistence configuration on per-cache basis. With this new design, users will be able to configure some caches to be in-memory only and others to be on disk.
Given that we are already refactoring, it only makes sense to pick better, more appropriate names. D. On Tue, Sep 26, 2017 at 7:20 AM, Dmitry Pavlov <dpavlov....@gmail.com> wrote: > Vladimir, it is not clear for me, why we need to rename existing > configuration classes. Could you explain? And if we can't get consensus > now, should we pospond solution? > > My idea is that user needs this feature more than elegant names in > configuration. > > Moreover once MemoryPolicyConfiguration was introduced as Ignite term it is > simpler to keep it as is, than create new terms. > > Sincerely, > Dmitriy Pavlov > > вт, 26 сент. 2017 г. в 16:59, Vladimir Ozerov <voze...@gridgain.com>: > > > I do not understand why we should delay with renames. Yes, it will cause > > questions, so we will have to put additional efforts to docs and > JavaDocs. > > But the earlier we do that, the better. > > > > On Tue, Sep 26, 2017 at 4:50 PM, Dmitry Pavlov <dpavlov....@gmail.com> > > wrote: > > > > > Hi Igniters, sorry for late response. I didn't catch idea of renaming. > > > PersistentStoreConfiguration is intuitive, and > MemoryPolicyConfiguration > > is > > > intuitive also. > > > > > > If we rename these classes now, it will bring more questions to user > > list. > > > Users may be confused by old and new names and by trying to match it. > > More > > > issues can came from XML configs that users already have. > > > > > > Can we postpone the renaming? I suggest to finish 'persistence per > memory > > > policy' task without renaming, and create separate JIRA issue for > > creating > > > future decision? > > > > > > вт, 26 сент. 2017 г. в 15:25, Alexey Goncharuk < > > alexey.goncha...@gmail.com > > > >: > > > > > > > I do not like DurableMemoryConfiguration, because it's quite > confusing > > - > > > we > > > > configure in-memory caches using DurableMemory class, which > immediately > > > > suggests that everything will be persisted. I am not sure if this is > a > > > > right wording choice for the documentation either. I would go with > > > > DataStoreConfiguration and DataRegionConfiguration. > > > > > > > > --AG > > > > > > > > 2017-09-26 2:22 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > > > > > > > > > Given that we already have a notion of CacheStore which comes from > > > JCache > > > > > spec, I think having other stores may get confusing. I like > > > > > DurableMemoryConfiguration. > > > > > > > > > > Other opinions? > > > > > > > > > > D. > > > > > > > > > > On Mon, Sep 25, 2017 at 12:24 PM, Vladimir Ozerov < > > > voze...@gridgain.com> > > > > > wrote: > > > > > > > > > > > Dima, let's finalize the design first. > > > > > > > > > > > > As I understand, we are happy with idea to merge > > MemoryConfiguration > > > > > > and PersistentStoreConfiguration > > > > > > into something what I called DataConfiguration, and to rename > > > > > > MemoryPolicyConfiguration to DataRegionConfiguration. > > > > > > > > > > > > The only outstanding qurestion is whether DataConfiguration is a > > good > > > > > name. > > > > > > I am not very happy with it, so let's think of other > alternatives. > > > > Quick > > > > > > ideas: > > > > > > 1) StoreConfiguration - looks perfect to me - short and > > > > self-describing, > > > > > > but clashes a bit with existing CacheStore > > > > > > 2) DataStoreConfiguration - same as p.1, but the word "data" is > not > > > > > > necessary IMO > > > > > > 3) PageStoreConfiguration? GIves a hint to our page-based > > > architecture. > > > > > > 4) DurableMemoryConfiguration - aligns well with our docs, but I > do > > > not > > > > > > like it - too long and misleading > > > > > > > > > > > > Any other ideas? > > > > > > > > > > > > I would prefer to have either [StoreConfiguration + > > > > > > StoreRegionConfiguration] or [PageStoreConfiguration and > > > > > > PageStoreRegionConfiguration]. Looks clean and simple. > > > > > > > > > > > > Vladimir. > > > > > > > > > > > > > > > > > > On Mon, Sep 25, 2017 at 3:49 PM, Dmitriy Setrakyan < > > > > > dsetrak...@apache.org> > > > > > > wrote: > > > > > > > > > > > > > Vladimir, > > > > > > > > > > > > > > Can you please add the configuration example in the ticket? > > > > > > > > > > > > > > D. > > > > > > > > > > > > > > On Mon, Sep 25, 2017 at 12:20 AM, Alexey Goncharuk < > > > > > > > alexey.goncha...@gmail.com> wrote: > > > > > > > > > > > > > > > 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 DataRegionPersistenceConfigura > > > tion() > > > > > > > > > > > >>>>> .setMaxDiskSize(...)))); > > > > > > > > > > > >>>>> > > > > > > > > > > > >>>>> cfg.setPersistentStoreConfiguration(new > > > > > > > > > > > >> PersistentStoreConfiguration() > > > > > > > > > > > >>> ); > > > > > > > > > > > >>>>> > > > > > > > > > > > >>>> > > > > > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
