Ivan, I left some comments in the ticket [1], please take a look.
[1] https://issues.apache.org/jira/browse/IGNITE-6030?focusedCommentId=16200050&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16200050 On Wed, Oct 11, 2017 at 12:04 PM, Ivan Rakov <ivan.glu...@gmail.com> wrote: > Igniters, > > https://issues.apache.org/jira/browse/IGNITE-6030 is ready and enqueued > for TC run. > PR: https://github.com/apache/ignite/pull/2828 > > Everyone interested in new data storage configuration API, please pay > attention and review. > > > Best Regards, > Ivan Rakov > > > On 09.10.2017 12:40, Pavel Tupitsyn wrote: > >> Sounds good to me. >> >> On Mon, Oct 9, 2017 at 12:35 PM, Ivan Rakov <ivan.glu...@gmail.com> >> wrote: >> >> Pavel, >>> >>> Sounds reasonable. >>> I suggest to include both "data" and "configuration" to make it even more >>> obvious: >>> >>> set/getDefaultDataRegionConfiguration >>> set/getDataRegionConfigurations >>> >>> Best Regards, >>> Ivan Rakov >>> >>> >>> On 09.10.2017 10:51, Pavel Tupitsyn wrote: >>> >>> Sorry that I'm late to the party, but this looks inconsistent: >>>> >>>> DataStorageConfiguration defaultRegionConfiguration >>>> DataRegionConfiguration[] getDataRegions >>>> >>>> defaultRegionConfiguration + getRegionConfigurations >>>> - or - >>>> defaultDataRegion + getDataRegions >>>> >>>> Thoughts? >>>> >>>> On Mon, Oct 2, 2017 at 9:10 PM, Ivan Rakov <ivan.glu...@gmail.com> >>>> wrote: >>>> >>>> Denis, >>>> >>>>> Yes, you're right. All cache groups without specific data region >>>>> configured will be persistent. >>>>> And if you want to add another persistent data region, you should set >>>>> *isPeristenceEnabled* flag in its *DataRegionConfiguration* explictly. >>>>> >>>>> Best Regards, >>>>> Ivan Rakov >>>>> >>>>> >>>>> On 02.10.2017 21:01, Denis Magda wrote: >>>>> >>>>> Missed the point with defaults. Makes sense to me now. So to wrap this >>>>> >>>>>> up, if I want to enable the persistence globally and don’t have any >>>>>> regions >>>>>> configured explicitly I need to take the default region and switch the >>>>>> persistence on for it. Is my understanding correct? >>>>>> >>>>>> — >>>>>> Denis >>>>>> >>>>>> On Oct 2, 2017, at 10:57 AM, Ivan Rakov <ivan.glu...@gmail.com> >>>>>> wrote: >>>>>> >>>>>> Denis, why do you need to access an instance of the default region >>>>>>> bean? >>>>>>> If you want to set any parameter, just instantiate new bean with this >>>>>>> parameter set (like in XML snipped below). Other parameters will be >>>>>>> automatically initialized with their default values. >>>>>>> >>>>>>> Best Regards, >>>>>>> Ivan Rakov >>>>>>> >>>>>>> On 02.10.2017 19:28, Denis Magda wrote: >>>>>>> >>>>>>> <property name="dataStorageConfiguration"> >>>>>>> >>>>>>>> <bean class="org.apache.ignite.confi >>>>>>>>> >>>>>>>>>> guration.DataStorageConfiguration"> >>>>>>>>>> <property name="systemCacheInitialSize" >>>>>>>>>> value="#{100 >>>>>>>>>> * >>>>>>>>>> 1024 * 1024}"/> >>>>>>>>>> <property name="defaultRegionConfiguration"> >>>>>>>>>> <bean class="org.apache.ignite.confi >>>>>>>>>> guration.DataRegionConfiguration"> >>>>>>>>>> <property name="maxSize" value="#{5 * >>>>>>>>>> 1024 * >>>>>>>>>> 102 * 1024}"/> >>>>>>>>>> </bean> >>>>>>>>>> </property> >>>>>>>>>> </bean> >>>>>>>>>> </property> >>>>>>>>>> >>>>>>>>>> In other data regions persistence will be disabled by default. >>>>>>>>>> >>>>>>>>> Ivan, how to get an instance to the default region bean and change >>>>>>>>> a >>>>>>>>> >>>>>>>> parameter? Obviously, if the goal is to enable the persistence I >>>>>>>> don’t want >>>>>>>> to create the default region bean from scratch. >>>>>>>> >>>>>>>> — >>>>>>>> Denis >>>>>>>> >>>>>>>> On Oct 2, 2017, at 9:11 AM, Ivan Rakov <ivan.glu...@gmail.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>> Agree with Alexey. >>>>>>>>> >>>>>>>>> Properties like *defaultDataRegionSize*, >>>>>>>>> *isDefaultPersistenceEnabled* >>>>>>>>> can confuse users who don't know that there's such thing as default >>>>>>>>> data >>>>>>>>> region. They can decide they are inherited by all data regions >>>>>>>>> where >>>>>>>>> size >>>>>>>>> and persistence flag are not explicitly set. >>>>>>>>> >>>>>>>>> Let's get rid of these properties and add >>>>>>>>> *defaultRegionConfiguration* >>>>>>>>> property with explicit configuration of default data region. >>>>>>>>> >>>>>>>>> Regarding XML configuration, changing size or persistence flag of >>>>>>>>> default data region will be just two lines longer (for bean >>>>>>>>> description): >>>>>>>>> >>>>>>>>> <property name="dataStorageConfiguration"> >>>>>>>>> >>>>>>>>> <bean class="org.apache.ignite.confi >>>>>>>>>> guration.DataStorageConfiguration"> >>>>>>>>>> <property name="systemCacheInitialSize" >>>>>>>>>> value="#{100 >>>>>>>>>> * >>>>>>>>>> 1024 * 1024}"/> >>>>>>>>>> <property name="defaultRegionConfiguration"> >>>>>>>>>> <bean class="org.apache.ignite.confi >>>>>>>>>> guration.DataRegionConfiguration"> >>>>>>>>>> <property name="maxSize" value="#{5 * >>>>>>>>>> 1024 * >>>>>>>>>> 102 * 1024}"/> >>>>>>>>>> </bean> >>>>>>>>>> </property> >>>>>>>>>> </bean> >>>>>>>>>> </property> >>>>>>>>>> >>>>>>>>>> In other data regions persistence will be disabled by default. >>>>>>>>>> >>>>>>>>> I've updated draft in https://issues.apache.org/jira >>>>>>>>> /browse/IGNITE-6030 with these changes. >>>>>>>>> >>>>>>>>> Best Regards, >>>>>>>>> Ivan Rakov >>>>>>>>> >>>>>>>>> On 02.10.2017 18:35, Denis Magda wrote: >>>>>>>>> >>>>>>>>> To resolve this, I suggest to >>>>>>>>> >>>>>>>>>> introduce just another field defaultRegionConfiguration and get >>>>>>>>>>> rid >>>>>>>>>>> of >>>>>>>>>>> other defaults in DataStorageConfiguration. >>>>>>>>>>> >>>>>>>>>>> Won’t it complicate the configuration from a Spring XML file? I’m >>>>>>>>>>> >>>>>>>>>> not >>>>>>>>>> an expert in Spring so how do I get defaultRegionConfiguration >>>>>>>>>> bean >>>>>>>>>> first >>>>>>>>>> to change any parameter? >>>>>>>>>> >>>>>>>>>> — >>>>>>>>>> Denis >>>>>>>>>> >>>>>>>>>> On Oct 2, 2017, at 8:30 AM, Alexey Goncharuk < >>>>>>>>>> >>>>>>>>>> alexey.goncha...@gmail.com> wrote: >>>>>>>>>>> >>>>>>>>>>> Agree with Vladimir. If we are to implement this, we would either >>>>>>>>>>> need to >>>>>>>>>>> have a Boolean (non-primitive) for persistenceEnabled on >>>>>>>>>>> DataRegionConfiguration, or introduce an enum for this field >>>>>>>>>>> which >>>>>>>>>>> is also >>>>>>>>>>> an overkill. On the other hand, one can assume that the defaults >>>>>>>>>>> we >>>>>>>>>>> are >>>>>>>>>>> talking about are actually inherited. To resolve this, I suggest >>>>>>>>>>> to >>>>>>>>>>> introduce just another field defaultRegionConfiguration and get >>>>>>>>>>> rid >>>>>>>>>>> of >>>>>>>>>>> other defaults in DataStorageConfiguration. >>>>>>>>>>> >>>>>>>>>>> Thoughts? >>>>>>>>>>> >>>>>>>>>>> 2017-10-02 15:19 GMT+03:00 Ivan Rakov <ivan.glu...@gmail.com>: >>>>>>>>>>> >>>>>>>>>>> Vladimir, >>>>>>>>>>> >>>>>>>>>>> I like your approach because it's easier to implement. >>>>>>>>>>>> >>>>>>>>>>>> However, user may be confused by setting >>>>>>>>>>>> *isDefaultPersistenceEnabled* >>>>>>>>>>>> flag and seeing that persistence is not enabled by default in >>>>>>>>>>>> custom memory >>>>>>>>>>>> region. I'll add clarifying Javadoc at this place. >>>>>>>>>>>> >>>>>>>>>>>> Best Regards, >>>>>>>>>>>> Ivan Rakov >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On 02.10.2017 11:28, Vladimir Ozerov wrote: >>>>>>>>>>>> >>>>>>>>>>>> Ivan, >>>>>>>>>>>> >>>>>>>>>>>> I do not think this is correct approach, because it will be hard >>>>>>>>>>>>> to >>>>>>>>>>>>> explain, and you will have to use "Boolean" instead of >>>>>>>>>>>>> "boolean" >>>>>>>>>>>>> for >>>>>>>>>>>>> DataRegionConfiguration. I do not think we need default >>>>>>>>>>>>> "persistence >>>>>>>>>>>>> enabled" for all regions. Instead, we should have "persistence >>>>>>>>>>>>> enabled" >>>>>>>>>>>>> flag for default region only. It should not be propagated to >>>>>>>>>>>>> custom >>>>>>>>>>>>> regions. >>>>>>>>>>>>> >>>>>>>>>>>>> On Mon, Oct 2, 2017 at 11:14 AM, Ivan Rakov < >>>>>>>>>>>>> ivan.glu...@gmail.com >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> Guys, I think I got the point now. >>>>>>>>>>>>> >>>>>>>>>>>>> Let's check the final design: >>>>>>>>>>>>> >>>>>>>>>>>>>> *DataStorageConfiguration* will have >>>>>>>>>>>>>> *isDefaultPersistenceEnabled* >>>>>>>>>>>>>> property (default = false), which will be used for enabling >>>>>>>>>>>>>> persistence >>>>>>>>>>>>>> in >>>>>>>>>>>>>> default data region. >>>>>>>>>>>>>> *DataRegionConfiguration* will have *isPersistenceEnabled* >>>>>>>>>>>>>> property, >>>>>>>>>>>>>> which >>>>>>>>>>>>>> will be used for enabling persistence in corresponding data >>>>>>>>>>>>>> region. If >>>>>>>>>>>>>> value is not set, value of *DataStorageConfiguration::isD >>>>>>>>>>>>>> efaultPersistenceEnabled* >>>>>>>>>>>>>> will be used by default. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Best Regards, >>>>>>>>>>>>>> Ivan Rakov >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> On 02.10.2017 7:49, Dmitriy Setrakyan wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Mon, Oct 2, 2017 at 7:46 AM, Denis Magda < >>>>>>>>>>>>>> dma...@apache.org> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Oct 1, 2017, at 4:41 AM, Ivan Rakov <ivan.glu...@gmail.com >>>>>>>>>>>>>> > >>>>>>>>>>>>>> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> 1) You're right. I forgot to include the main flag in >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> DataRegionConfiguration - *isPersistenceEnabled*. Persistence >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> will be >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> enabled globally if at least one memory region has this >>>>>>>>>>>>>>>>> flag >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> set. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I’m confused. Why the persistence should be enabled >>>>>>>>>>>>>>>> *globally* >>>>>>>>>>>>>>>> if the >>>>>>>>>>>>>>>> purpose is to have it set for a specific region? If it’s >>>>>>>>>>>>>>>> enabled for >>>>>>>>>>>>>>>> region >>>>>>>>>>>>>>>> A only, I don’t want to have it activated for region B. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Yes, you are right. By default the persistence will be >>>>>>>>>>>>>>>> disabled >>>>>>>>>>>>>>>> globally. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> But we should also give users a way to switch the default >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> behavior from >>>>>>>>>>>>>>> in-memory only (no-persistence) to persistence. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >
