Re: IGNITE-4188, savepoints with atomic cache?

[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 :

> 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 
>>> 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 :

 Vote for #2 since it can shed light on hidden bug at production.
>
> ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk <
>
 alexey.goncha...@gmail.com

> :
>> 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, Дмитрий Рябов :
>>
>> 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_IgniteTests=pull%2F1709%
>>> 2Fhead=buildTypeStatusDiv
>>>
>>> 2017-03-29 20:58 GMT+03:00 Denis Magda :
>>>
>>> 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, Дмитрий Рябов <
>
 somefire...@gmail.com>

> 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 Дмитрий Рябов 
 :
>
>> Finish savepoints or flag 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 :
>>
>> 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 <

>>> dsetrak...@apache.org>

> wrote:
>>>
 On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <

>>> sergi.vlady...@gmail.com>
>>>
 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, 

Re: IGNITE-4188, savepoints with atomic cache?

[Dmitry Karachentsev]

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 
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 :


Vote for #2 since it can shed light on hidden bug at production.

ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk <

alexey.goncha...@gmail.com

:
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, Дмитрий Рябов :


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_IgniteTests=pull%2F1709%
2Fhead=buildTypeStatusDiv

2017-03-29 20:58 GMT+03:00 Denis Magda :


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, Дмитрий Рябов <

somefire...@gmail.com>

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 Дмитрий Рябов 
:

Finish savepoints or flag 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 :


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 <

dsetrak...@apache.org>

wrote:

On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <

sergi.vlady...@gmail.com>

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.

Re: IGNITE-4188, savepoints with atomic cache?

[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 
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 :


Vote for #2 since it can shed light on hidden bug at production.

ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk <

alexey.goncha...@gmail.com

:
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, Дмитрий Рябов :


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_IgniteTests=pull%2F1709%
2Fhead=buildTypeStatusDiv

2017-03-29 20:58 GMT+03:00 Denis Magda :


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, Дмитрий Рябов <

somefire...@gmail.com>

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 Дмитрий Рябов 
:

Finish savepoints or flag 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 :


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 <

dsetrak...@apache.org>

wrote:

On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <

sergi.vlady...@gmail.com>

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.

Re: 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 
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 :
>
> > Vote for #2 since it can shed light on hidden bug at production.
> >
> > ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk <
> alexey.goncha...@gmail.com
> > >:
> >
> > > 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, Дмитрий Рябов :
> > >
> > > > 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_IgniteTests=pull%2F1709%
> > > > 2Fhead=buildTypeStatusDiv
> > > >
> > > > 2017-03-29 20:58 GMT+03:00 Denis Magda :
> > > >
> > > > > 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, Дмитрий Рябов <
> somefire...@gmail.com>
> > > > > 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 Дмитрий Рябов  >:
> > > > > >
> > > > > >> Finish savepoints or flag 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 :
> > > > > >>
> > > > > >>> 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 <
> > > > > dsetrak...@apache.org>
> > > > > >>> wrote:
> > > > > 
> > > > >  On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
> > > > > >>> sergi.vlady...@gmail.com>
> > > > >  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.
> > > > > >>>
> > > > > >>>
> > > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>

Re: IGNITE-4188, savepoints with atomic cache?

[Dmitrii Ryabov]

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 :

> Vote for #2 since it can shed light on hidden bug at production.
>
> ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk  >:
>
> > 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, Дмитрий Рябов :
> >
> > > 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_IgniteTests=pull%2F1709%
> > > 2Fhead=buildTypeStatusDiv
> > >
> > > 2017-03-29 20:58 GMT+03:00 Denis Magda :
> > >
> > > > 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, Дмитрий Рябов 
> > > > 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 Дмитрий Рябов :
> > > > >
> > > > >> Finish savepoints or flag 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 :
> > > > >>
> > > > >>> 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 <
> > > > dsetrak...@apache.org>
> > > > >>> wrote:
> > > > 
> > > >  On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
> > > > >>> sergi.vlady...@gmail.com>
> > > >  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.
> > > > >>>
> > > > >>>
> > > > >>
> > > >
> > > >
> > >
> >
>

Re: IGNITE-4188, savepoints with atomic cache?

[Anton Vinogradov]

Vote for #2 since it can shed light on hidden bug at production.

ср, 13 июн. 2018 г. в 18:10, Alexey Goncharuk :

> 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, Дмитрий Рябов :
>
> > 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_IgniteTests=pull%2F1709%
> > 2Fhead=buildTypeStatusDiv
> >
> > 2017-03-29 20:58 GMT+03:00 Denis Magda :
> >
> > > 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, Дмитрий Рябов 
> > > 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 Дмитрий Рябов :
> > > >
> > > >> Finish savepoints or flag 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 :
> > > >>
> > > >>> 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 <
> > > dsetrak...@apache.org>
> > > >>> wrote:
> > > 
> > >  On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
> > > >>> sergi.vlady...@gmail.com>
> > >  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.
> > > >>>
> > > >>>
> > > >>
> > >
> > >
> >
>

Re: IGNITE-4188, savepoints with atomic cache?

[Alexey Goncharuk]

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, Дмитрий Рябов :

> 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_IgniteTests=pull%2F1709%
> 2Fhead=buildTypeStatusDiv
>
> 2017-03-29 20:58 GMT+03:00 Denis Magda :
>
> > 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, Дмитрий Рябов 
> > 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 Дмитрий Рябов :
> > >
> > >> Finish savepoints or flag 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 :
> > >>
> > >>> 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 <
> > dsetrak...@apache.org>
> > >>> wrote:
> > 
> >  On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
> > >>> sergi.vlady...@gmail.com>
> >  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.
> > >>>
> > >>>
> > >>
> >
> >
>

Re: IGNITE-4188, savepoints with atomic cache?

[Дмитрий Рябов]

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_IgniteTests=pull%2F1709%
2Fhead=buildTypeStatusDiv

2017-03-29 20:58 GMT+03:00 Denis Magda :

> 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, Дмитрий Рябов 
> 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 Дмитрий Рябов :
> >
> >> Finish savepoints or flag 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 :
> >>
> >>> 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 <
> dsetrak...@apache.org>
> >>> wrote:
> 
>  On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
> >>> sergi.vlady...@gmail.com>
>  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.
> >>>
> >>>
> >>
>
>

Re: IGNITE-4188, savepoints with atomic cache?

[Denis Magda]

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, Дмитрий Рябов  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 Дмитрий Рябов :
> 
>> Finish savepoints or flag 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 :
>> 
>>> 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 
>>> wrote:
 
 On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
>>> sergi.vlady...@gmail.com>
 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.
>>> 
>>> 
>> 

Re: IGNITE-4188, savepoints with atomic cache?

[Дмитрий Рябов]

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 Дмитрий Рябов :

> Finish savepoints or flag 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 :
>
>> 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 
>> wrote:
>> >
>> > On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
>> sergi.vlady...@gmail.com>
>> > 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.
>>
>>
>

Re: IGNITE-4188, savepoints with atomic cache?

[Дмитрий Рябов]

Finish savepoints or flag 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 :

> 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 
> wrote:
> >
> > On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin <
> sergi.vlady...@gmail.com>
> > 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.
>
>

Re: IGNITE-4188, savepoints with atomic cache?

[Denis Magda]

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  wrote:
> 
> On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin 
> 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.

Re: IGNITE-4188, savepoints with atomic cache?

[Dmitriy Setrakyan]

On Tue, Mar 28, 2017 at 11:54 AM, Sergi Vladykin 
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.

Re: IGNITE-4188, savepoints with atomic cache?

[Sergi Vladykin]

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.

Sergi

2017-03-28 21:51 GMT+03:00 Dmitriy Setrakyan :

> On Tue, Mar 28, 2017 at 11:42 AM, Valentin Kulichenko <
> valentin.kuliche...@gmail.com> wrote:
>
> > Dmitry,
> >
> > Transaction is not started on a particular cache, it's separate top-level
> > API. You can then include operations on different caches, like this:
> >
> > try (Transaction tx = ignite.transactions.txStart()) {
> > cache1.put(..);
> > cache2.put(..);
> > cache3.put(..);
> >
> > tx.commit();
> > }
> >
> > In the code above it's easy to think that all three operations are
> enlisted
> > in transaction, but if one of the caches happens to be atomic (due to
> > misconfiguration, for example), that's actually not true. And we don't
> even
> > warn user about this.
> >
>
> Got it.
>
> But what if a user really needs to update an atomic cache from within some
> transactional logic? I think we should allow it, but print out an warning
> to the log on per-cache basis the first time we see some atomic cache used
> from within a transaction.
>
> D.
>

Re: IGNITE-4188, savepoints with atomic cache?

[Dmitriy Setrakyan]

On Tue, Mar 28, 2017 at 11:42 AM, Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:

> Dmitry,
>
> Transaction is not started on a particular cache, it's separate top-level
> API. You can then include operations on different caches, like this:
>
> try (Transaction tx = ignite.transactions.txStart()) {
> cache1.put(..);
> cache2.put(..);
> cache3.put(..);
>
> tx.commit();
> }
>
> In the code above it's easy to think that all three operations are enlisted
> in transaction, but if one of the caches happens to be atomic (due to
> misconfiguration, for example), that's actually not true. And we don't even
> warn user about this.
>

Got it.

But what if a user really needs to update an atomic cache from within some
transactional logic? I think we should allow it, but print out an warning
to the log on per-cache basis the first time we see some atomic cache used
from within a transaction.

D.

Re: IGNITE-4188, savepoints with atomic cache?

[Valentin Kulichenko]

Dmitry,

Transaction is not started on a particular cache, it's separate top-level
API. You can then include operations on different caches, like this:

try (Transaction tx = ignite.transactions.txStart()) {
cache1.put(..);
cache2.put(..);
cache3.put(..);

tx.commit();
}

In the code above it's easy to think that all three operations are enlisted
in transaction, but if one of the caches happens to be atomic (due to
misconfiguration, for example), that's actually not true. And we don't even
warn user about this.

-Val

On Tue, Mar 28, 2017 at 11:28 AM, Dmitriy Setrakyan 
wrote:

> I think an exception should be thrown if someone tries to start a
> transaction on an atomic cache. Throwing an exception at the "put(...)"
> time seems late and counter-intuitive.
>
> However, if a user starts a transaction on a transactional cache, say
> CacheA, and inside that transaction updates an entry from a different
> atomic cache, say CacheB, then such update should be allowed in my view,
> because the transaction was started on a completely different cache.
>
> Does this make sense?
>
> D.
>
> On Tue, Mar 28, 2017 at 11:22 AM, Valentin Kulichenko <
> valentin.kuliche...@gmail.com> wrote:
>
> > I actually think exception makes sense here. Currently it's very error
> > prone - the same code can behave differently depending on configuration.
> > And at the same time I can't imagine a use case which implies using
> atomic
> > operation within a transaction on purpose. If someone ever does this, it
> > would most likely be done by mistake.
> >
> > -Val
> >
> > On Tue, Mar 28, 2017 at 10:13 AM, Denis Magda  wrote:
> >
> > > Yakov,
> > >
> > > My vote goes for the current behavior - an atomic operation is applied
> > > right away if it’s a part of a transaction. It’s better not to break
> > > compatibility here, there are already to many incompatible changes in
> > 2.0.
> > >
> > > —
> > > Denis
> > >
> > > > On Mar 28, 2017, at 3:08 AM, Yakov Zhdanov 
> > wrote:
> > > >
> > > > As far as I know operations on atomic caches are applied immediately
> > > > dishonoring any tx context.
> > > >
> > > > I would suggest that atomic cache update operation called from active
> > tx
> > > > throws illegal state exception, unless user intentionally permits
> this
> > > > update by calling atomicCache.withAllowInTx() (similar to
> > > withSkipStore()).
> > > >
> > > > Thoughts?
> > > >
> > > > --Yakov
> > >
> > >
> >
>

Re: IGNITE-4188, savepoints with atomic cache?

[Dmitriy Setrakyan]

I think an exception should be thrown if someone tries to start a
transaction on an atomic cache. Throwing an exception at the "put(...)"
time seems late and counter-intuitive.

However, if a user starts a transaction on a transactional cache, say
CacheA, and inside that transaction updates an entry from a different
atomic cache, say CacheB, then such update should be allowed in my view,
because the transaction was started on a completely different cache.

Does this make sense?

D.

On Tue, Mar 28, 2017 at 11:22 AM, Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:

> I actually think exception makes sense here. Currently it's very error
> prone - the same code can behave differently depending on configuration.
> And at the same time I can't imagine a use case which implies using atomic
> operation within a transaction on purpose. If someone ever does this, it
> would most likely be done by mistake.
>
> -Val
>
> On Tue, Mar 28, 2017 at 10:13 AM, Denis Magda  wrote:
>
> > Yakov,
> >
> > My vote goes for the current behavior - an atomic operation is applied
> > right away if it’s a part of a transaction. It’s better not to break
> > compatibility here, there are already to many incompatible changes in
> 2.0.
> >
> > —
> > Denis
> >
> > > On Mar 28, 2017, at 3:08 AM, Yakov Zhdanov 
> wrote:
> > >
> > > As far as I know operations on atomic caches are applied immediately
> > > dishonoring any tx context.
> > >
> > > I would suggest that atomic cache update operation called from active
> tx
> > > throws illegal state exception, unless user intentionally permits this
> > > update by calling atomicCache.withAllowInTx() (similar to
> > withSkipStore()).
> > >
> > > Thoughts?
> > >
> > > --Yakov
> >
> >
>

Re: IGNITE-4188, savepoints with atomic cache?

[Valentin Kulichenko]

I actually think exception makes sense here. Currently it's very error
prone - the same code can behave differently depending on configuration.
And at the same time I can't imagine a use case which implies using atomic
operation within a transaction on purpose. If someone ever does this, it
would most likely be done by mistake.

-Val

On Tue, Mar 28, 2017 at 10:13 AM, Denis Magda  wrote:

> Yakov,
>
> My vote goes for the current behavior - an atomic operation is applied
> right away if it’s a part of a transaction. It’s better not to break
> compatibility here, there are already to many incompatible changes in 2.0.
>
> —
> Denis
>
> > On Mar 28, 2017, at 3:08 AM, Yakov Zhdanov  wrote:
> >
> > As far as I know operations on atomic caches are applied immediately
> > dishonoring any tx context.
> >
> > I would suggest that atomic cache update operation called from active tx
> > throws illegal state exception, unless user intentionally permits this
> > update by calling atomicCache.withAllowInTx() (similar to
> withSkipStore()).
> >
> > Thoughts?
> >
> > --Yakov
>
>

Re: IGNITE-4188, savepoints with atomic cache?

[Дмитрий Рябов]

I mean ignore it in savepoints and rollbacks to savepoints becouse
transaction rollbacks already ignore it.

2017-03-28 18:10 GMT+03:00 Yakov Zhdanov :

> Ignore atomicCache.put(k, v) if it is called inside transaction? This is
> very counter intuitive. I am strongly against that.
>
> --Yakov
>

Re: IGNITE-4188, savepoints with atomic cache?

[Yakov Zhdanov]

Ignore atomicCache.put(k, v) if it is called inside transaction? This is
very counter intuitive. I am strongly against that.

--Yakov

Re: IGNITE-4188, savepoints with atomic cache?

[Yakov Zhdanov]

As far as I know operations on atomic caches are applied immediately
dishonoring any tx context.

I would suggest that atomic cache update operation called from active tx
throws illegal state exception, unless user intentionally permits this
update by calling atomicCache.withAllowInTx() (similar to withSkipStore()).

Thoughts?

--Yakov