ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Denis Magda <dma...@apache.org>
Subject Re: Persistence per memory policy configuration
Date Wed, 11 Oct 2017 22:54:09 GMT
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.glukos@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.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
View raw message