Is the storage path the root folder for the persistence or only the root path for the main storage?
On Wed, Oct 11, 2017 at 3:54 PM, Denis Magda <dma...@apache.org> wrote: > Ivan, > > Instead of “setStoragePath” I would suggest “setPersistencePath”. Left > some extra notes in the ticket. > > — > Denis > > > On Oct 11, 2017, at 4:30 AM, Ivan Rakov <ivan.glu...@gmail.com> wrote: > > > > Vladimir, > > > > Thanks for focusing on existing namings. Most of your suggestions really > sound better. I've posted my thoughts under your comment. > > > > By the way, we should decide two things: > > > > 1) Naming for methods for configuring store path. I suggest the > following: > > > > *setStoragePath* - for partition and index files > > *setWalPath* - for WAL files > > *walArchivePath* - for WAL archive files > > > > 2) Renaming *checkpointingFrequency* to *checkpointFrequency* (same with > *checkpointingPageBufferSize* and *checkpointingThreads*). Both options > sounds ok to me, let's see what community thinks. > > > > Best Regards, > > Ivan Rakov > > > > On 11.10.2017 14:05, Vladimir Ozerov wrote: > >> 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. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > > > >
