ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vladimir Ozerov <voze...@gridgain.com>
Subject Re: Persistence per memory policy configuration
Date Wed, 11 Oct 2017 11:05:15 GMT
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.glukos@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.glukos@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.glukos@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.glukos@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.glukos@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.goncharuk@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.glukos@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.glukos@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 <
>>>>>>>>>>>>>> dmagda@apache.org>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Oct 1, 2017, at 4:41 AM, Ivan
Rakov <ivan.glukos@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.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message