IGNITE-4188, savepoints with atomic cache?

[Eduard Shangareev]

Guys,

I believe, that it's not the case when we should change default behaviour.
So, #1 and make it default in 3.0.

On Wed, Jun 13, 2018 at 6:46 PM, Dmitrii Ryabov <[hidden email]>
wrote:

> Vote for #2. I think no one will change this defaults in configuration in
> #1.
>
> 2018-06-13 18:29 GMT+03:00 Anton Vinogradov <[hidden email]>:
>
> > Vote for #2 since it can shed light on hidden bug at production.
> >
> > ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk <
> [hidden email]
> > >:
> >
> > > Igniters,
> > >
> > > Bumping up this discussion. The fix has been implemented and it is fine
> > > from the technical point of view, but since the fix did not make it to
> > the
> > > Ignite 2.0, the implemented fix [1] now will be a breaking change for
> > > current Ignite users.
> > >
> > > I see the following options:
> > > 1) Have the fix merged, but do not change the defaults - atomic caches
> > will
> > > still be allowed in transactions by default and only configuration
> change
> > > will make Ignite throw exceptions in this case
> > > 2) Have the fix merged as is and describe this change in the release
> > notes
> > > 3) Postpone the fix until Ignite 3.0
> > >
> > > I would vote for option #1 and change only the defaults in Ignite 3.0.
> > >
> > > Thoughts?
> > >
> > > [1] https://issues.apache.org/jira/browse/IGNITE-2313
> > >
> > > ср, 5 апр. 2017 г. в 22:53, Дмитрий Рябов <[hidden email]>:
> > >
> > > > IGNITE-2313 done, can you review it?
> > > >
> > > > PR: https://github.com/apache/ignite/pull/1709/files
> > > > JIRA: https://issues.apache.org/jira/browse/IGNITE-2313
> > > > CI: http://ci.ignite.apache.org/viewType.html?buildTypeId=
> > > > IgniteTests_RatJavadoc&branch_IgniteTests=pull%2F1709%
> > > > 2Fhead&tab=buildTypeStatusDiv
> > > >
> > > > 2017-03-29 20:58 GMT+03:00 Denis Magda <[hidden email]>:
> > > >
> > > > > Sorry, I get lost in tickets.
> > > > >
> > > > > Yes, IGNITE-2313 has to be completed in 2.0 if we want to makes
> this
> > > > > change.
> > > > >
> > > > > —
> > > > > Denis
> > > > >
> > > > > > On Mar 29, 2017, at 2:12 AM, Дмитрий Рябов <
> [hidden email]>
> > > > > wrote:
> > > > > >
> > > > > > Savepoints marked for 2.1, exceptions for 2.0. Do you want me to
> > make
> > > > > > exceptions first?
> > > > > >
> > > > > > 2017-03-29 11:24 GMT+03:00 Дмитрий Рябов <[hidden email]
> >:
> > > > > >
> > > > > >> Finish savepoints or flag&exceptions for atomic operations?
> > > > > >> Not sure about savepoints. Exceptions - yes.
> > https://issues.apache.
> > > > > >> org/jira/browse/IGNITE-2313 isn't it?
> > > > > >>
> > > > > >> 2017-03-29 2:12 GMT+03:00 Denis Magda <[hidden email]>:
> > > > > >>
> > > > > >>> If we want to make the exception based approach the default one
> > > then
> > > > > the
> > > > > >>> task has to be released in 2.0.
> > > > > >>>
> > > > > >>> Dmitriy Ryabov, do you think you can finish it (dev, review,
> QA)
> > by
> > > > the
> > > > > >>> code freeze data (April 14)?
> > > > > >>>
> > > > > >>> —
> > > > > >>> Denis
> > > > > >>>
> > > > > >>>> On Mar 28, 2017, at 11:57 AM, Dmitriy Setrakyan <
> > > > > [hidden email]>
> > > > > >>> wrote:
> > > > > >>>>
> > > > > >>>> On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
> > > > > >>> [hidden email]>
> > > > > >>>> wrote:
> > > > > >>>>
> > > > > >>>>> I think updating an Atomic cache from within a transaction
> > > > perfectly
> > > > > >>> makes
> > > > > >>>>> sense. For example for some kind of operations logging and so
> > > > forth.
> > > > > >>> Still
> > > > > >>>>> I agree that this can be error prone and forbidden by
> default.
> > I
> > > > > agree
> > > > > >>> with
> > > > > >>>>> Yakov that by default we should throw an exception and have
> > some
> > > > kind
> > > > > >>> of
> > > > > >>>>> flag (on cache or on TX?) to be able to explicitly enable
> this
> > > > > >>> behavior.
> > > > > >>>>>
> > > > > >>>>
> > > > > >>>>
> > > > > >>>> Agree, this sounds like a good idea.
> > > > > >>>
> > > > > >>>
> > > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>

[Ivan Rakov]

+1 to Eduard.

It's a reasonable change, but we can't just break working code of all
the guys that access atomic caches in their transactions. If we try, we
will end up with another emergency release.

Best Regards,
Ivan Rakov

On 13.06.2018 19:13, Eduard Shangareev wrote:

> Guys,
>
> I believe, that it's not the case when we should change default behaviour.
> So, #1 and make it default in 3.0.
>
> On Wed, Jun 13, 2018 at 6:46 PM, Dmitrii Ryabov <[hidden email]>
> wrote:
>
>> Vote for #2. I think no one will change this defaults in configuration in
>> #1.
>>
>> 2018-06-13 18:29 GMT+03:00 Anton Vinogradov <[hidden email]>:
>>
>>> Vote for #2 since it can shed light on hidden bug at production.
>>>
>>> ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk <
>> [hidden email]
>>>> :
>>>> Igniters,
>>>>
>>>> Bumping up this discussion. The fix has been implemented and it is fine
>>>> from the technical point of view, but since the fix did not make it to
>>> the
>>>> Ignite 2.0, the implemented fix [1] now will be a breaking change for
>>>> current Ignite users.
>>>>
>>>> I see the following options:
>>>> 1) Have the fix merged, but do not change the defaults - atomic caches
>>> will
>>>> still be allowed in transactions by default and only configuration
>> change
>>>> will make Ignite throw exceptions in this case
>>>> 2) Have the fix merged as is and describe this change in the release
>>> notes
>>>> 3) Postpone the fix until Ignite 3.0
>>>>
>>>> I would vote for option #1 and change only the defaults in Ignite 3.0.
>>>>
>>>> Thoughts?
>>>>
>>>> [1] https://issues.apache.org/jira/browse/IGNITE-2313
>>>>
>>>> ср, 5 апр. 2017 г. в 22:53, Дмитрий Рябов <[hidden email]>:
>>>>
>>>>> IGNITE-2313 done, can you review it?
>>>>>
>>>>> PR: https://github.com/apache/ignite/pull/1709/files
>>>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-2313
>>>>> CI: http://ci.ignite.apache.org/viewType.html?buildTypeId=
>>>>> IgniteTests_RatJavadoc&branch_IgniteTests=pull%2F1709%
>>>>> 2Fhead&tab=buildTypeStatusDiv
>>>>>
>>>>> 2017-03-29 20:58 GMT+03:00 Denis Magda <[hidden email]>:
>>>>>
>>>>>> Sorry, I get lost in tickets.
>>>>>>
>>>>>> Yes, IGNITE-2313 has to be completed in 2.0 if we want to makes
>> this
>>>>>> change.
>>>>>>
>>>>>> —
>>>>>> Denis
>>>>>>
>>>>>>> On Mar 29, 2017, at 2:12 AM, Дмитрий Рябов <
>> [hidden email]>
>>>>>> wrote:
>>>>>>> Savepoints marked for 2.1, exceptions for 2.0. Do you want me to
>>> make
>>>>>>> exceptions first?
>>>>>>>
>>>>>>> 2017-03-29 11:24 GMT+03:00 Дмитрий Рябов <[hidden email]
>>> :
>>>>>>>> Finish savepoints or flag&exceptions for atomic operations?
>>>>>>>> Not sure about savepoints. Exceptions - yes.
>>> https://issues.apache.
>>>>>>>> org/jira/browse/IGNITE-2313 isn't it?
>>>>>>>>
>>>>>>>> 2017-03-29 2:12 GMT+03:00 Denis Magda <[hidden email]>:
>>>>>>>>
>>>>>>>>> If we want to make the exception based approach the default one
>>>> then
>>>>>> the
>>>>>>>>> task has to be released in 2.0.
>>>>>>>>>
>>>>>>>>> Dmitriy Ryabov, do you think you can finish it (dev, review,
>> QA)
>>> by
>>>>> the
>>>>>>>>> code freeze data (April 14)?
>>>>>>>>>
>>>>>>>>> —
>>>>>>>>> Denis
>>>>>>>>>
>>>>>>>>>> On Mar 28, 2017, at 11:57 AM, Dmitriy Setrakyan <
>>>>>> [hidden email]>
>>>>>>>>> wrote:
>>>>>>>>>> On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
>>>>>>>>> [hidden email]>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> I think updating an Atomic cache from within a transaction
>>>>> perfectly
>>>>>>>>> makes
>>>>>>>>>>> sense. For example for some kind of operations logging and so
>>>>> forth.
>>>>>>>>> Still
>>>>>>>>>>> I agree that this can be error prone and forbidden by
>> default.
>>> I
>>>>>> agree
>>>>>>>>> with
>>>>>>>>>>> Yakov that by default we should throw an exception and have
>>> some
>>>>> kind
>>>>>>>>> of
>>>>>>>>>>> flag (on cache or on TX?) to be able to explicitly enable
>> this
>>>>>>>>> behavior.
>>>>>>>>>>
>>>>>>>>>> Agree, this sounds like a good idea.
>>>>>>>>>
>>>>>>

[dkarachentsev]

I would vote for 1 or 3 (this I like more), but not 2 as such breaking
change involves more pain to our users. Let it be in 3.0 and included in
migration guide, I don't see much drawbacks here.

Thanks!

13.06.2018 19:20, Ivan Rakov пишет:

> +1 to Eduard.
>
> It's a reasonable change, but we can't just break working code of all
> the guys that access atomic caches in their transactions. If we try,
> we will end up with another emergency release.
>
> Best Regards,
> Ivan Rakov
>
> On 13.06.2018 19:13, Eduard Shangareev wrote:
>> Guys,
>>
>> I believe, that it's not the case when we should change default
>> behaviour.
>> So, #1 and make it default in 3.0.
>>
>> On Wed, Jun 13, 2018 at 6:46 PM, Dmitrii Ryabov <[hidden email]>
>> wrote:
>>
>>> Vote for #2. I think no one will change this defaults in
>>> configuration in
>>> #1.
>>>
>>> 2018-06-13 18:29 GMT+03:00 Anton Vinogradov <[hidden email]>:
>>>
>>>> Vote for #2 since it can shed light on hidden bug at production.
>>>>
>>>> ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk <
>>> [hidden email]
>>>>> :
>>>>> Igniters,
>>>>>
>>>>> Bumping up this discussion. The fix has been implemented and it is
>>>>> fine
>>>>> from the technical point of view, but since the fix did not make
>>>>> it to
>>>> the
>>>>> Ignite 2.0, the implemented fix [1] now will be a breaking change for
>>>>> current Ignite users.
>>>>>
>>>>> I see the following options:
>>>>> 1) Have the fix merged, but do not change the defaults - atomic
>>>>> caches
>>>> will
>>>>> still be allowed in transactions by default and only configuration
>>> change
>>>>> will make Ignite throw exceptions in this case
>>>>> 2) Have the fix merged as is and describe this change in the release
>>>> notes
>>>>> 3) Postpone the fix until Ignite 3.0
>>>>>
>>>>> I would vote for option #1 and change only the defaults in Ignite
>>>>> 3.0.
>>>>>
>>>>> Thoughts?
>>>>>
>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-2313
>>>>>
>>>>> ср, 5 апр. 2017 г. в 22:53, Дмитрий Рябов <[hidden email]>:
>>>>>
>>>>>> IGNITE-2313 done, can you review it?
>>>>>>
>>>>>> PR: https://github.com/apache/ignite/pull/1709/files
>>>>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-2313
>>>>>> CI: http://ci.ignite.apache.org/viewType.html?buildTypeId=
>>>>>> IgniteTests_RatJavadoc&branch_IgniteTests=pull%2F1709%
>>>>>> 2Fhead&tab=buildTypeStatusDiv
>>>>>>
>>>>>> 2017-03-29 20:58 GMT+03:00 Denis Magda <[hidden email]>:
>>>>>>
>>>>>>> Sorry, I get lost in tickets.
>>>>>>>
>>>>>>> Yes, IGNITE-2313 has to be completed in 2.0 if we want to makes
>>> this
>>>>>>> change.
>>>>>>>
>>>>>>> —
>>>>>>> Denis
>>>>>>>
>>>>>>>> On Mar 29, 2017, at 2:12 AM, Дмитрий Рябов <
>>> [hidden email]>
>>>>>>> wrote:
>>>>>>>> Savepoints marked for 2.1, exceptions for 2.0. Do you want me to
>>>> make
>>>>>>>> exceptions first?
>>>>>>>>
>>>>>>>> 2017-03-29 11:24 GMT+03:00 Дмитрий Рябов <[hidden email]
>>>> :
>>>>>>>>> Finish savepoints or flag&exceptions for atomic operations?
>>>>>>>>> Not sure about savepoints. Exceptions - yes.
>>>> https://issues.apache.
>>>>>>>>> org/jira/browse/IGNITE-2313 isn't it?
>>>>>>>>>
>>>>>>>>> 2017-03-29 2:12 GMT+03:00 Denis Magda <[hidden email]>:
>>>>>>>>>
>>>>>>>>>> If we want to make the exception based approach the default one
>>>>> then
>>>>>>> the
>>>>>>>>>> task has to be released in 2.0.
>>>>>>>>>>
>>>>>>>>>> Dmitriy Ryabov, do you think you can finish it (dev, review,
>>> QA)
>>>> by
>>>>>> the
>>>>>>>>>> code freeze data (April 14)?
>>>>>>>>>>
>>>>>>>>>> —
>>>>>>>>>> Denis
>>>>>>>>>>
>>>>>>>>>>> On Mar 28, 2017, at 11:57 AM, Dmitriy Setrakyan <
>>>>>>> [hidden email]>
>>>>>>>>>> wrote:
>>>>>>>>>>> On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
>>>>>>>>>> [hidden email]>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> I think updating an Atomic cache from within a transaction
>>>>>> perfectly
>>>>>>>>>> makes
>>>>>>>>>>>> sense. For example for some kind of operations logging and so
>>>>>> forth.
>>>>>>>>>> Still
>>>>>>>>>>>> I agree that this can be error prone and forbidden by
>>> default.
>>>> I
>>>>>>> agree
>>>>>>>>>> with
>>>>>>>>>>>> Yakov that by default we should throw an exception and have
>>>> some
>>>>>> kind
>>>>>>>>>> of
>>>>>>>>>>>> flag (on cache or on TX?) to be able to explicitly enable
>>> this
>>>>>>>>>> behavior.
>>>>>>>>>>>
>>>>>>>>>>> Agree, this sounds like a good idea.
>>>>>>>>>>
>>>>>>>
>

[Dmitrii Ryabov]

Ok, go #1. I'll create ticket for defaults change and make appropriate
changes in current PR with link to new ticket.

2018-06-14 10:56 GMT+03:00 Dmitry Karachentsev <[hidden email]>:

> I would vote for 1 or 3 (this I like more), but not 2 as such breaking
> change involves more pain to our users. Let it be in 3.0 and included in
> migration guide, I don't see much drawbacks here.
>
> Thanks!
>
> 13.06.2018 19:20, Ivan Rakov пишет:
>
> +1 to Eduard.
>>
>> It's a reasonable change, but we can't just break working code of all the
>> guys that access atomic caches in their transactions. If we try, we will
>> end up with another emergency release.
>>
>> Best Regards,
>> Ivan Rakov
>>
>> On 13.06.2018 19:13, Eduard Shangareev wrote:
>>
>>> Guys,
>>>
>>> I believe, that it's not the case when we should change default
>>> behaviour.
>>> So, #1 and make it default in 3.0.
>>>
>>> On Wed, Jun 13, 2018 at 6:46 PM, Dmitrii Ryabov <[hidden email]>
>>> wrote:
>>>
>>> Vote for #2. I think no one will change this defaults in configuration in
>>>> #1.
>>>>
>>>> 2018-06-13 18:29 GMT+03:00 Anton Vinogradov <[hidden email]>:
>>>>
>>>> Vote for #2 since it can shed light on hidden bug at production.
>>>>>
>>>>> ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk <
>>>>>
>>>> [hidden email]
>>>>
>>>>> :
>>>>>> Igniters,
>>>>>>
>>>>>> Bumping up this discussion. The fix has been implemented and it is
>>>>>> fine
>>>>>> from the technical point of view, but since the fix did not make it to
>>>>>>
>>>>> the
>>>>>
>>>>>> Ignite 2.0, the implemented fix [1] now will be a breaking change for
>>>>>> current Ignite users.
>>>>>>
>>>>>> I see the following options:
>>>>>> 1) Have the fix merged, but do not change the defaults - atomic caches
>>>>>>
>>>>> will
>>>>>
>>>>>> still be allowed in transactions by default and only configuration
>>>>>>
>>>>> change
>>>>
>>>>> will make Ignite throw exceptions in this case
>>>>>> 2) Have the fix merged as is and describe this change in the release
>>>>>>
>>>>> notes
>>>>>
>>>>>> 3) Postpone the fix until Ignite 3.0
>>>>>>
>>>>>> I would vote for option #1 and change only the defaults in Ignite 3.0.
>>>>>>
>>>>>> Thoughts?
>>>>>>
>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-2313
>>>>>>
>>>>>> ср, 5 апр. 2017 г. в 22:53, Дмитрий Рябов <[hidden email]>:
>>>>>>
>>>>>> IGNITE-2313 done, can you review it?
>>>>>>>
>>>>>>> PR: https://github.com/apache/ignite/pull/1709/files
>>>>>>> JIRA: https://issues.apache.org/jira/browse/IGNITE-2313
>>>>>>> CI: http://ci.ignite.apache.org/viewType.html?buildTypeId=
>>>>>>> IgniteTests_RatJavadoc&branch_IgniteTests=pull%2F1709%
>>>>>>> 2Fhead&tab=buildTypeStatusDiv
>>>>>>>
>>>>>>> 2017-03-29 20:58 GMT+03:00 Denis Magda <[hidden email]>:
>>>>>>>
>>>>>>> Sorry, I get lost in tickets.
>>>>>>>>
>>>>>>>> Yes, IGNITE-2313 has to be completed in 2.0 if we want to makes
>>>>>>>>
>>>>>>> this
>>>>
>>>>> change.
>>>>>>>>
>>>>>>>> —
>>>>>>>> Denis
>>>>>>>>
>>>>>>>> On Mar 29, 2017, at 2:12 AM, Дмитрий Рябов <
>>>>>>>>>
>>>>>>>> [hidden email]>
>>>>
>>>>> wrote:
>>>>>>>>
>>>>>>>>> Savepoints marked for 2.1, exceptions for 2.0. Do you want me to
>>>>>>>>>
>>>>>>>> make
>>>>>
>>>>>> exceptions first?
>>>>>>>>>
>>>>>>>>> 2017-03-29 11:24 GMT+03:00 Дмитрий Рябов <[hidden email]
>>>>>>>>>
>>>>>>>> :
>>>>>
>>>>>> Finish savepoints or flag&exceptions for atomic operations?
>>>>>>>>>> Not sure about savepoints. Exceptions - yes.
>>>>>>>>>>
>>>>>>>>> https://issues.apache.
>>>>>
>>>>>> org/jira/browse/IGNITE-2313 isn't it?
>>>>>>>>>>
>>>>>>>>>> 2017-03-29 2:12 GMT+03:00 Denis Magda <[hidden email]>:
>>>>>>>>>>
>>>>>>>>>> If we want to make the exception based approach the default one
>>>>>>>>>>>
>>>>>>>>>> then
>>>>>>
>>>>>>> the
>>>>>>>>
>>>>>>>>> task has to be released in 2.0.
>>>>>>>>>>>
>>>>>>>>>>> Dmitriy Ryabov, do you think you can finish it (dev, review,
>>>>>>>>>>>
>>>>>>>>>> QA)
>>>>
>>>>> by
>>>>>
>>>>>> the
>>>>>>>
>>>>>>>> code freeze data (April 14)?
>>>>>>>>>>>
>>>>>>>>>>> —
>>>>>>>>>>> Denis
>>>>>>>>>>>
>>>>>>>>>>> On Mar 28, 2017, at 11:57 AM, Dmitriy Setrakyan <
>>>>>>>>>>>>
>>>>>>>>>>> [hidden email]>
>>>>>>>>
>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
>>>>>>>>>>>>
>>>>>>>>>>> [hidden email]>
>>>>>>>>>>>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> I think updating an Atomic cache from within a transaction
>>>>>>>>>>>>>
>>>>>>>>>>>> perfectly
>>>>>>>
>>>>>>>> makes
>>>>>>>>>>>
>>>>>>>>>>>> sense. For example for some kind of operations logging and so
>>>>>>>>>>>>>
>>>>>>>>>>>> forth.
>>>>>>>
>>>>>>>> Still
>>>>>>>>>>>
>>>>>>>>>>>> I agree that this can be error prone and forbidden by
>>>>>>>>>>>>>
>>>>>>>>>>>> default.
>>>>
>>>>> I
>>>>>
>>>>>> agree
>>>>>>>>
>>>>>>>>> with
>>>>>>>>>>>
>>>>>>>>>>>> Yakov that by default we should throw an exception and have
>>>>>>>>>>>>>
>>>>>>>>>>>> some
>>>>>
>>>>>> kind
>>>>>>>
>>>>>>>> of
>>>>>>>>>>>
>>>>>>>>>>>> flag (on cache or on TX?) to be able to explicitly enable
>>>>>>>>>>>>>
>>>>>>>>>>>> this
>>>>
>>>>> behavior.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Agree, this sounds like a good idea.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>
>