Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

I'll keep the common pool then, thank you.
Public pool is the "work-horse of the Compute Grid" [1], so it does not
seem to fit here at all.

[1]
https://ignite.apache.org/docs/latest/perf-and-troubleshooting/thread-pools-tuning#public-pool

On Thu, Apr 15, 2021 at 2:18 PM Stanislav Lukyanov 
wrote:

> Giving this another thought - I'm kinda torn on this myself now, as I like
> you argument that a chain of multiple (GG and not GG) continuations should
> execute in the same pool.
> This would probably be easier to understand for a beginning or
> intermediate Ignite user, which is the majority.
> Anyway, I'll leave to you to decide.
>
> > On 15 Apr 2021, at 11:02, Stanislav Lukyanov 
> wrote:
> >
> > Hi Pavel,
> >
> > I'd prefer public pool.
> >
> > Thanks,
> > Stan
> >
> >> On 12 Apr 2021, at 20:17, Pavel Tupitsyn  wrote:
> >>
> >> Stan,
> >>
> >> Sorry for the late reply (had a vacation).
> >>
> >>> In my ideal world, the users don't configure thread pools, they just
> have
> >> safe default behavior (async execution)
> >>> and a way to make it fast for one particular function (with annotation
> or
> >> anything else).
> >>
> >> I've
> >> added
> testRemoteOperationListenerExecutesOnStripedPoolWhenCustomExecutorIsProvided
> >> to demonstrate this use case.
> >>
> >>
> >>> I'm OK to proceed with the approach you're suggesting if I haven't
> >> convinced you by now
> >>
> >> Are you OK to merge the changes as is (ForkJoinPool#commonPool as the
> >> default executor),
> >> or do we change it to Ignite public pool?
> >>
> >> On Mon, Mar 29, 2021 at 11:09 PM Stanislav Lukyanov <
> stanlukya...@gmail.com>
> >> wrote:
> >>
> >>> But what if I need to have exactly one callback synchronous, and all
> other
> >>> can be asynchronous?
> >>>
> >>> I would separate two cases: an existing user who wants their old
> behavior
> >>> back, and a new user that wants to fine tune their app.
> >>> The existing user needs a global "make it all synchronous" switch.
> >>> The new user should only enable the fast-but-dangerous behavior
> locally,
> >>> exactly where they need it.
> >>>
> >>> In my ideal world, the users don't configure thread pools, they just
> have
> >>> safe default behavior (async execution)
> >>> and a way to make it fast for one particular function (with annotation
> or
> >>> anything else).
> >>> Also, this should work in a similar way for different APIs - so I'm
> trying
> >>> to lay some basis to rework all of these continuous queries and event
> >>> listeners,
> >>> even though they're explicitly mentioned as out of scope for IEP-70.
> >>>
> >>> At the same time, I understand that any change we make now will have
> pros
> >>> and cons, and we can't make it perfect because of compatibility
> reasons.
> >>> I'm OK to proceed with the approach you're suggesting if I haven't
> >>> convinced you by now :)
> >>>
> >>> Thanks,
> >>> Stan
> >>>
>  On 29 Mar 2021, at 22:47, Pavel Tupitsyn 
> wrote:
> 
>  Stan,
> 
>  Unfortunately, annotations have a few drawbacks:
>  * Can't configure it globally ("I already use sync callbacks, give me
> >>> back
>  the old behavior in one line")
>  * Can't configure in Spring
>  * Useless in C++ & .NET
>  * You can already specify executor in IgniteFuture#listenAsync, so
> there
>  seems to be no added value
> 
> > the only value we really expect the user to set in that property is
>  Runnable::run
>  Not really - there are lots of available options [1].
>  Some apps may already have one or more thread pools that can be used
> for
>  continuations.
> 
> > you can't specify Runnable::run in a Spring XML
>  Good point, but creating a class for that is trivial.
>  We can ship a ready-made class and mention it in the docs for
> simplicity.
> 
> 
>  Globally configurable Executor fits nicely with
>  existing IgniteFuture#listenAsync,
>  not sure why you dislike it.
> 
> 
>  [1]
> 
> >>>
> https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html
> 
>  On Mon, Mar 29, 2021 at 10:23 PM Stanislav Lukyanov <
> >>> stanlukya...@gmail.com>
>  wrote:
> 
> > Thought about this some more.
> >
> > I agree that we need to be able to switch to synchronous listeners
> when
> > it's critical for performance.
> > However, I don't like to introduce an Executor property for that. In
> >>> fact,
> > the only value
> > we really expect the user to set in that property is Runnable::run -
> >>> seems
> > to be an overkill to have accept an Executor for that.
> > Furthermore, you can't specify Runnable::run in a Spring XML, can
> you?
> >
> > I'm thinking that maybe we should go the annotation route here.
> > Let's introduce an annotation @IgniteSyncCallback. It's the same as
> > @IgniteAsyncCallback but reverse :)
> > If a listener is annotated like that then we execute it in the same
> > 

Re: IEP-70: Async Continuation Executor

[Stanislav Lukyanov]

Giving this another thought - I'm kinda torn on this myself now, as I like you 
argument that a chain of multiple (GG and not GG) continuations should execute 
in the same pool.
This would probably be easier to understand for a beginning or intermediate 
Ignite user, which is the majority.
Anyway, I'll leave to you to decide. 

> On 15 Apr 2021, at 11:02, Stanislav Lukyanov  wrote:
> 
> Hi Pavel,
> 
> I'd prefer public pool.
> 
> Thanks,
> Stan
> 
>> On 12 Apr 2021, at 20:17, Pavel Tupitsyn  wrote:
>> 
>> Stan,
>> 
>> Sorry for the late reply (had a vacation).
>> 
>>> In my ideal world, the users don't configure thread pools, they just have
>> safe default behavior (async execution)
>>> and a way to make it fast for one particular function (with annotation or
>> anything else).
>> 
>> I've
>> added 
>> testRemoteOperationListenerExecutesOnStripedPoolWhenCustomExecutorIsProvided
>> to demonstrate this use case.
>> 
>> 
>>> I'm OK to proceed with the approach you're suggesting if I haven't
>> convinced you by now
>> 
>> Are you OK to merge the changes as is (ForkJoinPool#commonPool as the
>> default executor),
>> or do we change it to Ignite public pool?
>> 
>> On Mon, Mar 29, 2021 at 11:09 PM Stanislav Lukyanov 
>> wrote:
>> 
>>> But what if I need to have exactly one callback synchronous, and all other
>>> can be asynchronous?
>>> 
>>> I would separate two cases: an existing user who wants their old behavior
>>> back, and a new user that wants to fine tune their app.
>>> The existing user needs a global "make it all synchronous" switch.
>>> The new user should only enable the fast-but-dangerous behavior locally,
>>> exactly where they need it.
>>> 
>>> In my ideal world, the users don't configure thread pools, they just have
>>> safe default behavior (async execution)
>>> and a way to make it fast for one particular function (with annotation or
>>> anything else).
>>> Also, this should work in a similar way for different APIs - so I'm trying
>>> to lay some basis to rework all of these continuous queries and event
>>> listeners,
>>> even though they're explicitly mentioned as out of scope for IEP-70.
>>> 
>>> At the same time, I understand that any change we make now will have pros
>>> and cons, and we can't make it perfect because of compatibility reasons.
>>> I'm OK to proceed with the approach you're suggesting if I haven't
>>> convinced you by now :)
>>> 
>>> Thanks,
>>> Stan
>>> 
 On 29 Mar 2021, at 22:47, Pavel Tupitsyn  wrote:
 
 Stan,
 
 Unfortunately, annotations have a few drawbacks:
 * Can't configure it globally ("I already use sync callbacks, give me
>>> back
 the old behavior in one line")
 * Can't configure in Spring
 * Useless in C++ & .NET
 * You can already specify executor in IgniteFuture#listenAsync, so there
 seems to be no added value
 
> the only value we really expect the user to set in that property is
 Runnable::run
 Not really - there are lots of available options [1].
 Some apps may already have one or more thread pools that can be used for
 continuations.
 
> you can't specify Runnable::run in a Spring XML
 Good point, but creating a class for that is trivial.
 We can ship a ready-made class and mention it in the docs for simplicity.
 
 
 Globally configurable Executor fits nicely with
 existing IgniteFuture#listenAsync,
 not sure why you dislike it.
 
 
 [1]
 
>>> https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html
 
 On Mon, Mar 29, 2021 at 10:23 PM Stanislav Lukyanov <
>>> stanlukya...@gmail.com>
 wrote:
 
> Thought about this some more.
> 
> I agree that we need to be able to switch to synchronous listeners when
> it's critical for performance.
> However, I don't like to introduce an Executor property for that. In
>>> fact,
> the only value
> we really expect the user to set in that property is Runnable::run -
>>> seems
> to be an overkill to have accept an Executor for that.
> Furthermore, you can't specify Runnable::run in a Spring XML, can you?
> 
> I'm thinking that maybe we should go the annotation route here.
> Let's introduce an annotation @IgniteSyncCallback. It's the same as
> @IgniteAsyncCallback but reverse :)
> If a listener is annotated like that then we execute it in the same
> thread; by default, we execute in the public pool.
> We can also reuse the same annotation for all other callbacks we have in
> the system - right now, the callbacks are a mix of sync and async
>>> behavior,
> and we could transition all APIs to use async by default and enforce
>>> sync
> callbacks when the annotation is used.
> @IgniteAsyncCallback should eventually be deprecated.
> 
> WDYT?
> 
> Thanks,
> Stan
> 
>> On 29 Mar 2021, at 14:09, Pavel Tupitsyn  wrote:
>> 
>> Stan,
>> 
>> I'm ok with using 

Re: IEP-70: Async Continuation Executor

[Stanislav Lukyanov]

Hi Pavel,

I'd prefer public pool.

Thanks,
Stan

> On 12 Apr 2021, at 20:17, Pavel Tupitsyn  wrote:
> 
> Stan,
> 
> Sorry for the late reply (had a vacation).
> 
>> In my ideal world, the users don't configure thread pools, they just have
> safe default behavior (async execution)
>> and a way to make it fast for one particular function (with annotation or
> anything else).
> 
> I've
> added 
> testRemoteOperationListenerExecutesOnStripedPoolWhenCustomExecutorIsProvided
> to demonstrate this use case.
> 
> 
>> I'm OK to proceed with the approach you're suggesting if I haven't
> convinced you by now
> 
> Are you OK to merge the changes as is (ForkJoinPool#commonPool as the
> default executor),
> or do we change it to Ignite public pool?
> 
> On Mon, Mar 29, 2021 at 11:09 PM Stanislav Lukyanov 
> wrote:
> 
>> But what if I need to have exactly one callback synchronous, and all other
>> can be asynchronous?
>> 
>> I would separate two cases: an existing user who wants their old behavior
>> back, and a new user that wants to fine tune their app.
>> The existing user needs a global "make it all synchronous" switch.
>> The new user should only enable the fast-but-dangerous behavior locally,
>> exactly where they need it.
>> 
>> In my ideal world, the users don't configure thread pools, they just have
>> safe default behavior (async execution)
>> and a way to make it fast for one particular function (with annotation or
>> anything else).
>> Also, this should work in a similar way for different APIs - so I'm trying
>> to lay some basis to rework all of these continuous queries and event
>> listeners,
>> even though they're explicitly mentioned as out of scope for IEP-70.
>> 
>> At the same time, I understand that any change we make now will have pros
>> and cons, and we can't make it perfect because of compatibility reasons.
>> I'm OK to proceed with the approach you're suggesting if I haven't
>> convinced you by now :)
>> 
>> Thanks,
>> Stan
>> 
>>> On 29 Mar 2021, at 22:47, Pavel Tupitsyn  wrote:
>>> 
>>> Stan,
>>> 
>>> Unfortunately, annotations have a few drawbacks:
>>> * Can't configure it globally ("I already use sync callbacks, give me
>> back
>>> the old behavior in one line")
>>> * Can't configure in Spring
>>> * Useless in C++ & .NET
>>> * You can already specify executor in IgniteFuture#listenAsync, so there
>>> seems to be no added value
>>> 
 the only value we really expect the user to set in that property is
>>> Runnable::run
>>> Not really - there are lots of available options [1].
>>> Some apps may already have one or more thread pools that can be used for
>>> continuations.
>>> 
 you can't specify Runnable::run in a Spring XML
>>> Good point, but creating a class for that is trivial.
>>> We can ship a ready-made class and mention it in the docs for simplicity.
>>> 
>>> 
>>> Globally configurable Executor fits nicely with
>>> existing IgniteFuture#listenAsync,
>>> not sure why you dislike it.
>>> 
>>> 
>>> [1]
>>> 
>> https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html
>>> 
>>> On Mon, Mar 29, 2021 at 10:23 PM Stanislav Lukyanov <
>> stanlukya...@gmail.com>
>>> wrote:
>>> 
 Thought about this some more.
 
 I agree that we need to be able to switch to synchronous listeners when
 it's critical for performance.
 However, I don't like to introduce an Executor property for that. In
>> fact,
 the only value
 we really expect the user to set in that property is Runnable::run -
>> seems
 to be an overkill to have accept an Executor for that.
 Furthermore, you can't specify Runnable::run in a Spring XML, can you?
 
 I'm thinking that maybe we should go the annotation route here.
 Let's introduce an annotation @IgniteSyncCallback. It's the same as
 @IgniteAsyncCallback but reverse :)
 If a listener is annotated like that then we execute it in the same
 thread; by default, we execute in the public pool.
 We can also reuse the same annotation for all other callbacks we have in
 the system - right now, the callbacks are a mix of sync and async
>> behavior,
 and we could transition all APIs to use async by default and enforce
>> sync
 callbacks when the annotation is used.
 @IgniteAsyncCallback should eventually be deprecated.
 
 WDYT?
 
 Thanks,
 Stan
 
> On 29 Mar 2021, at 14:09, Pavel Tupitsyn  wrote:
> 
> Stan,
> 
> I'm ok with using public pool by default, but we need a way to restore
 the
> old behavior, do you agree?
> I think we should keep the new IgniteConfiguration property.
> 
> On Fri, Mar 26, 2021 at 2:12 PM Alexei Scherbakov <
> alexey.scherbak...@gmail.com> wrote:
> 
>> Pavel,
>> 
>> Dedicated pool looks safer and more manageable to me. Make sure the
 threads
>> in the pool are lazily started and stopped if not used for some time.
>> 
>> Because I have no more real arguments against 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Stan,

Sorry for the late reply (had a vacation).

> In my ideal world, the users don't configure thread pools, they just have
safe default behavior (async execution)
> and a way to make it fast for one particular function (with annotation or
anything else).

I've
added 
testRemoteOperationListenerExecutesOnStripedPoolWhenCustomExecutorIsProvided
to demonstrate this use case.


> I'm OK to proceed with the approach you're suggesting if I haven't
convinced you by now

Are you OK to merge the changes as is (ForkJoinPool#commonPool as the
default executor),
or do we change it to Ignite public pool?

On Mon, Mar 29, 2021 at 11:09 PM Stanislav Lukyanov 
wrote:

> But what if I need to have exactly one callback synchronous, and all other
> can be asynchronous?
>
> I would separate two cases: an existing user who wants their old behavior
> back, and a new user that wants to fine tune their app.
> The existing user needs a global "make it all synchronous" switch.
> The new user should only enable the fast-but-dangerous behavior locally,
> exactly where they need it.
>
> In my ideal world, the users don't configure thread pools, they just have
> safe default behavior (async execution)
> and a way to make it fast for one particular function (with annotation or
> anything else).
> Also, this should work in a similar way for different APIs - so I'm trying
> to lay some basis to rework all of these continuous queries and event
> listeners,
> even though they're explicitly mentioned as out of scope for IEP-70.
>
> At the same time, I understand that any change we make now will have pros
> and cons, and we can't make it perfect because of compatibility reasons.
> I'm OK to proceed with the approach you're suggesting if I haven't
> convinced you by now :)
>
> Thanks,
> Stan
>
> > On 29 Mar 2021, at 22:47, Pavel Tupitsyn  wrote:
> >
> > Stan,
> >
> > Unfortunately, annotations have a few drawbacks:
> > * Can't configure it globally ("I already use sync callbacks, give me
> back
> > the old behavior in one line")
> > * Can't configure in Spring
> > * Useless in C++ & .NET
> > * You can already specify executor in IgniteFuture#listenAsync, so there
> > seems to be no added value
> >
> >> the only value we really expect the user to set in that property is
> > Runnable::run
> > Not really - there are lots of available options [1].
> > Some apps may already have one or more thread pools that can be used for
> > continuations.
> >
> >> you can't specify Runnable::run in a Spring XML
> > Good point, but creating a class for that is trivial.
> > We can ship a ready-made class and mention it in the docs for simplicity.
> >
> >
> > Globally configurable Executor fits nicely with
> > existing IgniteFuture#listenAsync,
> > not sure why you dislike it.
> >
> >
> > [1]
> >
> https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html
> >
> > On Mon, Mar 29, 2021 at 10:23 PM Stanislav Lukyanov <
> stanlukya...@gmail.com>
> > wrote:
> >
> >> Thought about this some more.
> >>
> >> I agree that we need to be able to switch to synchronous listeners when
> >> it's critical for performance.
> >> However, I don't like to introduce an Executor property for that. In
> fact,
> >> the only value
> >> we really expect the user to set in that property is Runnable::run -
> seems
> >> to be an overkill to have accept an Executor for that.
> >> Furthermore, you can't specify Runnable::run in a Spring XML, can you?
> >>
> >> I'm thinking that maybe we should go the annotation route here.
> >> Let's introduce an annotation @IgniteSyncCallback. It's the same as
> >> @IgniteAsyncCallback but reverse :)
> >> If a listener is annotated like that then we execute it in the same
> >> thread; by default, we execute in the public pool.
> >> We can also reuse the same annotation for all other callbacks we have in
> >> the system - right now, the callbacks are a mix of sync and async
> behavior,
> >> and we could transition all APIs to use async by default and enforce
> sync
> >> callbacks when the annotation is used.
> >> @IgniteAsyncCallback should eventually be deprecated.
> >>
> >> WDYT?
> >>
> >> Thanks,
> >> Stan
> >>
> >>> On 29 Mar 2021, at 14:09, Pavel Tupitsyn  wrote:
> >>>
> >>> Stan,
> >>>
> >>> I'm ok with using public pool by default, but we need a way to restore
> >> the
> >>> old behavior, do you agree?
> >>> I think we should keep the new IgniteConfiguration property.
> >>>
> >>> On Fri, Mar 26, 2021 at 2:12 PM Alexei Scherbakov <
> >>> alexey.scherbak...@gmail.com> wrote:
> >>>
>  Pavel,
> 
>  Dedicated pool looks safer and more manageable to me. Make sure the
> >> threads
>  in the pool are lazily started and stopped if not used for some time.
> 
>  Because I have no more real arguments against the change, I suggest to
>  proceed with this approach.
> 
>  чт, 25 мар. 2021 г. в 22:16, Pavel Tupitsyn :
> 
> > Alexei,
> >
> >> we already have ways to control a listener's behavior

Re: IEP-70: Async Continuation Executor

[Stanislav Lukyanov]

But what if I need to have exactly one callback synchronous, and all other can 
be asynchronous?

I would separate two cases: an existing user who wants their old behavior back, 
and a new user that wants to fine tune their app.
The existing user needs a global "make it all synchronous" switch.
The new user should only enable the fast-but-dangerous behavior locally, 
exactly where they need it.

In my ideal world, the users don't configure thread pools, they just have safe 
default behavior (async execution)
and a way to make it fast for one particular function (with annotation or 
anything else).
Also, this should work in a similar way for different APIs - so I'm trying to 
lay some basis to rework all of these continuous queries and event listeners,
even though they're explicitly mentioned as out of scope for IEP-70.

At the same time, I understand that any change we make now will have pros and 
cons, and we can't make it perfect because of compatibility reasons.
I'm OK to proceed with the approach you're suggesting if I haven't convinced 
you by now :)

Thanks,
Stan

> On 29 Mar 2021, at 22:47, Pavel Tupitsyn  wrote:
> 
> Stan,
> 
> Unfortunately, annotations have a few drawbacks:
> * Can't configure it globally ("I already use sync callbacks, give me back
> the old behavior in one line")
> * Can't configure in Spring
> * Useless in C++ & .NET
> * You can already specify executor in IgniteFuture#listenAsync, so there
> seems to be no added value
> 
>> the only value we really expect the user to set in that property is
> Runnable::run
> Not really - there are lots of available options [1].
> Some apps may already have one or more thread pools that can be used for
> continuations.
> 
>> you can't specify Runnable::run in a Spring XML
> Good point, but creating a class for that is trivial.
> We can ship a ready-made class and mention it in the docs for simplicity.
> 
> 
> Globally configurable Executor fits nicely with
> existing IgniteFuture#listenAsync,
> not sure why you dislike it.
> 
> 
> [1]
> https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html
> 
> On Mon, Mar 29, 2021 at 10:23 PM Stanislav Lukyanov 
> wrote:
> 
>> Thought about this some more.
>> 
>> I agree that we need to be able to switch to synchronous listeners when
>> it's critical for performance.
>> However, I don't like to introduce an Executor property for that. In fact,
>> the only value
>> we really expect the user to set in that property is Runnable::run - seems
>> to be an overkill to have accept an Executor for that.
>> Furthermore, you can't specify Runnable::run in a Spring XML, can you?
>> 
>> I'm thinking that maybe we should go the annotation route here.
>> Let's introduce an annotation @IgniteSyncCallback. It's the same as
>> @IgniteAsyncCallback but reverse :)
>> If a listener is annotated like that then we execute it in the same
>> thread; by default, we execute in the public pool.
>> We can also reuse the same annotation for all other callbacks we have in
>> the system - right now, the callbacks are a mix of sync and async behavior,
>> and we could transition all APIs to use async by default and enforce sync
>> callbacks when the annotation is used.
>> @IgniteAsyncCallback should eventually be deprecated.
>> 
>> WDYT?
>> 
>> Thanks,
>> Stan
>> 
>>> On 29 Mar 2021, at 14:09, Pavel Tupitsyn  wrote:
>>> 
>>> Stan,
>>> 
>>> I'm ok with using public pool by default, but we need a way to restore
>> the
>>> old behavior, do you agree?
>>> I think we should keep the new IgniteConfiguration property.
>>> 
>>> On Fri, Mar 26, 2021 at 2:12 PM Alexei Scherbakov <
>>> alexey.scherbak...@gmail.com> wrote:
>>> 
 Pavel,
 
 Dedicated pool looks safer and more manageable to me. Make sure the
>> threads
 in the pool are lazily started and stopped if not used for some time.
 
 Because I have no more real arguments against the change, I suggest to
 proceed with this approach.
 
 чт, 25 мар. 2021 г. в 22:16, Pavel Tupitsyn :
 
> Alexei,
> 
>> we already have ways to control a listener's behavior
> No, we don't have a way to fix current broken and dangerous behavior
> globally.
> You should not expect the user to fix every async call manually.
> 
>> commonPool can alter existing deployments in unpredictable ways,
>> if commonPool is heavily used for other purposes
> Common pool resizes dynamically to accommodate the load [1]
> What do you think about Stan's suggestion to use our public pool
>> instead?
> 
> [1]
> 
> 
 
>> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ForkJoinPool.html
> 
> On Thu, Mar 25, 2021 at 10:10 PM Pavel Tupitsyn 
> wrote:
> 
>>> I don't agree that the code isn't related to Ignite - it is something
>> that the user does via Ignite API
>> 
>> This is a misconception. When you write general-purpose async code, it
>> looks like this:
>> 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Stan,

Unfortunately, annotations have a few drawbacks:
* Can't configure it globally ("I already use sync callbacks, give me back
the old behavior in one line")
* Can't configure in Spring
* Useless in C++ & .NET
* You can already specify executor in IgniteFuture#listenAsync, so there
seems to be no added value

> the only value we really expect the user to set in that property is
Runnable::run
Not really - there are lots of available options [1].
Some apps may already have one or more thread pools that can be used for
continuations.

> you can't specify Runnable::run in a Spring XML
Good point, but creating a class for that is trivial.
We can ship a ready-made class and mention it in the docs for simplicity.


Globally configurable Executor fits nicely with
existing IgniteFuture#listenAsync,
not sure why you dislike it.


[1]
https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html

On Mon, Mar 29, 2021 at 10:23 PM Stanislav Lukyanov 
wrote:

> Thought about this some more.
>
> I agree that we need to be able to switch to synchronous listeners when
> it's critical for performance.
> However, I don't like to introduce an Executor property for that. In fact,
> the only value
> we really expect the user to set in that property is Runnable::run - seems
> to be an overkill to have accept an Executor for that.
> Furthermore, you can't specify Runnable::run in a Spring XML, can you?
>
> I'm thinking that maybe we should go the annotation route here.
> Let's introduce an annotation @IgniteSyncCallback. It's the same as
> @IgniteAsyncCallback but reverse :)
> If a listener is annotated like that then we execute it in the same
> thread; by default, we execute in the public pool.
> We can also reuse the same annotation for all other callbacks we have in
> the system - right now, the callbacks are a mix of sync and async behavior,
> and we could transition all APIs to use async by default and enforce sync
> callbacks when the annotation is used.
> @IgniteAsyncCallback should eventually be deprecated.
>
> WDYT?
>
> Thanks,
> Stan
>
> > On 29 Mar 2021, at 14:09, Pavel Tupitsyn  wrote:
> >
> > Stan,
> >
> > I'm ok with using public pool by default, but we need a way to restore
> the
> > old behavior, do you agree?
> > I think we should keep the new IgniteConfiguration property.
> >
> > On Fri, Mar 26, 2021 at 2:12 PM Alexei Scherbakov <
> > alexey.scherbak...@gmail.com> wrote:
> >
> >> Pavel,
> >>
> >> Dedicated pool looks safer and more manageable to me. Make sure the
> threads
> >> in the pool are lazily started and stopped if not used for some time.
> >>
> >> Because I have no more real arguments against the change, I suggest to
> >> proceed with this approach.
> >>
> >> чт, 25 мар. 2021 г. в 22:16, Pavel Tupitsyn :
> >>
> >>> Alexei,
> >>>
>  we already have ways to control a listener's behavior
> >>> No, we don't have a way to fix current broken and dangerous behavior
> >>> globally.
> >>> You should not expect the user to fix every async call manually.
> >>>
>  commonPool can alter existing deployments in unpredictable ways,
>  if commonPool is heavily used for other purposes
> >>> Common pool resizes dynamically to accommodate the load [1]
> >>> What do you think about Stan's suggestion to use our public pool
> instead?
> >>>
> >>> [1]
> >>>
> >>>
> >>
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ForkJoinPool.html
> >>>
> >>> On Thu, Mar 25, 2021 at 10:10 PM Pavel Tupitsyn 
> >>> wrote:
> >>>
> > I don't agree that the code isn't related to Ignite - it is something
>  that the user does via Ignite API
> 
>  This is a misconception. When you write general-purpose async code, it
>  looks like this:
> 
>  myClass.fooAsync()
>  .chain(igniteCache.putAsync)
>  .chain(myClass.barAsync)
>  .chain(...)
> 
>  And so on, you jump from one continuation to another.
>  You don't think about this as "I use Ignite API to run my
> >> continuation",
>  this is just another async call among hundreds of others.
> 
>  And you don't want 1 of 20 libraries that you use to have "special
> >> needs"
>  like Ignite does right now.
> 
>  I know Java is late to the async party and not everyone is used to
> this
>  mindset,
>  but the situation changes, more and more code bases go async all the
> >> way,
>  use CompletionStage everywhere, etc.
> 
> 
> > If we go with the public pool - no additional options needed.
> 
>  I guess public pool should work.
>  However, I would prefer to keep using commonPool, which is recommended
> >>> for
>  a general purpose like this.
> 
>  On Thu, Mar 25, 2021 at 3:56 PM Alexei Scherbakov <
>  alexey.scherbak...@gmail.com> wrote:
> 
> > Pavel,
> >
> > The change still looks a bit risky to me, because the default
> executor
> >>> is
> > set to commonPool and can alter existing deployments in unpredictable
> 

Re: IEP-70: Async Continuation Executor

[Stanislav Lukyanov]

Thought about this some more.

I agree that we need to be able to switch to synchronous listeners when it's 
critical for performance.
However, I don't like to introduce an Executor property for that. In fact, the 
only value 
we really expect the user to set in that property is Runnable::run - seems to 
be an overkill to have accept an Executor for that.
Furthermore, you can't specify Runnable::run in a Spring XML, can you?

I'm thinking that maybe we should go the annotation route here.
Let's introduce an annotation @IgniteSyncCallback. It's the same as 
@IgniteAsyncCallback but reverse :)
If a listener is annotated like that then we execute it in the same thread; by 
default, we execute in the public pool.
We can also reuse the same annotation for all other callbacks we have in the 
system - right now, the callbacks are a mix of sync and async behavior,
and we could transition all APIs to use async by default and enforce sync 
callbacks when the annotation is used.
@IgniteAsyncCallback should eventually be deprecated.

WDYT?

Thanks,
Stan

> On 29 Mar 2021, at 14:09, Pavel Tupitsyn  wrote:
> 
> Stan,
> 
> I'm ok with using public pool by default, but we need a way to restore the
> old behavior, do you agree?
> I think we should keep the new IgniteConfiguration property.
> 
> On Fri, Mar 26, 2021 at 2:12 PM Alexei Scherbakov <
> alexey.scherbak...@gmail.com> wrote:
> 
>> Pavel,
>> 
>> Dedicated pool looks safer and more manageable to me. Make sure the threads
>> in the pool are lazily started and stopped if not used for some time.
>> 
>> Because I have no more real arguments against the change, I suggest to
>> proceed with this approach.
>> 
>> чт, 25 мар. 2021 г. в 22:16, Pavel Tupitsyn :
>> 
>>> Alexei,
>>> 
 we already have ways to control a listener's behavior
>>> No, we don't have a way to fix current broken and dangerous behavior
>>> globally.
>>> You should not expect the user to fix every async call manually.
>>> 
 commonPool can alter existing deployments in unpredictable ways,
 if commonPool is heavily used for other purposes
>>> Common pool resizes dynamically to accommodate the load [1]
>>> What do you think about Stan's suggestion to use our public pool instead?
>>> 
>>> [1]
>>> 
>>> 
>> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ForkJoinPool.html
>>> 
>>> On Thu, Mar 25, 2021 at 10:10 PM Pavel Tupitsyn 
>>> wrote:
>>> 
> I don't agree that the code isn't related to Ignite - it is something
 that the user does via Ignite API
 
 This is a misconception. When you write general-purpose async code, it
 looks like this:
 
 myClass.fooAsync()
 .chain(igniteCache.putAsync)
 .chain(myClass.barAsync)
 .chain(...)
 
 And so on, you jump from one continuation to another.
 You don't think about this as "I use Ignite API to run my
>> continuation",
 this is just another async call among hundreds of others.
 
 And you don't want 1 of 20 libraries that you use to have "special
>> needs"
 like Ignite does right now.
 
 I know Java is late to the async party and not everyone is used to this
 mindset,
 but the situation changes, more and more code bases go async all the
>> way,
 use CompletionStage everywhere, etc.
 
 
> If we go with the public pool - no additional options needed.
 
 I guess public pool should work.
 However, I would prefer to keep using commonPool, which is recommended
>>> for
 a general purpose like this.
 
 On Thu, Mar 25, 2021 at 3:56 PM Alexei Scherbakov <
 alexey.scherbak...@gmail.com> wrote:
 
> Pavel,
> 
> The change still looks a bit risky to me, because the default executor
>>> is
> set to commonPool and can alter existing deployments in unpredictable
> ways,
> if commonPool is heavily used for other purposes.
> 
> Runnable::run usage is not obvious as well and should be properly
> documented as a way to return to old behavior.
> 
> I'm not sure we need it in 2.X for the reasons above - we already have
> ways
> to control a listener's behavior - it's a matter of good documentation
>>> to
> me.
> 
> 
> 
> 
> 
> 
> чт, 25 мар. 2021 г. в 15:33, Pavel Tupitsyn :
> 
>> Alexei,
>> 
>>> Sometimes it's more desirable to execute the listener in the same
> thread
>>> It's up to the user to decide.
>> 
>> Yes, we give users a choice to configure the executor as
>> Runnable::run
> and
>> use the same thread if needed.
>> However, it should not be the default behavior as explained above
>> (bad
>> usability, unexpected major issues).
>> 
>> On Thu, Mar 25, 2021 at 3:06 PM Alexei Scherbakov <
>> alexey.scherbak...@gmail.com> wrote:
>> 
>>> Pavel,
>>> 
>>> While I understand the issue and overall agree with you, I'm
>> against
> the
>>> execution of listeners in 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Stan,

I'm ok with using public pool by default, but we need a way to restore the
old behavior, do you agree?
I think we should keep the new IgniteConfiguration property.

On Fri, Mar 26, 2021 at 2:12 PM Alexei Scherbakov <
alexey.scherbak...@gmail.com> wrote:

> Pavel,
>
> Dedicated pool looks safer and more manageable to me. Make sure the threads
> in the pool are lazily started and stopped if not used for some time.
>
> Because I have no more real arguments against the change, I suggest to
> proceed with this approach.
>
> чт, 25 мар. 2021 г. в 22:16, Pavel Tupitsyn :
>
> > Alexei,
> >
> > > we already have ways to control a listener's behavior
> > No, we don't have a way to fix current broken and dangerous behavior
> > globally.
> > You should not expect the user to fix every async call manually.
> >
> > > commonPool can alter existing deployments in unpredictable ways,
> > > if commonPool is heavily used for other purposes
> > Common pool resizes dynamically to accommodate the load [1]
> > What do you think about Stan's suggestion to use our public pool instead?
> >
> > [1]
> >
> >
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ForkJoinPool.html
> >
> > On Thu, Mar 25, 2021 at 10:10 PM Pavel Tupitsyn 
> > wrote:
> >
> > > > I don't agree that the code isn't related to Ignite - it is something
> > > that the user does via Ignite API
> > >
> > > This is a misconception. When you write general-purpose async code, it
> > > looks like this:
> > >
> > > myClass.fooAsync()
> > > .chain(igniteCache.putAsync)
> > > .chain(myClass.barAsync)
> > > .chain(...)
> > >
> > > And so on, you jump from one continuation to another.
> > > You don't think about this as "I use Ignite API to run my
> continuation",
> > > this is just another async call among hundreds of others.
> > >
> > > And you don't want 1 of 20 libraries that you use to have "special
> needs"
> > > like Ignite does right now.
> > >
> > > I know Java is late to the async party and not everyone is used to this
> > > mindset,
> > > but the situation changes, more and more code bases go async all the
> way,
> > > use CompletionStage everywhere, etc.
> > >
> > >
> > > > If we go with the public pool - no additional options needed.
> > >
> > > I guess public pool should work.
> > > However, I would prefer to keep using commonPool, which is recommended
> > for
> > > a general purpose like this.
> > >
> > > On Thu, Mar 25, 2021 at 3:56 PM Alexei Scherbakov <
> > > alexey.scherbak...@gmail.com> wrote:
> > >
> > >> Pavel,
> > >>
> > >> The change still looks a bit risky to me, because the default executor
> > is
> > >> set to commonPool and can alter existing deployments in unpredictable
> > >> ways,
> > >> if commonPool is heavily used for other purposes.
> > >>
> > >> Runnable::run usage is not obvious as well and should be properly
> > >> documented as a way to return to old behavior.
> > >>
> > >> I'm not sure we need it in 2.X for the reasons above - we already have
> > >> ways
> > >> to control a listener's behavior - it's a matter of good documentation
> > to
> > >> me.
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> чт, 25 мар. 2021 г. в 15:33, Pavel Tupitsyn :
> > >>
> > >> > Alexei,
> > >> >
> > >> > > Sometimes it's more desirable to execute the listener in the same
> > >> thread
> > >> > > It's up to the user to decide.
> > >> >
> > >> > Yes, we give users a choice to configure the executor as
> Runnable::run
> > >> and
> > >> > use the same thread if needed.
> > >> > However, it should not be the default behavior as explained above
> (bad
> > >> > usability, unexpected major issues).
> > >> >
> > >> > On Thu, Mar 25, 2021 at 3:06 PM Alexei Scherbakov <
> > >> > alexey.scherbak...@gmail.com> wrote:
> > >> >
> > >> > > Pavel,
> > >> > >
> > >> > > While I understand the issue and overall agree with you, I'm
> against
> > >> the
> > >> > > execution of listeners in separate thread pool by default.
> > >> > >
> > >> > > Sometimes it's more desirable to execute the listener in the same
> > >> thread,
> > >> > > for example if it's some lightweight closure.
> > >> > >
> > >> > > It's up to the user to decide.
> > >> > >
> > >> > > I think the IgniteFuture.listen method should be properly
> documented
> > >> to
> > >> > > avoid execution of cluster operations or any other potentially
> > >> blocking
> > >> > > operations inside the listener.
> > >> > >
> > >> > > Otherwise listenAsync should be used.
> > >> > >
> > >> > >
> > >> > >
> > >> > > чт, 25 мар. 2021 г. в 14:04, Pavel Tupitsyn  >:
> > >> > >
> > >> > > > Stan,
> > >> > > >
> > >> > > > We have thread pools dedicated for specific purposes, like cache
> > >> > > (striped),
> > >> > > > compute (pub), query, etc
> > >> > > > As I understand it, the reason here is to limit the number of
> > >> threads
> > >> > > > dedicated to a given subsystem.
> > >> > > > For example, Compute may be overloaded with work, but Cache and
> > >> > Discovery
> > >> > > > will keep going.
> > >> > > 

Re: IEP-70: Async Continuation Executor

[Alexei Scherbakov]

Pavel,

Dedicated pool looks safer and more manageable to me. Make sure the threads
in the pool are lazily started and stopped if not used for some time.

Because I have no more real arguments against the change, I suggest to
proceed with this approach.

чт, 25 мар. 2021 г. в 22:16, Pavel Tupitsyn :

> Alexei,
>
> > we already have ways to control a listener's behavior
> No, we don't have a way to fix current broken and dangerous behavior
> globally.
> You should not expect the user to fix every async call manually.
>
> > commonPool can alter existing deployments in unpredictable ways,
> > if commonPool is heavily used for other purposes
> Common pool resizes dynamically to accommodate the load [1]
> What do you think about Stan's suggestion to use our public pool instead?
>
> [1]
>
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ForkJoinPool.html
>
> On Thu, Mar 25, 2021 at 10:10 PM Pavel Tupitsyn 
> wrote:
>
> > > I don't agree that the code isn't related to Ignite - it is something
> > that the user does via Ignite API
> >
> > This is a misconception. When you write general-purpose async code, it
> > looks like this:
> >
> > myClass.fooAsync()
> > .chain(igniteCache.putAsync)
> > .chain(myClass.barAsync)
> > .chain(...)
> >
> > And so on, you jump from one continuation to another.
> > You don't think about this as "I use Ignite API to run my continuation",
> > this is just another async call among hundreds of others.
> >
> > And you don't want 1 of 20 libraries that you use to have "special needs"
> > like Ignite does right now.
> >
> > I know Java is late to the async party and not everyone is used to this
> > mindset,
> > but the situation changes, more and more code bases go async all the way,
> > use CompletionStage everywhere, etc.
> >
> >
> > > If we go with the public pool - no additional options needed.
> >
> > I guess public pool should work.
> > However, I would prefer to keep using commonPool, which is recommended
> for
> > a general purpose like this.
> >
> > On Thu, Mar 25, 2021 at 3:56 PM Alexei Scherbakov <
> > alexey.scherbak...@gmail.com> wrote:
> >
> >> Pavel,
> >>
> >> The change still looks a bit risky to me, because the default executor
> is
> >> set to commonPool and can alter existing deployments in unpredictable
> >> ways,
> >> if commonPool is heavily used for other purposes.
> >>
> >> Runnable::run usage is not obvious as well and should be properly
> >> documented as a way to return to old behavior.
> >>
> >> I'm not sure we need it in 2.X for the reasons above - we already have
> >> ways
> >> to control a listener's behavior - it's a matter of good documentation
> to
> >> me.
> >>
> >>
> >>
> >>
> >>
> >>
> >> чт, 25 мар. 2021 г. в 15:33, Pavel Tupitsyn :
> >>
> >> > Alexei,
> >> >
> >> > > Sometimes it's more desirable to execute the listener in the same
> >> thread
> >> > > It's up to the user to decide.
> >> >
> >> > Yes, we give users a choice to configure the executor as Runnable::run
> >> and
> >> > use the same thread if needed.
> >> > However, it should not be the default behavior as explained above (bad
> >> > usability, unexpected major issues).
> >> >
> >> > On Thu, Mar 25, 2021 at 3:06 PM Alexei Scherbakov <
> >> > alexey.scherbak...@gmail.com> wrote:
> >> >
> >> > > Pavel,
> >> > >
> >> > > While I understand the issue and overall agree with you, I'm against
> >> the
> >> > > execution of listeners in separate thread pool by default.
> >> > >
> >> > > Sometimes it's more desirable to execute the listener in the same
> >> thread,
> >> > > for example if it's some lightweight closure.
> >> > >
> >> > > It's up to the user to decide.
> >> > >
> >> > > I think the IgniteFuture.listen method should be properly documented
> >> to
> >> > > avoid execution of cluster operations or any other potentially
> >> blocking
> >> > > operations inside the listener.
> >> > >
> >> > > Otherwise listenAsync should be used.
> >> > >
> >> > >
> >> > >
> >> > > чт, 25 мар. 2021 г. в 14:04, Pavel Tupitsyn :
> >> > >
> >> > > > Stan,
> >> > > >
> >> > > > We have thread pools dedicated for specific purposes, like cache
> >> > > (striped),
> >> > > > compute (pub), query, etc
> >> > > > As I understand it, the reason here is to limit the number of
> >> threads
> >> > > > dedicated to a given subsystem.
> >> > > > For example, Compute may be overloaded with work, but Cache and
> >> > Discovery
> >> > > > will keep going.
> >> > > >
> >> > > > This is different from async continuations, which are arbitrary
> user
> >> > > code.
> >> > > > So what is the benefit of having a new user pool for arbitrary
> code
> >> > that
> >> > > is
> >> > > > probably not related to Ignite at all?
> >> > > >
> >> > > > On Thu, Mar 25, 2021 at 1:31 PM  wrote:
> >> > > >
> >> > > > > Pavel,
> >> > > > >
> >> > > > > This is a great work, fully agree with the overall idea and
> >> approach.
> >> > > > >
> >> > > > > However, I have some reservations about the API. We sure do
> have a
> >> > 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Alexei,

> we already have ways to control a listener's behavior
No, we don't have a way to fix current broken and dangerous behavior
globally.
You should not expect the user to fix every async call manually.

> commonPool can alter existing deployments in unpredictable ways,
> if commonPool is heavily used for other purposes
Common pool resizes dynamically to accommodate the load [1]
What do you think about Stan's suggestion to use our public pool instead?

[1]
https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ForkJoinPool.html

On Thu, Mar 25, 2021 at 10:10 PM Pavel Tupitsyn 
wrote:

> > I don't agree that the code isn't related to Ignite - it is something
> that the user does via Ignite API
>
> This is a misconception. When you write general-purpose async code, it
> looks like this:
>
> myClass.fooAsync()
> .chain(igniteCache.putAsync)
> .chain(myClass.barAsync)
> .chain(...)
>
> And so on, you jump from one continuation to another.
> You don't think about this as "I use Ignite API to run my continuation",
> this is just another async call among hundreds of others.
>
> And you don't want 1 of 20 libraries that you use to have "special needs"
> like Ignite does right now.
>
> I know Java is late to the async party and not everyone is used to this
> mindset,
> but the situation changes, more and more code bases go async all the way,
> use CompletionStage everywhere, etc.
>
>
> > If we go with the public pool - no additional options needed.
>
> I guess public pool should work.
> However, I would prefer to keep using commonPool, which is recommended for
> a general purpose like this.
>
> On Thu, Mar 25, 2021 at 3:56 PM Alexei Scherbakov <
> alexey.scherbak...@gmail.com> wrote:
>
>> Pavel,
>>
>> The change still looks a bit risky to me, because the default executor is
>> set to commonPool and can alter existing deployments in unpredictable
>> ways,
>> if commonPool is heavily used for other purposes.
>>
>> Runnable::run usage is not obvious as well and should be properly
>> documented as a way to return to old behavior.
>>
>> I'm not sure we need it in 2.X for the reasons above - we already have
>> ways
>> to control a listener's behavior - it's a matter of good documentation to
>> me.
>>
>>
>>
>>
>>
>>
>> чт, 25 мар. 2021 г. в 15:33, Pavel Tupitsyn :
>>
>> > Alexei,
>> >
>> > > Sometimes it's more desirable to execute the listener in the same
>> thread
>> > > It's up to the user to decide.
>> >
>> > Yes, we give users a choice to configure the executor as Runnable::run
>> and
>> > use the same thread if needed.
>> > However, it should not be the default behavior as explained above (bad
>> > usability, unexpected major issues).
>> >
>> > On Thu, Mar 25, 2021 at 3:06 PM Alexei Scherbakov <
>> > alexey.scherbak...@gmail.com> wrote:
>> >
>> > > Pavel,
>> > >
>> > > While I understand the issue and overall agree with you, I'm against
>> the
>> > > execution of listeners in separate thread pool by default.
>> > >
>> > > Sometimes it's more desirable to execute the listener in the same
>> thread,
>> > > for example if it's some lightweight closure.
>> > >
>> > > It's up to the user to decide.
>> > >
>> > > I think the IgniteFuture.listen method should be properly documented
>> to
>> > > avoid execution of cluster operations or any other potentially
>> blocking
>> > > operations inside the listener.
>> > >
>> > > Otherwise listenAsync should be used.
>> > >
>> > >
>> > >
>> > > чт, 25 мар. 2021 г. в 14:04, Pavel Tupitsyn :
>> > >
>> > > > Stan,
>> > > >
>> > > > We have thread pools dedicated for specific purposes, like cache
>> > > (striped),
>> > > > compute (pub), query, etc
>> > > > As I understand it, the reason here is to limit the number of
>> threads
>> > > > dedicated to a given subsystem.
>> > > > For example, Compute may be overloaded with work, but Cache and
>> > Discovery
>> > > > will keep going.
>> > > >
>> > > > This is different from async continuations, which are arbitrary user
>> > > code.
>> > > > So what is the benefit of having a new user pool for arbitrary code
>> > that
>> > > is
>> > > > probably not related to Ignite at all?
>> > > >
>> > > > On Thu, Mar 25, 2021 at 1:31 PM  wrote:
>> > > >
>> > > > > Pavel,
>> > > > >
>> > > > > This is a great work, fully agree with the overall idea and
>> approach.
>> > > > >
>> > > > > However, I have some reservations about the API. We sure do have a
>> > lot
>> > > of
>> > > > > async stuff in the system, and I would suggest to stick to the
>> usual
>> > > > design
>> > > > > - create a separate thread pool, add a single property to control
>> the
>> > > > size
>> > > > > of the pool.
>> > > > > Alternatively, we may consider using public pool for that. May I
>> ask
>> > if
>> > > > > there is an example use case which doesn’t work with public pool?
>> > > > >
>> > > > > For .NET, agree that we should follow the rules and APIs of the
>> > > platform,
>> > > > > so the behavior might slightly differ.
>> > > > >
>> > > > > Thanks,
>> > > 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

> I don't agree that the code isn't related to Ignite - it is something
that the user does via Ignite API

This is a misconception. When you write general-purpose async code, it
looks like this:

myClass.fooAsync()
.chain(igniteCache.putAsync)
.chain(myClass.barAsync)
.chain(...)

And so on, you jump from one continuation to another.
You don't think about this as "I use Ignite API to run my continuation",
this is just another async call among hundreds of others.

And you don't want 1 of 20 libraries that you use to have "special needs"
like Ignite does right now.

I know Java is late to the async party and not everyone is used to this
mindset,
but the situation changes, more and more code bases go async all the way,
use CompletionStage everywhere, etc.


> If we go with the public pool - no additional options needed.

I guess public pool should work.
However, I would prefer to keep using commonPool, which is recommended for
a general purpose like this.

On Thu, Mar 25, 2021 at 3:56 PM Alexei Scherbakov <
alexey.scherbak...@gmail.com> wrote:

> Pavel,
>
> The change still looks a bit risky to me, because the default executor is
> set to commonPool and can alter existing deployments in unpredictable ways,
> if commonPool is heavily used for other purposes.
>
> Runnable::run usage is not obvious as well and should be properly
> documented as a way to return to old behavior.
>
> I'm not sure we need it in 2.X for the reasons above - we already have ways
> to control a listener's behavior - it's a matter of good documentation to
> me.
>
>
>
>
>
>
> чт, 25 мар. 2021 г. в 15:33, Pavel Tupitsyn :
>
> > Alexei,
> >
> > > Sometimes it's more desirable to execute the listener in the same
> thread
> > > It's up to the user to decide.
> >
> > Yes, we give users a choice to configure the executor as Runnable::run
> and
> > use the same thread if needed.
> > However, it should not be the default behavior as explained above (bad
> > usability, unexpected major issues).
> >
> > On Thu, Mar 25, 2021 at 3:06 PM Alexei Scherbakov <
> > alexey.scherbak...@gmail.com> wrote:
> >
> > > Pavel,
> > >
> > > While I understand the issue and overall agree with you, I'm against
> the
> > > execution of listeners in separate thread pool by default.
> > >
> > > Sometimes it's more desirable to execute the listener in the same
> thread,
> > > for example if it's some lightweight closure.
> > >
> > > It's up to the user to decide.
> > >
> > > I think the IgniteFuture.listen method should be properly documented to
> > > avoid execution of cluster operations or any other potentially blocking
> > > operations inside the listener.
> > >
> > > Otherwise listenAsync should be used.
> > >
> > >
> > >
> > > чт, 25 мар. 2021 г. в 14:04, Pavel Tupitsyn :
> > >
> > > > Stan,
> > > >
> > > > We have thread pools dedicated for specific purposes, like cache
> > > (striped),
> > > > compute (pub), query, etc
> > > > As I understand it, the reason here is to limit the number of threads
> > > > dedicated to a given subsystem.
> > > > For example, Compute may be overloaded with work, but Cache and
> > Discovery
> > > > will keep going.
> > > >
> > > > This is different from async continuations, which are arbitrary user
> > > code.
> > > > So what is the benefit of having a new user pool for arbitrary code
> > that
> > > is
> > > > probably not related to Ignite at all?
> > > >
> > > > On Thu, Mar 25, 2021 at 1:31 PM  wrote:
> > > >
> > > > > Pavel,
> > > > >
> > > > > This is a great work, fully agree with the overall idea and
> approach.
> > > > >
> > > > > However, I have some reservations about the API. We sure do have a
> > lot
> > > of
> > > > > async stuff in the system, and I would suggest to stick to the
> usual
> > > > design
> > > > > - create a separate thread pool, add a single property to control
> the
> > > > size
> > > > > of the pool.
> > > > > Alternatively, we may consider using public pool for that. May I
> ask
> > if
> > > > > there is an example use case which doesn’t work with public pool?
> > > > >
> > > > > For .NET, agree that we should follow the rules and APIs of the
> > > platform,
> > > > > so the behavior might slightly differ.
> > > > >
> > > > > Thanks,
> > > > > Stan
> > > > >
> > > > > > On 24 Mar 2021, at 09:52, Pavel Tupitsyn 
> > > wrote:
> > > > > >
> > > > > > Igniters, since there are no more comments and/or review
> feedback,
> > > > > > I'm going to merge the changes by the end of the week.
> > > > > >
> > > > > >> On Mon, Mar 22, 2021 at 10:37 PM Pavel Tupitsyn <
> > > ptupit...@apache.org
> > > > >
> > > > > >> wrote:
> > > > > >>
> > > > > >> Ready for review:
> > > > > >> https://github.com/apache/ignite/pull/8870
> > > > > >>
> > > > > >> On Sun, Mar 21, 2021 at 8:09 PM Pavel Tupitsyn <
> > > ptupit...@apache.org>
> > > > > >> wrote:
> > > > > >>
> > > > > >>> Simple benchmark added - see JmhCacheAsyncListenBenchmark in
> the
> > > PR.
> > > > > >>> There is a 6-8% drop (1 client, 2 servers, 1 machine, int
> 

Re: IEP-70: Async Continuation Executor

[Alexei Scherbakov]

Pavel,

The change still looks a bit risky to me, because the default executor is
set to commonPool and can alter existing deployments in unpredictable ways,
if commonPool is heavily used for other purposes.

Runnable::run usage is not obvious as well and should be properly
documented as a way to return to old behavior.

I'm not sure we need it in 2.X for the reasons above - we already have ways
to control a listener's behavior - it's a matter of good documentation to
me.






чт, 25 мар. 2021 г. в 15:33, Pavel Tupitsyn :

> Alexei,
>
> > Sometimes it's more desirable to execute the listener in the same thread
> > It's up to the user to decide.
>
> Yes, we give users a choice to configure the executor as Runnable::run and
> use the same thread if needed.
> However, it should not be the default behavior as explained above (bad
> usability, unexpected major issues).
>
> On Thu, Mar 25, 2021 at 3:06 PM Alexei Scherbakov <
> alexey.scherbak...@gmail.com> wrote:
>
> > Pavel,
> >
> > While I understand the issue and overall agree with you, I'm against the
> > execution of listeners in separate thread pool by default.
> >
> > Sometimes it's more desirable to execute the listener in the same thread,
> > for example if it's some lightweight closure.
> >
> > It's up to the user to decide.
> >
> > I think the IgniteFuture.listen method should be properly documented to
> > avoid execution of cluster operations or any other potentially blocking
> > operations inside the listener.
> >
> > Otherwise listenAsync should be used.
> >
> >
> >
> > чт, 25 мар. 2021 г. в 14:04, Pavel Tupitsyn :
> >
> > > Stan,
> > >
> > > We have thread pools dedicated for specific purposes, like cache
> > (striped),
> > > compute (pub), query, etc
> > > As I understand it, the reason here is to limit the number of threads
> > > dedicated to a given subsystem.
> > > For example, Compute may be overloaded with work, but Cache and
> Discovery
> > > will keep going.
> > >
> > > This is different from async continuations, which are arbitrary user
> > code.
> > > So what is the benefit of having a new user pool for arbitrary code
> that
> > is
> > > probably not related to Ignite at all?
> > >
> > > On Thu, Mar 25, 2021 at 1:31 PM  wrote:
> > >
> > > > Pavel,
> > > >
> > > > This is a great work, fully agree with the overall idea and approach.
> > > >
> > > > However, I have some reservations about the API. We sure do have a
> lot
> > of
> > > > async stuff in the system, and I would suggest to stick to the usual
> > > design
> > > > - create a separate thread pool, add a single property to control the
> > > size
> > > > of the pool.
> > > > Alternatively, we may consider using public pool for that. May I ask
> if
> > > > there is an example use case which doesn’t work with public pool?
> > > >
> > > > For .NET, agree that we should follow the rules and APIs of the
> > platform,
> > > > so the behavior might slightly differ.
> > > >
> > > > Thanks,
> > > > Stan
> > > >
> > > > > On 24 Mar 2021, at 09:52, Pavel Tupitsyn 
> > wrote:
> > > > >
> > > > > Igniters, since there are no more comments and/or review feedback,
> > > > > I'm going to merge the changes by the end of the week.
> > > > >
> > > > >> On Mon, Mar 22, 2021 at 10:37 PM Pavel Tupitsyn <
> > ptupit...@apache.org
> > > >
> > > > >> wrote:
> > > > >>
> > > > >> Ready for review:
> > > > >> https://github.com/apache/ignite/pull/8870
> > > > >>
> > > > >> On Sun, Mar 21, 2021 at 8:09 PM Pavel Tupitsyn <
> > ptupit...@apache.org>
> > > > >> wrote:
> > > > >>
> > > > >>> Simple benchmark added - see JmhCacheAsyncListenBenchmark in the
> > PR.
> > > > >>> There is a 6-8% drop (1 client, 2 servers, 1 machine, int
> key/val).
> > > > >>> I expect this difference to become barely observable on
> real-world
> > > > >>> workloads.
> > > > >>>
> > > > >>> On Thu, Mar 18, 2021 at 12:35 PM Pavel Tupitsyn <
> > > ptupit...@apache.org>
> > > > >>> wrote:
> > > > >>>
> > > >  Denis,
> > > > 
> > > >  For a reproducer, please see
> > CacheAsyncContinuationExecutorTest.java
> > > > in
> > > >  the linked PoC [1]
> > > > 
> > > >  [1]
> > > > 
> > > >
> > >
> >
> https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54
> > > > 
> > > >  On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson <
> > > >  raymond_wil...@trimble.com> wrote:
> > > > 
> > > > > We implemented the ContinueWith() suggestion from Pavel, and it
> > > works
> > > > > well
> > > > > so far in testing, though we do not have data to support if
> there
> > > is
> > > > or
> > > > > is
> > > > > not a performance penalty in our use case..
> > > > >
> > > > > To lend another vote to the 'Don't do continuations on the
> > striped
> > > > > thread
> > > > > pool' line of thinking: Deadlocking is an issue as is
> starvation.
> > > In
> > > > > some
> > > > > ways starvation is more insidious because 

Re: IEP-70: Async Continuation Executor

[Stanislav Lukyanov]

I don't agree that the code isn't related to Ignite - it is something that the 
user does via Ignite API,
so they would be right to expect Ignite to handle the invocations.

One more thing I'd like to mention - monitoring.
I don't think commonPool exposes anything to JMX by default, and a custom 
executor created by user certainly won't either.
If we create the pool we can register the MX Bean for it, as we usually do.

Let's split this topic into two questions: what should be the default executor, 
and what should be the configuration options.

1. What should be the default executor?
My first choice would be public thread pool.
In fact, the continuations are likely to be created in two contexts: on a 
client, or in a compute job on a server.
In both cases, public pool seems alright to me.

My second choice - a separate new IgniteThreadPoolExecutor.
I like it better than the proposed commonPool()l because of the monitoring 
options, and because we can control it better if needed.

2. What should be the configuration options?
If we go with the public pool - no additional options needed.

For a new ignite pool, we'll need one more option with the size of the pool - 
easy to understand and should be more than enough for the user.

I don't like accepting the executor from the user, mostly because I don't see 
benefits in this flexibility, but it makes API non-uniform 
(e.g. if the user wants to make all pools smaller they'd need to change size 
properties for other pools, but create a custom pool for this one),
and makes it harder to configure parallelism level via XML (which is the most 
common thing people want to configure for thread pools).

WDYT?

Thanks,
Stan



> On 25 Mar 2021, at 14:04, Pavel Tupitsyn  wrote:
> 
> Stan,
> 
> We have thread pools dedicated for specific purposes, like cache (striped),
> compute (pub), query, etc
> As I understand it, the reason here is to limit the number of threads
> dedicated to a given subsystem.
> For example, Compute may be overloaded with work, but Cache and Discovery
> will keep going.
> 
> This is different from async continuations, which are arbitrary user code.
> So what is the benefit of having a new user pool for arbitrary code that is
> probably not related to Ignite at all?
> 
> On Thu, Mar 25, 2021 at 1:31 PM  wrote:
> 
>> Pavel,
>> 
>> This is a great work, fully agree with the overall idea and approach.
>> 
>> However, I have some reservations about the API. We sure do have a lot of
>> async stuff in the system, and I would suggest to stick to the usual design
>> - create a separate thread pool, add a single property to control the size
>> of the pool.
>> Alternatively, we may consider using public pool for that. May I ask if
>> there is an example use case which doesn’t work with public pool?
>> 
>> For .NET, agree that we should follow the rules and APIs of the platform,
>> so the behavior might slightly differ.
>> 
>> Thanks,
>> Stan
>> 
>>> On 24 Mar 2021, at 09:52, Pavel Tupitsyn  wrote:
>>> 
>>> Igniters, since there are no more comments and/or review feedback,
>>> I'm going to merge the changes by the end of the week.
>>> 
 On Mon, Mar 22, 2021 at 10:37 PM Pavel Tupitsyn 
 wrote:
 
 Ready for review:
 https://github.com/apache/ignite/pull/8870
 
 On Sun, Mar 21, 2021 at 8:09 PM Pavel Tupitsyn 
 wrote:
 
> Simple benchmark added - see JmhCacheAsyncListenBenchmark in the PR.
> There is a 6-8% drop (1 client, 2 servers, 1 machine, int key/val).
> I expect this difference to become barely observable on real-world
> workloads.
> 
> On Thu, Mar 18, 2021 at 12:35 PM Pavel Tupitsyn 
> wrote:
> 
>> Denis,
>> 
>> For a reproducer, please see CacheAsyncContinuationExecutorTest.java
>> in
>> the linked PoC [1]
>> 
>> [1]
>> 
>> https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54
>> 
>> On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson <
>> raymond_wil...@trimble.com> wrote:
>> 
>>> We implemented the ContinueWith() suggestion from Pavel, and it works
>>> well
>>> so far in testing, though we do not have data to support if there is
>> or
>>> is
>>> not a performance penalty in our use case..
>>> 
>>> To lend another vote to the 'Don't do continuations on the striped
>>> thread
>>> pool' line of thinking: Deadlocking is an issue as is starvation. In
>>> some
>>> ways starvation is more insidious because by the time things stop
>>> working
>>> the cause and effect distance may be large.
>>> 
>>> I appreciate the documentation does make statements about not
>> performing
>>> cache operations in a continuation due to deadlock possibilities, but
>>> that
>>> statement does not reveal why this is an issue. It is less a case of
>> a
>>> async cache operation being followed by some other cache operation
>> (an

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Alexei,

> Sometimes it's more desirable to execute the listener in the same thread
> It's up to the user to decide.

Yes, we give users a choice to configure the executor as Runnable::run and
use the same thread if needed.
However, it should not be the default behavior as explained above (bad
usability, unexpected major issues).

On Thu, Mar 25, 2021 at 3:06 PM Alexei Scherbakov <
alexey.scherbak...@gmail.com> wrote:

> Pavel,
>
> While I understand the issue and overall agree with you, I'm against the
> execution of listeners in separate thread pool by default.
>
> Sometimes it's more desirable to execute the listener in the same thread,
> for example if it's some lightweight closure.
>
> It's up to the user to decide.
>
> I think the IgniteFuture.listen method should be properly documented to
> avoid execution of cluster operations or any other potentially blocking
> operations inside the listener.
>
> Otherwise listenAsync should be used.
>
>
>
> чт, 25 мар. 2021 г. в 14:04, Pavel Tupitsyn :
>
> > Stan,
> >
> > We have thread pools dedicated for specific purposes, like cache
> (striped),
> > compute (pub), query, etc
> > As I understand it, the reason here is to limit the number of threads
> > dedicated to a given subsystem.
> > For example, Compute may be overloaded with work, but Cache and Discovery
> > will keep going.
> >
> > This is different from async continuations, which are arbitrary user
> code.
> > So what is the benefit of having a new user pool for arbitrary code that
> is
> > probably not related to Ignite at all?
> >
> > On Thu, Mar 25, 2021 at 1:31 PM  wrote:
> >
> > > Pavel,
> > >
> > > This is a great work, fully agree with the overall idea and approach.
> > >
> > > However, I have some reservations about the API. We sure do have a lot
> of
> > > async stuff in the system, and I would suggest to stick to the usual
> > design
> > > - create a separate thread pool, add a single property to control the
> > size
> > > of the pool.
> > > Alternatively, we may consider using public pool for that. May I ask if
> > > there is an example use case which doesn’t work with public pool?
> > >
> > > For .NET, agree that we should follow the rules and APIs of the
> platform,
> > > so the behavior might slightly differ.
> > >
> > > Thanks,
> > > Stan
> > >
> > > > On 24 Mar 2021, at 09:52, Pavel Tupitsyn 
> wrote:
> > > >
> > > > Igniters, since there are no more comments and/or review feedback,
> > > > I'm going to merge the changes by the end of the week.
> > > >
> > > >> On Mon, Mar 22, 2021 at 10:37 PM Pavel Tupitsyn <
> ptupit...@apache.org
> > >
> > > >> wrote:
> > > >>
> > > >> Ready for review:
> > > >> https://github.com/apache/ignite/pull/8870
> > > >>
> > > >> On Sun, Mar 21, 2021 at 8:09 PM Pavel Tupitsyn <
> ptupit...@apache.org>
> > > >> wrote:
> > > >>
> > > >>> Simple benchmark added - see JmhCacheAsyncListenBenchmark in the
> PR.
> > > >>> There is a 6-8% drop (1 client, 2 servers, 1 machine, int key/val).
> > > >>> I expect this difference to become barely observable on real-world
> > > >>> workloads.
> > > >>>
> > > >>> On Thu, Mar 18, 2021 at 12:35 PM Pavel Tupitsyn <
> > ptupit...@apache.org>
> > > >>> wrote:
> > > >>>
> > >  Denis,
> > > 
> > >  For a reproducer, please see
> CacheAsyncContinuationExecutorTest.java
> > > in
> > >  the linked PoC [1]
> > > 
> > >  [1]
> > > 
> > >
> >
> https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54
> > > 
> > >  On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson <
> > >  raymond_wil...@trimble.com> wrote:
> > > 
> > > > We implemented the ContinueWith() suggestion from Pavel, and it
> > works
> > > > well
> > > > so far in testing, though we do not have data to support if there
> > is
> > > or
> > > > is
> > > > not a performance penalty in our use case..
> > > >
> > > > To lend another vote to the 'Don't do continuations on the
> striped
> > > > thread
> > > > pool' line of thinking: Deadlocking is an issue as is starvation.
> > In
> > > > some
> > > > ways starvation is more insidious because by the time things stop
> > > > working
> > > > the cause and effect distance may be large.
> > > >
> > > > I appreciate the documentation does make statements about not
> > > performing
> > > > cache operations in a continuation due to deadlock possibilities,
> > but
> > > > that
> > > > statement does not reveal why this is an issue. It is less a case
> > of
> > > a
> > > > async cache operation being followed by some other cache
> operation
> > > (an
> > > > immediate issue), and more a general case of the continuation of
> > > > application logic using a striped pool thread in a way that might
> > > mean
> > > > that
> > > > thread is never given back - it's now just a piece of the
> > application
> > > > infrastructure until some other 

Re: IEP-70: Async Continuation Executor

[Alexei Scherbakov]

Pavel,

While I understand the issue and overall agree with you, I'm against the
execution of listeners in separate thread pool by default.

Sometimes it's more desirable to execute the listener in the same thread,
for example if it's some lightweight closure.

It's up to the user to decide.

I think the IgniteFuture.listen method should be properly documented to
avoid execution of cluster operations or any other potentially blocking
operations inside the listener.

Otherwise listenAsync should be used.



чт, 25 мар. 2021 г. в 14:04, Pavel Tupitsyn :

> Stan,
>
> We have thread pools dedicated for specific purposes, like cache (striped),
> compute (pub), query, etc
> As I understand it, the reason here is to limit the number of threads
> dedicated to a given subsystem.
> For example, Compute may be overloaded with work, but Cache and Discovery
> will keep going.
>
> This is different from async continuations, which are arbitrary user code.
> So what is the benefit of having a new user pool for arbitrary code that is
> probably not related to Ignite at all?
>
> On Thu, Mar 25, 2021 at 1:31 PM  wrote:
>
> > Pavel,
> >
> > This is a great work, fully agree with the overall idea and approach.
> >
> > However, I have some reservations about the API. We sure do have a lot of
> > async stuff in the system, and I would suggest to stick to the usual
> design
> > - create a separate thread pool, add a single property to control the
> size
> > of the pool.
> > Alternatively, we may consider using public pool for that. May I ask if
> > there is an example use case which doesn’t work with public pool?
> >
> > For .NET, agree that we should follow the rules and APIs of the platform,
> > so the behavior might slightly differ.
> >
> > Thanks,
> > Stan
> >
> > > On 24 Mar 2021, at 09:52, Pavel Tupitsyn  wrote:
> > >
> > > Igniters, since there are no more comments and/or review feedback,
> > > I'm going to merge the changes by the end of the week.
> > >
> > >> On Mon, Mar 22, 2021 at 10:37 PM Pavel Tupitsyn  >
> > >> wrote:
> > >>
> > >> Ready for review:
> > >> https://github.com/apache/ignite/pull/8870
> > >>
> > >> On Sun, Mar 21, 2021 at 8:09 PM Pavel Tupitsyn 
> > >> wrote:
> > >>
> > >>> Simple benchmark added - see JmhCacheAsyncListenBenchmark in the PR.
> > >>> There is a 6-8% drop (1 client, 2 servers, 1 machine, int key/val).
> > >>> I expect this difference to become barely observable on real-world
> > >>> workloads.
> > >>>
> > >>> On Thu, Mar 18, 2021 at 12:35 PM Pavel Tupitsyn <
> ptupit...@apache.org>
> > >>> wrote:
> > >>>
> >  Denis,
> > 
> >  For a reproducer, please see CacheAsyncContinuationExecutorTest.java
> > in
> >  the linked PoC [1]
> > 
> >  [1]
> > 
> >
> https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54
> > 
> >  On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson <
> >  raymond_wil...@trimble.com> wrote:
> > 
> > > We implemented the ContinueWith() suggestion from Pavel, and it
> works
> > > well
> > > so far in testing, though we do not have data to support if there
> is
> > or
> > > is
> > > not a performance penalty in our use case..
> > >
> > > To lend another vote to the 'Don't do continuations on the striped
> > > thread
> > > pool' line of thinking: Deadlocking is an issue as is starvation.
> In
> > > some
> > > ways starvation is more insidious because by the time things stop
> > > working
> > > the cause and effect distance may be large.
> > >
> > > I appreciate the documentation does make statements about not
> > performing
> > > cache operations in a continuation due to deadlock possibilities,
> but
> > > that
> > > statement does not reveal why this is an issue. It is less a case
> of
> > a
> > > async cache operation being followed by some other cache operation
> > (an
> > > immediate issue), and more a general case of the continuation of
> > > application logic using a striped pool thread in a way that might
> > mean
> > > that
> > > thread is never given back - it's now just a piece of the
> application
> > > infrastructure until some other application activity schedules away
> > from
> > > that thread (eg: by ContinueWith or some other async operation in
> the
> > > application code that releases the thread). To be fair, beyond
> > > structures
> > > like ContinueWith(), it is not obvious how that continuation thread
> > > should
> > > be handed back. This will be the same behaviour for dedicated
> > > continuation
> > > pools, but with far less risk in the absence of ContinueWith()
> > > constructs.
> > >
> > > In the .Net world this is becoming more of an issue as fewer .Net
> use
> > > cases
> > > outside of UI bother with synchronization contexts by default.
> > >
> > > Raymond.
> > >
> > >
> > > 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Stan,

We have thread pools dedicated for specific purposes, like cache (striped),
compute (pub), query, etc
As I understand it, the reason here is to limit the number of threads
dedicated to a given subsystem.
For example, Compute may be overloaded with work, but Cache and Discovery
will keep going.

This is different from async continuations, which are arbitrary user code.
So what is the benefit of having a new user pool for arbitrary code that is
probably not related to Ignite at all?

On Thu, Mar 25, 2021 at 1:31 PM  wrote:

> Pavel,
>
> This is a great work, fully agree with the overall idea and approach.
>
> However, I have some reservations about the API. We sure do have a lot of
> async stuff in the system, and I would suggest to stick to the usual design
> - create a separate thread pool, add a single property to control the size
> of the pool.
> Alternatively, we may consider using public pool for that. May I ask if
> there is an example use case which doesn’t work with public pool?
>
> For .NET, agree that we should follow the rules and APIs of the platform,
> so the behavior might slightly differ.
>
> Thanks,
> Stan
>
> > On 24 Mar 2021, at 09:52, Pavel Tupitsyn  wrote:
> >
> > Igniters, since there are no more comments and/or review feedback,
> > I'm going to merge the changes by the end of the week.
> >
> >> On Mon, Mar 22, 2021 at 10:37 PM Pavel Tupitsyn 
> >> wrote:
> >>
> >> Ready for review:
> >> https://github.com/apache/ignite/pull/8870
> >>
> >> On Sun, Mar 21, 2021 at 8:09 PM Pavel Tupitsyn 
> >> wrote:
> >>
> >>> Simple benchmark added - see JmhCacheAsyncListenBenchmark in the PR.
> >>> There is a 6-8% drop (1 client, 2 servers, 1 machine, int key/val).
> >>> I expect this difference to become barely observable on real-world
> >>> workloads.
> >>>
> >>> On Thu, Mar 18, 2021 at 12:35 PM Pavel Tupitsyn 
> >>> wrote:
> >>>
>  Denis,
> 
>  For a reproducer, please see CacheAsyncContinuationExecutorTest.java
> in
>  the linked PoC [1]
> 
>  [1]
> 
> https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54
> 
>  On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson <
>  raymond_wil...@trimble.com> wrote:
> 
> > We implemented the ContinueWith() suggestion from Pavel, and it works
> > well
> > so far in testing, though we do not have data to support if there is
> or
> > is
> > not a performance penalty in our use case..
> >
> > To lend another vote to the 'Don't do continuations on the striped
> > thread
> > pool' line of thinking: Deadlocking is an issue as is starvation. In
> > some
> > ways starvation is more insidious because by the time things stop
> > working
> > the cause and effect distance may be large.
> >
> > I appreciate the documentation does make statements about not
> performing
> > cache operations in a continuation due to deadlock possibilities, but
> > that
> > statement does not reveal why this is an issue. It is less a case of
> a
> > async cache operation being followed by some other cache operation
> (an
> > immediate issue), and more a general case of the continuation of
> > application logic using a striped pool thread in a way that might
> mean
> > that
> > thread is never given back - it's now just a piece of the application
> > infrastructure until some other application activity schedules away
> from
> > that thread (eg: by ContinueWith or some other async operation in the
> > application code that releases the thread). To be fair, beyond
> > structures
> > like ContinueWith(), it is not obvious how that continuation thread
> > should
> > be handed back. This will be the same behaviour for dedicated
> > continuation
> > pools, but with far less risk in the absence of ContinueWith()
> > constructs.
> >
> > In the .Net world this is becoming more of an issue as fewer .Net use
> > cases
> > outside of UI bother with synchronization contexts by default.
> >
> > Raymond.
> >
> >
> > On Thu, Mar 18, 2021 at 9:56 AM Valentin Kulichenko <
> > valentin.kuliche...@gmail.com> wrote:
> >
> >> Hi Denis,
> >>
> >> I think Pavel's main point is that behavior is unpredictable. For
> > example,
> >> AFAIK, putAsync can be executed in the main thread instead of the
> > striped
> >> pool thread if the operation is local. The listener can also be
> > executed in
> >> the main thread - this happens if the future is completed prior to
> > listener
> >> invocation (this is actually quite possible in the unit test
> > environment
> >> causing the test to pass). Finally, I'm pretty sure there are many
> > cases
> >> when a deadlock does not occur right away, but instead it will
> reveal
> >> itself under high load due to thread starvation. The latter 

Re: IEP-70: Async Continuation Executor

[stanlukyanov]

Pavel,

This is a great work, fully agree with the overall idea and approach.

However, I have some reservations about the API. We sure do have a lot of async 
stuff in the system, and I would suggest to stick to the usual design - create 
a separate thread pool, add a single property to control the size of the pool.
Alternatively, we may consider using public pool for that. May I ask if there 
is an example use case which doesn’t work with public pool?

For .NET, agree that we should follow the rules and APIs of the platform, so 
the behavior might slightly differ.

Thanks,
Stan

> On 24 Mar 2021, at 09:52, Pavel Tupitsyn  wrote:
> 
> Igniters, since there are no more comments and/or review feedback,
> I'm going to merge the changes by the end of the week.
> 
>> On Mon, Mar 22, 2021 at 10:37 PM Pavel Tupitsyn 
>> wrote:
>> 
>> Ready for review:
>> https://github.com/apache/ignite/pull/8870
>> 
>> On Sun, Mar 21, 2021 at 8:09 PM Pavel Tupitsyn 
>> wrote:
>> 
>>> Simple benchmark added - see JmhCacheAsyncListenBenchmark in the PR.
>>> There is a 6-8% drop (1 client, 2 servers, 1 machine, int key/val).
>>> I expect this difference to become barely observable on real-world
>>> workloads.
>>> 
>>> On Thu, Mar 18, 2021 at 12:35 PM Pavel Tupitsyn 
>>> wrote:
>>> 
 Denis,
 
 For a reproducer, please see CacheAsyncContinuationExecutorTest.java in
 the linked PoC [1]
 
 [1]
 https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54
 
 On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson <
 raymond_wil...@trimble.com> wrote:
 
> We implemented the ContinueWith() suggestion from Pavel, and it works
> well
> so far in testing, though we do not have data to support if there is or
> is
> not a performance penalty in our use case..
> 
> To lend another vote to the 'Don't do continuations on the striped
> thread
> pool' line of thinking: Deadlocking is an issue as is starvation. In
> some
> ways starvation is more insidious because by the time things stop
> working
> the cause and effect distance may be large.
> 
> I appreciate the documentation does make statements about not performing
> cache operations in a continuation due to deadlock possibilities, but
> that
> statement does not reveal why this is an issue. It is less a case of a
> async cache operation being followed by some other cache operation (an
> immediate issue), and more a general case of the continuation of
> application logic using a striped pool thread in a way that might mean
> that
> thread is never given back - it's now just a piece of the application
> infrastructure until some other application activity schedules away from
> that thread (eg: by ContinueWith or some other async operation in the
> application code that releases the thread). To be fair, beyond
> structures
> like ContinueWith(), it is not obvious how that continuation thread
> should
> be handed back. This will be the same behaviour for dedicated
> continuation
> pools, but with far less risk in the absence of ContinueWith()
> constructs.
> 
> In the .Net world this is becoming more of an issue as fewer .Net use
> cases
> outside of UI bother with synchronization contexts by default.
> 
> Raymond.
> 
> 
> On Thu, Mar 18, 2021 at 9:56 AM Valentin Kulichenko <
> valentin.kuliche...@gmail.com> wrote:
> 
>> Hi Denis,
>> 
>> I think Pavel's main point is that behavior is unpredictable. For
> example,
>> AFAIK, putAsync can be executed in the main thread instead of the
> striped
>> pool thread if the operation is local. The listener can also be
> executed in
>> the main thread - this happens if the future is completed prior to
> listener
>> invocation (this is actually quite possible in the unit test
> environment
>> causing the test to pass). Finally, I'm pretty sure there are many
> cases
>> when a deadlock does not occur right away, but instead it will reveal
>> itself under high load due to thread starvation. The latter type of
> issues
>> is the most dangerous because they are often reproduced only in
> production.
>> Finally, there are performance considerations as well - cache
> operations
>> and listeners share the same fixed-sized pool which can have negative
>> effects.
>> 
>> I'm OK with the change. Although, it might be better to introduce a
> new
>> fixed-sized pool instead of ForkJoinPool for listeners, simply
> because this
>> is the approach taken throughout the project. But this is up to a
> debate.
>> 
>> -Val
>> 
>> On Wed, Mar 17, 2021 at 11:31 AM Denis Garus 
> wrote:
>> 
>>> Pavel,
>>> I tried this:
>>> 
>>> @Test
>>> public void 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Igniters, since there are no more comments and/or review feedback,
I'm going to merge the changes by the end of the week.

On Mon, Mar 22, 2021 at 10:37 PM Pavel Tupitsyn 
wrote:

> Ready for review:
> https://github.com/apache/ignite/pull/8870
>
> On Sun, Mar 21, 2021 at 8:09 PM Pavel Tupitsyn 
> wrote:
>
>> Simple benchmark added - see JmhCacheAsyncListenBenchmark in the PR.
>> There is a 6-8% drop (1 client, 2 servers, 1 machine, int key/val).
>> I expect this difference to become barely observable on real-world
>> workloads.
>>
>> On Thu, Mar 18, 2021 at 12:35 PM Pavel Tupitsyn 
>> wrote:
>>
>>> Denis,
>>>
>>> For a reproducer, please see CacheAsyncContinuationExecutorTest.java in
>>> the linked PoC [1]
>>>
>>> [1]
>>> https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54
>>>
>>> On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson <
>>> raymond_wil...@trimble.com> wrote:
>>>
 We implemented the ContinueWith() suggestion from Pavel, and it works
 well
 so far in testing, though we do not have data to support if there is or
 is
 not a performance penalty in our use case..

 To lend another vote to the 'Don't do continuations on the striped
 thread
 pool' line of thinking: Deadlocking is an issue as is starvation. In
 some
 ways starvation is more insidious because by the time things stop
 working
 the cause and effect distance may be large.

 I appreciate the documentation does make statements about not performing
 cache operations in a continuation due to deadlock possibilities, but
 that
 statement does not reveal why this is an issue. It is less a case of a
 async cache operation being followed by some other cache operation (an
 immediate issue), and more a general case of the continuation of
 application logic using a striped pool thread in a way that might mean
 that
 thread is never given back - it's now just a piece of the application
 infrastructure until some other application activity schedules away from
 that thread (eg: by ContinueWith or some other async operation in the
 application code that releases the thread). To be fair, beyond
 structures
 like ContinueWith(), it is not obvious how that continuation thread
 should
 be handed back. This will be the same behaviour for dedicated
 continuation
 pools, but with far less risk in the absence of ContinueWith()
 constructs.

 In the .Net world this is becoming more of an issue as fewer .Net use
 cases
 outside of UI bother with synchronization contexts by default.

 Raymond.


 On Thu, Mar 18, 2021 at 9:56 AM Valentin Kulichenko <
 valentin.kuliche...@gmail.com> wrote:

 > Hi Denis,
 >
 > I think Pavel's main point is that behavior is unpredictable. For
 example,
 > AFAIK, putAsync can be executed in the main thread instead of the
 striped
 > pool thread if the operation is local. The listener can also be
 executed in
 > the main thread - this happens if the future is completed prior to
 listener
 > invocation (this is actually quite possible in the unit test
 environment
 > causing the test to pass). Finally, I'm pretty sure there are many
 cases
 > when a deadlock does not occur right away, but instead it will reveal
 > itself under high load due to thread starvation. The latter type of
 issues
 > is the most dangerous because they are often reproduced only in
 production.
 > Finally, there are performance considerations as well - cache
 operations
 > and listeners share the same fixed-sized pool which can have negative
 > effects.
 >
 > I'm OK with the change. Although, it might be better to introduce a
 new
 > fixed-sized pool instead of ForkJoinPool for listeners, simply
 because this
 > is the approach taken throughout the project. But this is up to a
 debate.
 >
 > -Val
 >
 > On Wed, Mar 17, 2021 at 11:31 AM Denis Garus 
 wrote:
 >
 > > Pavel,
 > > I tried this:
 > >
 > > @Test
 > > public void test() throws Exception {
 > > IgniteCache cache =
 > > startGrid().getOrCreateCache("test_cache");
 > >
 > > cache.putAsync(1, "one").listen(f -> cache.replace(1, "two"));
 > >
 > > assertEquals("two", cache.get(1));
 > > }
 > >
 > > and this test is green.
 > > I believe that an user can make listener that leads to deadlock, but
 > > the example in the IEP does not reflect this.
 > >
 > >
 > >
 > > ср, 17 мар. 2021 г. в 17:36, Вячеслав Коптилин <
 slava.kopti...@gmail.com
 > >:
 > >
 > > > Hi Pavel,
 > > >
 > > > > Not a good excuse really. We have a usability problem, you have
 to
 > > admit
 > > > it.
 > > > Fair enough. I agree that 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Ready for review:
https://github.com/apache/ignite/pull/8870

On Sun, Mar 21, 2021 at 8:09 PM Pavel Tupitsyn  wrote:

> Simple benchmark added - see JmhCacheAsyncListenBenchmark in the PR.
> There is a 6-8% drop (1 client, 2 servers, 1 machine, int key/val).
> I expect this difference to become barely observable on real-world
> workloads.
>
> On Thu, Mar 18, 2021 at 12:35 PM Pavel Tupitsyn 
> wrote:
>
>> Denis,
>>
>> For a reproducer, please see CacheAsyncContinuationExecutorTest.java in
>> the linked PoC [1]
>>
>> [1]
>> https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54
>>
>> On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson <
>> raymond_wil...@trimble.com> wrote:
>>
>>> We implemented the ContinueWith() suggestion from Pavel, and it works
>>> well
>>> so far in testing, though we do not have data to support if there is or
>>> is
>>> not a performance penalty in our use case..
>>>
>>> To lend another vote to the 'Don't do continuations on the striped thread
>>> pool' line of thinking: Deadlocking is an issue as is starvation. In some
>>> ways starvation is more insidious because by the time things stop working
>>> the cause and effect distance may be large.
>>>
>>> I appreciate the documentation does make statements about not performing
>>> cache operations in a continuation due to deadlock possibilities, but
>>> that
>>> statement does not reveal why this is an issue. It is less a case of a
>>> async cache operation being followed by some other cache operation (an
>>> immediate issue), and more a general case of the continuation of
>>> application logic using a striped pool thread in a way that might mean
>>> that
>>> thread is never given back - it's now just a piece of the application
>>> infrastructure until some other application activity schedules away from
>>> that thread (eg: by ContinueWith or some other async operation in the
>>> application code that releases the thread). To be fair, beyond structures
>>> like ContinueWith(), it is not obvious how that continuation thread
>>> should
>>> be handed back. This will be the same behaviour for dedicated
>>> continuation
>>> pools, but with far less risk in the absence of ContinueWith()
>>> constructs.
>>>
>>> In the .Net world this is becoming more of an issue as fewer .Net use
>>> cases
>>> outside of UI bother with synchronization contexts by default.
>>>
>>> Raymond.
>>>
>>>
>>> On Thu, Mar 18, 2021 at 9:56 AM Valentin Kulichenko <
>>> valentin.kuliche...@gmail.com> wrote:
>>>
>>> > Hi Denis,
>>> >
>>> > I think Pavel's main point is that behavior is unpredictable. For
>>> example,
>>> > AFAIK, putAsync can be executed in the main thread instead of the
>>> striped
>>> > pool thread if the operation is local. The listener can also be
>>> executed in
>>> > the main thread - this happens if the future is completed prior to
>>> listener
>>> > invocation (this is actually quite possible in the unit test
>>> environment
>>> > causing the test to pass). Finally, I'm pretty sure there are many
>>> cases
>>> > when a deadlock does not occur right away, but instead it will reveal
>>> > itself under high load due to thread starvation. The latter type of
>>> issues
>>> > is the most dangerous because they are often reproduced only in
>>> production.
>>> > Finally, there are performance considerations as well - cache
>>> operations
>>> > and listeners share the same fixed-sized pool which can have negative
>>> > effects.
>>> >
>>> > I'm OK with the change. Although, it might be better to introduce a new
>>> > fixed-sized pool instead of ForkJoinPool for listeners, simply because
>>> this
>>> > is the approach taken throughout the project. But this is up to a
>>> debate.
>>> >
>>> > -Val
>>> >
>>> > On Wed, Mar 17, 2021 at 11:31 AM Denis Garus 
>>> wrote:
>>> >
>>> > > Pavel,
>>> > > I tried this:
>>> > >
>>> > > @Test
>>> > > public void test() throws Exception {
>>> > > IgniteCache cache =
>>> > > startGrid().getOrCreateCache("test_cache");
>>> > >
>>> > > cache.putAsync(1, "one").listen(f -> cache.replace(1, "two"));
>>> > >
>>> > > assertEquals("two", cache.get(1));
>>> > > }
>>> > >
>>> > > and this test is green.
>>> > > I believe that an user can make listener that leads to deadlock, but
>>> > > the example in the IEP does not reflect this.
>>> > >
>>> > >
>>> > >
>>> > > ср, 17 мар. 2021 г. в 17:36, Вячеслав Коптилин <
>>> slava.kopti...@gmail.com
>>> > >:
>>> > >
>>> > > > Hi Pavel,
>>> > > >
>>> > > > > Not a good excuse really. We have a usability problem, you have
>>> to
>>> > > admit
>>> > > > it.
>>> > > > Fair enough. I agree that this is a usability issue, but I have
>>> doubts
>>> > > that
>>> > > > the proposed approach to overcome it is the best one.
>>> > > >
>>> > > > > Documentation won't help - no one is going to read the Javadoc
>>> for a
>>> > > > trivial method like putAsync
>>> > > > That is sad... However, I don't think that this is a strong
>>> 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Simple benchmark added - see JmhCacheAsyncListenBenchmark in the PR.
There is a 6-8% drop (1 client, 2 servers, 1 machine, int key/val).
I expect this difference to become barely observable on real-world
workloads.

On Thu, Mar 18, 2021 at 12:35 PM Pavel Tupitsyn 
wrote:

> Denis,
>
> For a reproducer, please see CacheAsyncContinuationExecutorTest.java in
> the linked PoC [1]
>
> [1]
> https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54
>
> On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson 
> wrote:
>
>> We implemented the ContinueWith() suggestion from Pavel, and it works well
>> so far in testing, though we do not have data to support if there is or is
>> not a performance penalty in our use case..
>>
>> To lend another vote to the 'Don't do continuations on the striped thread
>> pool' line of thinking: Deadlocking is an issue as is starvation. In some
>> ways starvation is more insidious because by the time things stop working
>> the cause and effect distance may be large.
>>
>> I appreciate the documentation does make statements about not performing
>> cache operations in a continuation due to deadlock possibilities, but that
>> statement does not reveal why this is an issue. It is less a case of a
>> async cache operation being followed by some other cache operation (an
>> immediate issue), and more a general case of the continuation of
>> application logic using a striped pool thread in a way that might mean
>> that
>> thread is never given back - it's now just a piece of the application
>> infrastructure until some other application activity schedules away from
>> that thread (eg: by ContinueWith or some other async operation in the
>> application code that releases the thread). To be fair, beyond structures
>> like ContinueWith(), it is not obvious how that continuation thread should
>> be handed back. This will be the same behaviour for dedicated continuation
>> pools, but with far less risk in the absence of ContinueWith() constructs.
>>
>> In the .Net world this is becoming more of an issue as fewer .Net use
>> cases
>> outside of UI bother with synchronization contexts by default.
>>
>> Raymond.
>>
>>
>> On Thu, Mar 18, 2021 at 9:56 AM Valentin Kulichenko <
>> valentin.kuliche...@gmail.com> wrote:
>>
>> > Hi Denis,
>> >
>> > I think Pavel's main point is that behavior is unpredictable. For
>> example,
>> > AFAIK, putAsync can be executed in the main thread instead of the
>> striped
>> > pool thread if the operation is local. The listener can also be
>> executed in
>> > the main thread - this happens if the future is completed prior to
>> listener
>> > invocation (this is actually quite possible in the unit test environment
>> > causing the test to pass). Finally, I'm pretty sure there are many cases
>> > when a deadlock does not occur right away, but instead it will reveal
>> > itself under high load due to thread starvation. The latter type of
>> issues
>> > is the most dangerous because they are often reproduced only in
>> production.
>> > Finally, there are performance considerations as well - cache operations
>> > and listeners share the same fixed-sized pool which can have negative
>> > effects.
>> >
>> > I'm OK with the change. Although, it might be better to introduce a new
>> > fixed-sized pool instead of ForkJoinPool for listeners, simply because
>> this
>> > is the approach taken throughout the project. But this is up to a
>> debate.
>> >
>> > -Val
>> >
>> > On Wed, Mar 17, 2021 at 11:31 AM Denis Garus 
>> wrote:
>> >
>> > > Pavel,
>> > > I tried this:
>> > >
>> > > @Test
>> > > public void test() throws Exception {
>> > > IgniteCache cache =
>> > > startGrid().getOrCreateCache("test_cache");
>> > >
>> > > cache.putAsync(1, "one").listen(f -> cache.replace(1, "two"));
>> > >
>> > > assertEquals("two", cache.get(1));
>> > > }
>> > >
>> > > and this test is green.
>> > > I believe that an user can make listener that leads to deadlock, but
>> > > the example in the IEP does not reflect this.
>> > >
>> > >
>> > >
>> > > ср, 17 мар. 2021 г. в 17:36, Вячеслав Коптилин <
>> slava.kopti...@gmail.com
>> > >:
>> > >
>> > > > Hi Pavel,
>> > > >
>> > > > > Not a good excuse really. We have a usability problem, you have to
>> > > admit
>> > > > it.
>> > > > Fair enough. I agree that this is a usability issue, but I have
>> doubts
>> > > that
>> > > > the proposed approach to overcome it is the best one.
>> > > >
>> > > > > Documentation won't help - no one is going to read the Javadoc
>> for a
>> > > > trivial method like putAsync
>> > > > That is sad... However, I don't think that this is a strong argument
>> > > here.
>> > > >
>> > > > This is just my opinion. Let's see what other community members
>> have to
>> > > > say.
>> > > >
>> > > > Thanks,
>> > > > S.
>> > > >
>> > > >
>> > > > ср, 17 мар. 2021 г. в 17:01, Pavel Tupitsyn :
>> > > >
>> > > > > > the user should use the right API
>> > > > >
>> > > > > Not a 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Denis,

For a reproducer, please see CacheAsyncContinuationExecutorTest.java in the
linked PoC [1]

[1]
https://github.com/apache/ignite/pull/8870/files#diff-c788c12013622093df07d8f628a6e8c5fb0c15007f0787f2d459dbb3e377fc5aR54

On Thu, Mar 18, 2021 at 1:58 AM Raymond Wilson 
wrote:

> We implemented the ContinueWith() suggestion from Pavel, and it works well
> so far in testing, though we do not have data to support if there is or is
> not a performance penalty in our use case..
>
> To lend another vote to the 'Don't do continuations on the striped thread
> pool' line of thinking: Deadlocking is an issue as is starvation. In some
> ways starvation is more insidious because by the time things stop working
> the cause and effect distance may be large.
>
> I appreciate the documentation does make statements about not performing
> cache operations in a continuation due to deadlock possibilities, but that
> statement does not reveal why this is an issue. It is less a case of a
> async cache operation being followed by some other cache operation (an
> immediate issue), and more a general case of the continuation of
> application logic using a striped pool thread in a way that might mean that
> thread is never given back - it's now just a piece of the application
> infrastructure until some other application activity schedules away from
> that thread (eg: by ContinueWith or some other async operation in the
> application code that releases the thread). To be fair, beyond structures
> like ContinueWith(), it is not obvious how that continuation thread should
> be handed back. This will be the same behaviour for dedicated continuation
> pools, but with far less risk in the absence of ContinueWith() constructs.
>
> In the .Net world this is becoming more of an issue as fewer .Net use cases
> outside of UI bother with synchronization contexts by default.
>
> Raymond.
>
>
> On Thu, Mar 18, 2021 at 9:56 AM Valentin Kulichenko <
> valentin.kuliche...@gmail.com> wrote:
>
> > Hi Denis,
> >
> > I think Pavel's main point is that behavior is unpredictable. For
> example,
> > AFAIK, putAsync can be executed in the main thread instead of the striped
> > pool thread if the operation is local. The listener can also be executed
> in
> > the main thread - this happens if the future is completed prior to
> listener
> > invocation (this is actually quite possible in the unit test environment
> > causing the test to pass). Finally, I'm pretty sure there are many cases
> > when a deadlock does not occur right away, but instead it will reveal
> > itself under high load due to thread starvation. The latter type of
> issues
> > is the most dangerous because they are often reproduced only in
> production.
> > Finally, there are performance considerations as well - cache operations
> > and listeners share the same fixed-sized pool which can have negative
> > effects.
> >
> > I'm OK with the change. Although, it might be better to introduce a new
> > fixed-sized pool instead of ForkJoinPool for listeners, simply because
> this
> > is the approach taken throughout the project. But this is up to a debate.
> >
> > -Val
> >
> > On Wed, Mar 17, 2021 at 11:31 AM Denis Garus 
> wrote:
> >
> > > Pavel,
> > > I tried this:
> > >
> > > @Test
> > > public void test() throws Exception {
> > > IgniteCache cache =
> > > startGrid().getOrCreateCache("test_cache");
> > >
> > > cache.putAsync(1, "one").listen(f -> cache.replace(1, "two"));
> > >
> > > assertEquals("two", cache.get(1));
> > > }
> > >
> > > and this test is green.
> > > I believe that an user can make listener that leads to deadlock, but
> > > the example in the IEP does not reflect this.
> > >
> > >
> > >
> > > ср, 17 мар. 2021 г. в 17:36, Вячеслав Коптилин <
> slava.kopti...@gmail.com
> > >:
> > >
> > > > Hi Pavel,
> > > >
> > > > > Not a good excuse really. We have a usability problem, you have to
> > > admit
> > > > it.
> > > > Fair enough. I agree that this is a usability issue, but I have
> doubts
> > > that
> > > > the proposed approach to overcome it is the best one.
> > > >
> > > > > Documentation won't help - no one is going to read the Javadoc for
> a
> > > > trivial method like putAsync
> > > > That is sad... However, I don't think that this is a strong argument
> > > here.
> > > >
> > > > This is just my opinion. Let's see what other community members have
> to
> > > > say.
> > > >
> > > > Thanks,
> > > > S.
> > > >
> > > >
> > > > ср, 17 мар. 2021 г. в 17:01, Pavel Tupitsyn :
> > > >
> > > > > > the user should use the right API
> > > > >
> > > > > Not a good excuse really. We have a usability problem, you have to
> > > admit
> > > > > it.
> > > > > "The brakes did not work on your car - too bad, you should have
> known
> > > > that
> > > > > on Sundays only your left foot is allowed on the pedal"
> > > > >
> > > > > This particular use case is too intricate.
> > > > > Even when you know about that, it is difficult to decide what can
> run
> > > on
> > > > > the 

Re: IEP-70: Async Continuation Executor

[Raymond Wilson]

We implemented the ContinueWith() suggestion from Pavel, and it works well
so far in testing, though we do not have data to support if there is or is
not a performance penalty in our use case..

To lend another vote to the 'Don't do continuations on the striped thread
pool' line of thinking: Deadlocking is an issue as is starvation. In some
ways starvation is more insidious because by the time things stop working
the cause and effect distance may be large.

I appreciate the documentation does make statements about not performing
cache operations in a continuation due to deadlock possibilities, but that
statement does not reveal why this is an issue. It is less a case of a
async cache operation being followed by some other cache operation (an
immediate issue), and more a general case of the continuation of
application logic using a striped pool thread in a way that might mean that
thread is never given back - it's now just a piece of the application
infrastructure until some other application activity schedules away from
that thread (eg: by ContinueWith or some other async operation in the
application code that releases the thread). To be fair, beyond structures
like ContinueWith(), it is not obvious how that continuation thread should
be handed back. This will be the same behaviour for dedicated continuation
pools, but with far less risk in the absence of ContinueWith() constructs.

In the .Net world this is becoming more of an issue as fewer .Net use cases
outside of UI bother with synchronization contexts by default.

Raymond.


On Thu, Mar 18, 2021 at 9:56 AM Valentin Kulichenko <
valentin.kuliche...@gmail.com> wrote:

> Hi Denis,
>
> I think Pavel's main point is that behavior is unpredictable. For example,
> AFAIK, putAsync can be executed in the main thread instead of the striped
> pool thread if the operation is local. The listener can also be executed in
> the main thread - this happens if the future is completed prior to listener
> invocation (this is actually quite possible in the unit test environment
> causing the test to pass). Finally, I'm pretty sure there are many cases
> when a deadlock does not occur right away, but instead it will reveal
> itself under high load due to thread starvation. The latter type of issues
> is the most dangerous because they are often reproduced only in production.
> Finally, there are performance considerations as well - cache operations
> and listeners share the same fixed-sized pool which can have negative
> effects.
>
> I'm OK with the change. Although, it might be better to introduce a new
> fixed-sized pool instead of ForkJoinPool for listeners, simply because this
> is the approach taken throughout the project. But this is up to a debate.
>
> -Val
>
> On Wed, Mar 17, 2021 at 11:31 AM Denis Garus  wrote:
>
> > Pavel,
> > I tried this:
> >
> > @Test
> > public void test() throws Exception {
> > IgniteCache cache =
> > startGrid().getOrCreateCache("test_cache");
> >
> > cache.putAsync(1, "one").listen(f -> cache.replace(1, "two"));
> >
> > assertEquals("two", cache.get(1));
> > }
> >
> > and this test is green.
> > I believe that an user can make listener that leads to deadlock, but
> > the example in the IEP does not reflect this.
> >
> >
> >
> > ср, 17 мар. 2021 г. в 17:36, Вячеслав Коптилин  >:
> >
> > > Hi Pavel,
> > >
> > > > Not a good excuse really. We have a usability problem, you have to
> > admit
> > > it.
> > > Fair enough. I agree that this is a usability issue, but I have doubts
> > that
> > > the proposed approach to overcome it is the best one.
> > >
> > > > Documentation won't help - no one is going to read the Javadoc for a
> > > trivial method like putAsync
> > > That is sad... However, I don't think that this is a strong argument
> > here.
> > >
> > > This is just my opinion. Let's see what other community members have to
> > > say.
> > >
> > > Thanks,
> > > S.
> > >
> > >
> > > ср, 17 мар. 2021 г. в 17:01, Pavel Tupitsyn :
> > >
> > > > > the user should use the right API
> > > >
> > > > Not a good excuse really. We have a usability problem, you have to
> > admit
> > > > it.
> > > > "The brakes did not work on your car - too bad, you should have known
> > > that
> > > > on Sundays only your left foot is allowed on the pedal"
> > > >
> > > > This particular use case is too intricate.
> > > > Even when you know about that, it is difficult to decide what can run
> > on
> > > > the striped pool,
> > > > and what can't. It is too easy to forget.
> > > > And most people don't know, even among Ignite developers.
> > > >
> > > > Documentation won't help - no one is going to read the Javadoc for a
> > > > trivial method like putAsync.
> > > >
> > > >
> > > > So I propose to have a safe default.
> > > > Then document the performance tuning opportunity on [1].
> > > >
> > > > Think about how many users abandon a product because it mysteriously
> > > > crashes and hangs.
> > > >
> > > > [1]
> > > >
> > > >
> > >
> >
> 

Re: IEP-70: Async Continuation Executor

[Valentin Kulichenko]

Hi Denis,

I think Pavel's main point is that behavior is unpredictable. For example,
AFAIK, putAsync can be executed in the main thread instead of the striped
pool thread if the operation is local. The listener can also be executed in
the main thread - this happens if the future is completed prior to listener
invocation (this is actually quite possible in the unit test environment
causing the test to pass). Finally, I'm pretty sure there are many cases
when a deadlock does not occur right away, but instead it will reveal
itself under high load due to thread starvation. The latter type of issues
is the most dangerous because they are often reproduced only in production.
Finally, there are performance considerations as well - cache operations
and listeners share the same fixed-sized pool which can have negative
effects.

I'm OK with the change. Although, it might be better to introduce a new
fixed-sized pool instead of ForkJoinPool for listeners, simply because this
is the approach taken throughout the project. But this is up to a debate.

-Val

On Wed, Mar 17, 2021 at 11:31 AM Denis Garus  wrote:

> Pavel,
> I tried this:
>
> @Test
> public void test() throws Exception {
> IgniteCache cache =
> startGrid().getOrCreateCache("test_cache");
>
> cache.putAsync(1, "one").listen(f -> cache.replace(1, "two"));
>
> assertEquals("two", cache.get(1));
> }
>
> and this test is green.
> I believe that an user can make listener that leads to deadlock, but
> the example in the IEP does not reflect this.
>
>
>
> ср, 17 мар. 2021 г. в 17:36, Вячеслав Коптилин :
>
> > Hi Pavel,
> >
> > > Not a good excuse really. We have a usability problem, you have to
> admit
> > it.
> > Fair enough. I agree that this is a usability issue, but I have doubts
> that
> > the proposed approach to overcome it is the best one.
> >
> > > Documentation won't help - no one is going to read the Javadoc for a
> > trivial method like putAsync
> > That is sad... However, I don't think that this is a strong argument
> here.
> >
> > This is just my opinion. Let's see what other community members have to
> > say.
> >
> > Thanks,
> > S.
> >
> >
> > ср, 17 мар. 2021 г. в 17:01, Pavel Tupitsyn :
> >
> > > > the user should use the right API
> > >
> > > Not a good excuse really. We have a usability problem, you have to
> admit
> > > it.
> > > "The brakes did not work on your car - too bad, you should have known
> > that
> > > on Sundays only your left foot is allowed on the pedal"
> > >
> > > This particular use case is too intricate.
> > > Even when you know about that, it is difficult to decide what can run
> on
> > > the striped pool,
> > > and what can't. It is too easy to forget.
> > > And most people don't know, even among Ignite developers.
> > >
> > > Documentation won't help - no one is going to read the Javadoc for a
> > > trivial method like putAsync.
> > >
> > >
> > > So I propose to have a safe default.
> > > Then document the performance tuning opportunity on [1].
> > >
> > > Think about how many users abandon a product because it mysteriously
> > > crashes and hangs.
> > >
> > > [1]
> > >
> > >
> >
> https://ignite.apache.org/docs/latest/perf-and-troubleshooting/general-perf-tips
> > >
> > >
> > > On Wed, Mar 17, 2021 at 4:21 PM Вячеслав Коптилин <
> > > slava.kopti...@gmail.com>
> > > wrote:
> > >
> > > > Hi Pavel,
> > > >
> > > > Well, I think that the user should use the right API instead of
> > > introducing
> > > > uncontested overhead for everyone.
> > > > For instance, the code that is provided by IEP can changed as
> follows:
> > > >
> > > > IgniteFuture fut = cache.putAsync(1, 1);
> > > > fut.listenAync(f -> {
> > > > // Executes on Striped pool and deadlocks.
> > > > cache.replace(1, 2);
> > > > }, ForkJoinPool.commonPool());
> > > >
> > > > Of course, it does not mean that this fact should not be properly
> > > > documented.
> > > > Perhaps, I am missing something.
> > > >
> > > > Thanks,
> > > > S.
> > > >
> > > > ср, 17 мар. 2021 г. в 16:01, Pavel Tupitsyn :
> > > >
> > > > > Slava,
> > > > >
> > > > > Your suggestion is to keep things as is and discard the IEP, right?
> > > > >
> > > > > >  this can lead to significant overhead
> > > > > Yes, there is some overhead, but the cost of accidentally starving
> > the
> > > > > striped pool is worse,
> > > > > not to mention the deadlocks.
> > > > >
> > > > > I believe that we should favor correctness over performance in any
> > > case.
> > > > >
> > > > >
> > > > > On Wed, Mar 17, 2021 at 3:34 PM Вячеслав Коптилин <
> > > > > slava.kopti...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Well, the specified method already exists :)
> > > > > >
> > > > > > /**
> > > > > >  * Registers listener closure to be asynchronously notified
> > > > whenever
> > > > > > future completes.
> > > > > >  * Closure will be processed in specified executor.
> > > > > >  *
> > > > > >  * @param lsnr Listener closure to register. Cannot be {@code
> > > > null}.
> 

Re: IEP-70: Async Continuation Executor

[Denis Garus]

Pavel,
I tried this:

@Test
public void test() throws Exception {
IgniteCache cache =
startGrid().getOrCreateCache("test_cache");

cache.putAsync(1, "one").listen(f -> cache.replace(1, "two"));

assertEquals("two", cache.get(1));
}

and this test is green.
I believe that an user can make listener that leads to deadlock, but
the example in the IEP does not reflect this.



ср, 17 мар. 2021 г. в 17:36, Вячеслав Коптилин :

> Hi Pavel,
>
> > Not a good excuse really. We have a usability problem, you have to admit
> it.
> Fair enough. I agree that this is a usability issue, but I have doubts that
> the proposed approach to overcome it is the best one.
>
> > Documentation won't help - no one is going to read the Javadoc for a
> trivial method like putAsync
> That is sad... However, I don't think that this is a strong argument here.
>
> This is just my opinion. Let's see what other community members have to
> say.
>
> Thanks,
> S.
>
>
> ср, 17 мар. 2021 г. в 17:01, Pavel Tupitsyn :
>
> > > the user should use the right API
> >
> > Not a good excuse really. We have a usability problem, you have to admit
> > it.
> > "The brakes did not work on your car - too bad, you should have known
> that
> > on Sundays only your left foot is allowed on the pedal"
> >
> > This particular use case is too intricate.
> > Even when you know about that, it is difficult to decide what can run on
> > the striped pool,
> > and what can't. It is too easy to forget.
> > And most people don't know, even among Ignite developers.
> >
> > Documentation won't help - no one is going to read the Javadoc for a
> > trivial method like putAsync.
> >
> >
> > So I propose to have a safe default.
> > Then document the performance tuning opportunity on [1].
> >
> > Think about how many users abandon a product because it mysteriously
> > crashes and hangs.
> >
> > [1]
> >
> >
> https://ignite.apache.org/docs/latest/perf-and-troubleshooting/general-perf-tips
> >
> >
> > On Wed, Mar 17, 2021 at 4:21 PM Вячеслав Коптилин <
> > slava.kopti...@gmail.com>
> > wrote:
> >
> > > Hi Pavel,
> > >
> > > Well, I think that the user should use the right API instead of
> > introducing
> > > uncontested overhead for everyone.
> > > For instance, the code that is provided by IEP can changed as follows:
> > >
> > > IgniteFuture fut = cache.putAsync(1, 1);
> > > fut.listenAync(f -> {
> > > // Executes on Striped pool and deadlocks.
> > > cache.replace(1, 2);
> > > }, ForkJoinPool.commonPool());
> > >
> > > Of course, it does not mean that this fact should not be properly
> > > documented.
> > > Perhaps, I am missing something.
> > >
> > > Thanks,
> > > S.
> > >
> > > ср, 17 мар. 2021 г. в 16:01, Pavel Tupitsyn :
> > >
> > > > Slava,
> > > >
> > > > Your suggestion is to keep things as is and discard the IEP, right?
> > > >
> > > > >  this can lead to significant overhead
> > > > Yes, there is some overhead, but the cost of accidentally starving
> the
> > > > striped pool is worse,
> > > > not to mention the deadlocks.
> > > >
> > > > I believe that we should favor correctness over performance in any
> > case.
> > > >
> > > >
> > > > On Wed, Mar 17, 2021 at 3:34 PM Вячеслав Коптилин <
> > > > slava.kopti...@gmail.com>
> > > > wrote:
> > > >
> > > > > Well, the specified method already exists :)
> > > > >
> > > > > /**
> > > > >  * Registers listener closure to be asynchronously notified
> > > whenever
> > > > > future completes.
> > > > >  * Closure will be processed in specified executor.
> > > > >  *
> > > > >  * @param lsnr Listener closure to register. Cannot be {@code
> > > null}.
> > > > >  * @param exec Executor to run listener. Cannot be {@code
> null}.
> > > > >  */
> > > > > public void listenAsync(IgniteInClosure IgniteFuture>
> > > > lsnr,
> > > > > Executor exec);
> > > > >
> > > > > Thanks,
> > > > > S.
> > > > >
> > > > > ср, 17 мар. 2021 г. в 15:25, Вячеслав Коптилин <
> > > slava.kopti...@gmail.com
> > > > >:
> > > > >
> > > > > > Hello Pavel,
> > > > > >
> > > > > > I took a look at your IEP and pool request. I have the following
> > > > > concerns.
> > > > > > First of all, this change breaks the contract of
> > > > > IgniteFuture#listen(lsnr)
> > > > > >
> > > > > > /**
> > > > > >  * Registers listener closure to be asynchronously notified
> > > > whenever
> > > > > > future completes.
> > > > > >  * Closure will be processed in thread that completes this
> > future
> > > > or
> > > > > > (if future already
> > > > > >  * completed) immediately in current thread.
> > > > > >  *
> > > > > >  * @param lsnr Listener closure to register. Cannot be {@code
> > > > null}.
> > > > > >  */
> > > > > > public void listen(IgniteInClosure>
> > > lsnr);
> > > > > >
> > > > > > In your pull request, the listener is always called from a
> > > > specified
> > > > > > thread pool (which is fork-join by default)
> > > > > > even though the future is already completed at the 

Re: IEP-70: Async Continuation Executor

[Вячеслав Коптилин]

Hi Pavel,

> Not a good excuse really. We have a usability problem, you have to admit
it.
Fair enough. I agree that this is a usability issue, but I have doubts that
the proposed approach to overcome it is the best one.

> Documentation won't help - no one is going to read the Javadoc for a
trivial method like putAsync
That is sad... However, I don't think that this is a strong argument here.

This is just my opinion. Let's see what other community members have to say.

Thanks,
S.


ср, 17 мар. 2021 г. в 17:01, Pavel Tupitsyn :

> > the user should use the right API
>
> Not a good excuse really. We have a usability problem, you have to admit
> it.
> "The brakes did not work on your car - too bad, you should have known that
> on Sundays only your left foot is allowed on the pedal"
>
> This particular use case is too intricate.
> Even when you know about that, it is difficult to decide what can run on
> the striped pool,
> and what can't. It is too easy to forget.
> And most people don't know, even among Ignite developers.
>
> Documentation won't help - no one is going to read the Javadoc for a
> trivial method like putAsync.
>
>
> So I propose to have a safe default.
> Then document the performance tuning opportunity on [1].
>
> Think about how many users abandon a product because it mysteriously
> crashes and hangs.
>
> [1]
>
> https://ignite.apache.org/docs/latest/perf-and-troubleshooting/general-perf-tips
>
>
> On Wed, Mar 17, 2021 at 4:21 PM Вячеслав Коптилин <
> slava.kopti...@gmail.com>
> wrote:
>
> > Hi Pavel,
> >
> > Well, I think that the user should use the right API instead of
> introducing
> > uncontested overhead for everyone.
> > For instance, the code that is provided by IEP can changed as follows:
> >
> > IgniteFuture fut = cache.putAsync(1, 1);
> > fut.listenAync(f -> {
> > // Executes on Striped pool and deadlocks.
> > cache.replace(1, 2);
> > }, ForkJoinPool.commonPool());
> >
> > Of course, it does not mean that this fact should not be properly
> > documented.
> > Perhaps, I am missing something.
> >
> > Thanks,
> > S.
> >
> > ср, 17 мар. 2021 г. в 16:01, Pavel Tupitsyn :
> >
> > > Slava,
> > >
> > > Your suggestion is to keep things as is and discard the IEP, right?
> > >
> > > >  this can lead to significant overhead
> > > Yes, there is some overhead, but the cost of accidentally starving the
> > > striped pool is worse,
> > > not to mention the deadlocks.
> > >
> > > I believe that we should favor correctness over performance in any
> case.
> > >
> > >
> > > On Wed, Mar 17, 2021 at 3:34 PM Вячеслав Коптилин <
> > > slava.kopti...@gmail.com>
> > > wrote:
> > >
> > > > Well, the specified method already exists :)
> > > >
> > > > /**
> > > >  * Registers listener closure to be asynchronously notified
> > whenever
> > > > future completes.
> > > >  * Closure will be processed in specified executor.
> > > >  *
> > > >  * @param lsnr Listener closure to register. Cannot be {@code
> > null}.
> > > >  * @param exec Executor to run listener. Cannot be {@code null}.
> > > >  */
> > > > public void listenAsync(IgniteInClosure>
> > > lsnr,
> > > > Executor exec);
> > > >
> > > > Thanks,
> > > > S.
> > > >
> > > > ср, 17 мар. 2021 г. в 15:25, Вячеслав Коптилин <
> > slava.kopti...@gmail.com
> > > >:
> > > >
> > > > > Hello Pavel,
> > > > >
> > > > > I took a look at your IEP and pool request. I have the following
> > > > concerns.
> > > > > First of all, this change breaks the contract of
> > > > IgniteFuture#listen(lsnr)
> > > > >
> > > > > /**
> > > > >  * Registers listener closure to be asynchronously notified
> > > whenever
> > > > > future completes.
> > > > >  * Closure will be processed in thread that completes this
> future
> > > or
> > > > > (if future already
> > > > >  * completed) immediately in current thread.
> > > > >  *
> > > > >  * @param lsnr Listener closure to register. Cannot be {@code
> > > null}.
> > > > >  */
> > > > > public void listen(IgniteInClosure>
> > lsnr);
> > > > >
> > > > > In your pull request, the listener is always called from a
> > > specified
> > > > > thread pool (which is fork-join by default)
> > > > > even though the future is already completed at the moment the
> > > listen
> > > > > method is called.
> > > > > In my opinion, this can lead to significant overhead -
> submission
> > > > > requires acquiring a lock and notifying a pool thread.
> > > > >
> > > > > It seems to me, that we should not change the current behavior.
> > > > > However, thread pool executor can be added as an optional parameter
> > of
> > > > > listen() method as follows:
> > > > >
> > > > > public void listen(IgniteInClosure>
> > > lsnr,
> > > > > Executor exec);
> > > > >
> > > > > Thanks,
> > > > > S.
> > > > >
> > > > > пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn  >:
> > > > >
> > > > >> Igniters,
> > > > >>
> > > > >> Please review the IEP [1] and let me know your thoughts.
> > > 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

> the user should use the right API

Not a good excuse really. We have a usability problem, you have to admit it.
"The brakes did not work on your car - too bad, you should have known that
on Sundays only your left foot is allowed on the pedal"

This particular use case is too intricate.
Even when you know about that, it is difficult to decide what can run on
the striped pool,
and what can't. It is too easy to forget.
And most people don't know, even among Ignite developers.

Documentation won't help - no one is going to read the Javadoc for a
trivial method like putAsync.


So I propose to have a safe default.
Then document the performance tuning opportunity on [1].

Think about how many users abandon a product because it mysteriously
crashes and hangs.

[1]
https://ignite.apache.org/docs/latest/perf-and-troubleshooting/general-perf-tips


On Wed, Mar 17, 2021 at 4:21 PM Вячеслав Коптилин 
wrote:

> Hi Pavel,
>
> Well, I think that the user should use the right API instead of introducing
> uncontested overhead for everyone.
> For instance, the code that is provided by IEP can changed as follows:
>
> IgniteFuture fut = cache.putAsync(1, 1);
> fut.listenAync(f -> {
> // Executes on Striped pool and deadlocks.
> cache.replace(1, 2);
> }, ForkJoinPool.commonPool());
>
> Of course, it does not mean that this fact should not be properly
> documented.
> Perhaps, I am missing something.
>
> Thanks,
> S.
>
> ср, 17 мар. 2021 г. в 16:01, Pavel Tupitsyn :
>
> > Slava,
> >
> > Your suggestion is to keep things as is and discard the IEP, right?
> >
> > >  this can lead to significant overhead
> > Yes, there is some overhead, but the cost of accidentally starving the
> > striped pool is worse,
> > not to mention the deadlocks.
> >
> > I believe that we should favor correctness over performance in any case.
> >
> >
> > On Wed, Mar 17, 2021 at 3:34 PM Вячеслав Коптилин <
> > slava.kopti...@gmail.com>
> > wrote:
> >
> > > Well, the specified method already exists :)
> > >
> > > /**
> > >  * Registers listener closure to be asynchronously notified
> whenever
> > > future completes.
> > >  * Closure will be processed in specified executor.
> > >  *
> > >  * @param lsnr Listener closure to register. Cannot be {@code
> null}.
> > >  * @param exec Executor to run listener. Cannot be {@code null}.
> > >  */
> > > public void listenAsync(IgniteInClosure>
> > lsnr,
> > > Executor exec);
> > >
> > > Thanks,
> > > S.
> > >
> > > ср, 17 мар. 2021 г. в 15:25, Вячеслав Коптилин <
> slava.kopti...@gmail.com
> > >:
> > >
> > > > Hello Pavel,
> > > >
> > > > I took a look at your IEP and pool request. I have the following
> > > concerns.
> > > > First of all, this change breaks the contract of
> > > IgniteFuture#listen(lsnr)
> > > >
> > > > /**
> > > >  * Registers listener closure to be asynchronously notified
> > whenever
> > > > future completes.
> > > >  * Closure will be processed in thread that completes this future
> > or
> > > > (if future already
> > > >  * completed) immediately in current thread.
> > > >  *
> > > >  * @param lsnr Listener closure to register. Cannot be {@code
> > null}.
> > > >  */
> > > > public void listen(IgniteInClosure>
> lsnr);
> > > >
> > > > In your pull request, the listener is always called from a
> > specified
> > > > thread pool (which is fork-join by default)
> > > > even though the future is already completed at the moment the
> > listen
> > > > method is called.
> > > > In my opinion, this can lead to significant overhead - submission
> > > > requires acquiring a lock and notifying a pool thread.
> > > >
> > > > It seems to me, that we should not change the current behavior.
> > > > However, thread pool executor can be added as an optional parameter
> of
> > > > listen() method as follows:
> > > >
> > > > public void listen(IgniteInClosure>
> > lsnr,
> > > > Executor exec);
> > > >
> > > > Thanks,
> > > > S.
> > > >
> > > > пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :
> > > >
> > > >> Igniters,
> > > >>
> > > >> Please review the IEP [1] and let me know your thoughts.
> > > >>
> > > >> [1]
> > > >>
> > > >>
> > >
> >
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
> > > >>
> > > >
> > >
> >
>

Re: IEP-70: Async Continuation Executor

[Вячеслав Коптилин]

Hi Pavel,

Well, I think that the user should use the right API instead of introducing
uncontested overhead for everyone.
For instance, the code that is provided by IEP can changed as follows:

IgniteFuture fut = cache.putAsync(1, 1);
fut.listenAync(f -> {
// Executes on Striped pool and deadlocks.
cache.replace(1, 2);
}, ForkJoinPool.commonPool());

Of course, it does not mean that this fact should not be properly
documented.
Perhaps, I am missing something.

Thanks,
S.

ср, 17 мар. 2021 г. в 16:01, Pavel Tupitsyn :

> Slava,
>
> Your suggestion is to keep things as is and discard the IEP, right?
>
> >  this can lead to significant overhead
> Yes, there is some overhead, but the cost of accidentally starving the
> striped pool is worse,
> not to mention the deadlocks.
>
> I believe that we should favor correctness over performance in any case.
>
>
> On Wed, Mar 17, 2021 at 3:34 PM Вячеслав Коптилин <
> slava.kopti...@gmail.com>
> wrote:
>
> > Well, the specified method already exists :)
> >
> > /**
> >  * Registers listener closure to be asynchronously notified whenever
> > future completes.
> >  * Closure will be processed in specified executor.
> >  *
> >  * @param lsnr Listener closure to register. Cannot be {@code null}.
> >  * @param exec Executor to run listener. Cannot be {@code null}.
> >  */
> > public void listenAsync(IgniteInClosure>
> lsnr,
> > Executor exec);
> >
> > Thanks,
> > S.
> >
> > ср, 17 мар. 2021 г. в 15:25, Вячеслав Коптилин  >:
> >
> > > Hello Pavel,
> > >
> > > I took a look at your IEP and pool request. I have the following
> > concerns.
> > > First of all, this change breaks the contract of
> > IgniteFuture#listen(lsnr)
> > >
> > > /**
> > >  * Registers listener closure to be asynchronously notified
> whenever
> > > future completes.
> > >  * Closure will be processed in thread that completes this future
> or
> > > (if future already
> > >  * completed) immediately in current thread.
> > >  *
> > >  * @param lsnr Listener closure to register. Cannot be {@code
> null}.
> > >  */
> > > public void listen(IgniteInClosure> lsnr);
> > >
> > > In your pull request, the listener is always called from a
> specified
> > > thread pool (which is fork-join by default)
> > > even though the future is already completed at the moment the
> listen
> > > method is called.
> > > In my opinion, this can lead to significant overhead - submission
> > > requires acquiring a lock and notifying a pool thread.
> > >
> > > It seems to me, that we should not change the current behavior.
> > > However, thread pool executor can be added as an optional parameter of
> > > listen() method as follows:
> > >
> > > public void listen(IgniteInClosure>
> lsnr,
> > > Executor exec);
> > >
> > > Thanks,
> > > S.
> > >
> > > пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :
> > >
> > >> Igniters,
> > >>
> > >> Please review the IEP [1] and let me know your thoughts.
> > >>
> > >> [1]
> > >>
> > >>
> >
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
> > >>
> > >
> >
>

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Slava,

Your suggestion is to keep things as is and discard the IEP, right?

>  this can lead to significant overhead
Yes, there is some overhead, but the cost of accidentally starving the
striped pool is worse,
not to mention the deadlocks.

I believe that we should favor correctness over performance in any case.


On Wed, Mar 17, 2021 at 3:34 PM Вячеслав Коптилин 
wrote:

> Well, the specified method already exists :)
>
> /**
>  * Registers listener closure to be asynchronously notified whenever
> future completes.
>  * Closure will be processed in specified executor.
>  *
>  * @param lsnr Listener closure to register. Cannot be {@code null}.
>  * @param exec Executor to run listener. Cannot be {@code null}.
>  */
> public void listenAsync(IgniteInClosure> lsnr,
> Executor exec);
>
> Thanks,
> S.
>
> ср, 17 мар. 2021 г. в 15:25, Вячеслав Коптилин :
>
> > Hello Pavel,
> >
> > I took a look at your IEP and pool request. I have the following
> concerns.
> > First of all, this change breaks the contract of
> IgniteFuture#listen(lsnr)
> >
> > /**
> >  * Registers listener closure to be asynchronously notified whenever
> > future completes.
> >  * Closure will be processed in thread that completes this future or
> > (if future already
> >  * completed) immediately in current thread.
> >  *
> >  * @param lsnr Listener closure to register. Cannot be {@code null}.
> >  */
> > public void listen(IgniteInClosure> lsnr);
> >
> > In your pull request, the listener is always called from a specified
> > thread pool (which is fork-join by default)
> > even though the future is already completed at the moment the listen
> > method is called.
> > In my opinion, this can lead to significant overhead - submission
> > requires acquiring a lock and notifying a pool thread.
> >
> > It seems to me, that we should not change the current behavior.
> > However, thread pool executor can be added as an optional parameter of
> > listen() method as follows:
> >
> > public void listen(IgniteInClosure> lsnr,
> > Executor exec);
> >
> > Thanks,
> > S.
> >
> > пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :
> >
> >> Igniters,
> >>
> >> Please review the IEP [1] and let me know your thoughts.
> >>
> >> [1]
> >>
> >>
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
> >>
> >
>

Re: IEP-70: Async Continuation Executor

[Вячеслав Коптилин]

Well, the specified method already exists :)

/**
 * Registers listener closure to be asynchronously notified whenever
future completes.
 * Closure will be processed in specified executor.
 *
 * @param lsnr Listener closure to register. Cannot be {@code null}.
 * @param exec Executor to run listener. Cannot be {@code null}.
 */
public void listenAsync(IgniteInClosure> lsnr,
Executor exec);

Thanks,
S.

ср, 17 мар. 2021 г. в 15:25, Вячеслав Коптилин :

> Hello Pavel,
>
> I took a look at your IEP and pool request. I have the following concerns.
> First of all, this change breaks the contract of IgniteFuture#listen(lsnr)
>
> /**
>  * Registers listener closure to be asynchronously notified whenever
> future completes.
>  * Closure will be processed in thread that completes this future or
> (if future already
>  * completed) immediately in current thread.
>  *
>  * @param lsnr Listener closure to register. Cannot be {@code null}.
>  */
> public void listen(IgniteInClosure> lsnr);
>
> In your pull request, the listener is always called from a specified
> thread pool (which is fork-join by default)
> even though the future is already completed at the moment the listen
> method is called.
> In my opinion, this can lead to significant overhead - submission
> requires acquiring a lock and notifying a pool thread.
>
> It seems to me, that we should not change the current behavior.
> However, thread pool executor can be added as an optional parameter of
> listen() method as follows:
>
> public void listen(IgniteInClosure> lsnr,
> Executor exec);
>
> Thanks,
> S.
>
> пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :
>
>> Igniters,
>>
>> Please review the IEP [1] and let me know your thoughts.
>>
>> [1]
>>
>> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
>>
>

Re: IEP-70: Async Continuation Executor

[Denis Garus]

Pavel, I got it.
Thank you.

ср, 17 мар. 2021 г. в 13:11, Pavel Tupitsyn :

> Denis,
>
> Yes, I've updated the Motivation section, it really was a bit vague - thank
> you.
>
> > why you suggest using ForkJoinPool.commonPool as the default pool to run
> listeners
>
> - It is recommended to be used by default [1]
> - There are no alternatives?
>
> [1]
>
> https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ForkJoinPool.html
>
> On Wed, Mar 17, 2021 at 1:02 PM Denis Garus  wrote:
>
> > Pavel, thank you for your answer.
> >
> > Sorry, the previous version of IEP motivation highlighted what could be
> > wrong with user listeners,
> > so I thought that is the main problem.
> >
> > I'm wondering why you suggest using ForkJoinPool.commonPool as the
> default
> > pool to run listeners?
> >
> > ср, 17 мар. 2021 г. в 11:51, Alexey Kukushkin  >:
> >
> > > My initial confusion was due to I did not know the
> > > TaskCompletionSource.SetResult() internally handles
> > > SynchronizationContext captured before the async operation. I thought
> we
> > > would have to do it in Ignite. Now I know that and have no problems
> with
> > > the IEP.
> > >
> > > ср, 17 мар. 2021 г. в 11:18, Pavel Tupitsyn :
> > >
> > > > Denis,
> > > >
> > > > > we don't try to solve the problem mentioned in the IEP
> > > >
> > > > The problem: user code executes on striped pool (which causes issues)
> > > > The solution: don't execute user code on striped pool
> > > >
> > > > I believe this solves the problem and removes any and all
> restrictions
> > > > for async listeners/continuations. Am I missing something else?
> > > >
> > > > On Wed, Mar 17, 2021 at 10:16 AM Denis Garus 
> > > wrote:
> > > >
> > > > > Hello!
> > > > >
> > > > > As I understood, we don't try to solve the problem mentioned in the
> > > IEP:
> > > > > there is unexpected behavior,
> > > > > users should carefully read the docs to know about this, and so on.
> > > > > A thread that executes an incorrect listener hung in any case,
> > > > > and the suggested solution is to change the pool which owns this
> > > thread.
> > > > > Did you consider an option to execute user-defined listeners with a
> > > > > timeout?
> > > > > I think that implicit using java pools to run a code that can be
> hung
> > > is
> > > > a
> > > > > questionable solution.
> > > > >
> > > > > вт, 16 мар. 2021 г. в 23:30, Alexey Kukushkin <
> > > kukushkinale...@gmail.com
> > > > >:
> > > > >
> > > > > > Pavel,
> > > > > >
> > > > > > So what you are saying is submitting a continuation to .NET
> Thread
> > > Pool
> > > > > > already respects current  SynchronizationContext and
> > ConfigureAwait.
> > > In
> > > > > > this case the IEP looks complete (for some reason I thought that
> we
> > > > > should
> > > > > > take care about the SynchronizationContext inside the Ignite.NET
> > > > > > implementation).
> > > > > >
> > > > > > --
> > > > > > Best regards,
> > > > > > Alexey
> > > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best regards,
> > > Alexey
> > >
> >
>

Re: IEP-70: Async Continuation Executor

[Вячеслав Коптилин]

Hello Pavel,

I took a look at your IEP and pool request. I have the following concerns.
First of all, this change breaks the contract of IgniteFuture#listen(lsnr)

/**
 * Registers listener closure to be asynchronously notified whenever
future completes.
 * Closure will be processed in thread that completes this future or
(if future already
 * completed) immediately in current thread.
 *
 * @param lsnr Listener closure to register. Cannot be {@code null}.
 */
public void listen(IgniteInClosure> lsnr);

In your pull request, the listener is always called from a specified
thread pool (which is fork-join by default)
even though the future is already completed at the moment the listen
method is called.
In my opinion, this can lead to significant overhead - submission
requires acquiring a lock and notifying a pool thread.

It seems to me, that we should not change the current behavior.
However, thread pool executor can be added as an optional parameter of
listen() method as follows:

public void listen(IgniteInClosure> lsnr,
Executor exec);

Thanks,
S.

пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :

> Igniters,
>
> Please review the IEP [1] and let me know your thoughts.
>
> [1]
>
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
>

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Denis,

Yes, I've updated the Motivation section, it really was a bit vague - thank
you.

> why you suggest using ForkJoinPool.commonPool as the default pool to run
listeners

- It is recommended to be used by default [1]
- There are no alternatives?

[1]
https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ForkJoinPool.html

On Wed, Mar 17, 2021 at 1:02 PM Denis Garus  wrote:

> Pavel, thank you for your answer.
>
> Sorry, the previous version of IEP motivation highlighted what could be
> wrong with user listeners,
> so I thought that is the main problem.
>
> I'm wondering why you suggest using ForkJoinPool.commonPool as the default
> pool to run listeners?
>
> ср, 17 мар. 2021 г. в 11:51, Alexey Kukushkin :
>
> > My initial confusion was due to I did not know the
> > TaskCompletionSource.SetResult() internally handles
> > SynchronizationContext captured before the async operation. I thought we
> > would have to do it in Ignite. Now I know that and have no problems with
> > the IEP.
> >
> > ср, 17 мар. 2021 г. в 11:18, Pavel Tupitsyn :
> >
> > > Denis,
> > >
> > > > we don't try to solve the problem mentioned in the IEP
> > >
> > > The problem: user code executes on striped pool (which causes issues)
> > > The solution: don't execute user code on striped pool
> > >
> > > I believe this solves the problem and removes any and all restrictions
> > > for async listeners/continuations. Am I missing something else?
> > >
> > > On Wed, Mar 17, 2021 at 10:16 AM Denis Garus 
> > wrote:
> > >
> > > > Hello!
> > > >
> > > > As I understood, we don't try to solve the problem mentioned in the
> > IEP:
> > > > there is unexpected behavior,
> > > > users should carefully read the docs to know about this, and so on.
> > > > A thread that executes an incorrect listener hung in any case,
> > > > and the suggested solution is to change the pool which owns this
> > thread.
> > > > Did you consider an option to execute user-defined listeners with a
> > > > timeout?
> > > > I think that implicit using java pools to run a code that can be hung
> > is
> > > a
> > > > questionable solution.
> > > >
> > > > вт, 16 мар. 2021 г. в 23:30, Alexey Kukushkin <
> > kukushkinale...@gmail.com
> > > >:
> > > >
> > > > > Pavel,
> > > > >
> > > > > So what you are saying is submitting a continuation to .NET Thread
> > Pool
> > > > > already respects current  SynchronizationContext and
> ConfigureAwait.
> > In
> > > > > this case the IEP looks complete (for some reason I thought that we
> > > > should
> > > > > take care about the SynchronizationContext inside the Ignite.NET
> > > > > implementation).
> > > > >
> > > > > --
> > > > > Best regards,
> > > > > Alexey
> > > > >
> > > >
> > >
> >
> >
> > --
> > Best regards,
> > Alexey
> >
>

Re: IEP-70: Async Continuation Executor

[Denis Garus]

Pavel, thank you for your answer.

Sorry, the previous version of IEP motivation highlighted what could be
wrong with user listeners,
so I thought that is the main problem.

I'm wondering why you suggest using ForkJoinPool.commonPool as the default
pool to run listeners?

ср, 17 мар. 2021 г. в 11:51, Alexey Kukushkin :

> My initial confusion was due to I did not know the
> TaskCompletionSource.SetResult() internally handles
> SynchronizationContext captured before the async operation. I thought we
> would have to do it in Ignite. Now I know that and have no problems with
> the IEP.
>
> ср, 17 мар. 2021 г. в 11:18, Pavel Tupitsyn :
>
> > Denis,
> >
> > > we don't try to solve the problem mentioned in the IEP
> >
> > The problem: user code executes on striped pool (which causes issues)
> > The solution: don't execute user code on striped pool
> >
> > I believe this solves the problem and removes any and all restrictions
> > for async listeners/continuations. Am I missing something else?
> >
> > On Wed, Mar 17, 2021 at 10:16 AM Denis Garus 
> wrote:
> >
> > > Hello!
> > >
> > > As I understood, we don't try to solve the problem mentioned in the
> IEP:
> > > there is unexpected behavior,
> > > users should carefully read the docs to know about this, and so on.
> > > A thread that executes an incorrect listener hung in any case,
> > > and the suggested solution is to change the pool which owns this
> thread.
> > > Did you consider an option to execute user-defined listeners with a
> > > timeout?
> > > I think that implicit using java pools to run a code that can be hung
> is
> > a
> > > questionable solution.
> > >
> > > вт, 16 мар. 2021 г. в 23:30, Alexey Kukushkin <
> kukushkinale...@gmail.com
> > >:
> > >
> > > > Pavel,
> > > >
> > > > So what you are saying is submitting a continuation to .NET Thread
> Pool
> > > > already respects current  SynchronizationContext and ConfigureAwait.
> In
> > > > this case the IEP looks complete (for some reason I thought that we
> > > should
> > > > take care about the SynchronizationContext inside the Ignite.NET
> > > > implementation).
> > > >
> > > > --
> > > > Best regards,
> > > > Alexey
> > > >
> > >
> >
>
>
> --
> Best regards,
> Alexey
>

Re: IEP-70: Async Continuation Executor

[Alexey Kukushkin]

My initial confusion was due to I did not know the
TaskCompletionSource.SetResult() internally handles
SynchronizationContext captured before the async operation. I thought we
would have to do it in Ignite. Now I know that and have no problems with
the IEP.

ср, 17 мар. 2021 г. в 11:18, Pavel Tupitsyn :

> Denis,
>
> > we don't try to solve the problem mentioned in the IEP
>
> The problem: user code executes on striped pool (which causes issues)
> The solution: don't execute user code on striped pool
>
> I believe this solves the problem and removes any and all restrictions
> for async listeners/continuations. Am I missing something else?
>
> On Wed, Mar 17, 2021 at 10:16 AM Denis Garus  wrote:
>
> > Hello!
> >
> > As I understood, we don't try to solve the problem mentioned in the IEP:
> > there is unexpected behavior,
> > users should carefully read the docs to know about this, and so on.
> > A thread that executes an incorrect listener hung in any case,
> > and the suggested solution is to change the pool which owns this thread.
> > Did you consider an option to execute user-defined listeners with a
> > timeout?
> > I think that implicit using java pools to run a code that can be hung is
> a
> > questionable solution.
> >
> > вт, 16 мар. 2021 г. в 23:30, Alexey Kukushkin  >:
> >
> > > Pavel,
> > >
> > > So what you are saying is submitting a continuation to .NET Thread Pool
> > > already respects current  SynchronizationContext and ConfigureAwait. In
> > > this case the IEP looks complete (for some reason I thought that we
> > should
> > > take care about the SynchronizationContext inside the Ignite.NET
> > > implementation).
> > >
> > > --
> > > Best regards,
> > > Alexey
> > >
> >
>


-- 
Best regards,
Alexey

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Denis,

> we don't try to solve the problem mentioned in the IEP

The problem: user code executes on striped pool (which causes issues)
The solution: don't execute user code on striped pool

I believe this solves the problem and removes any and all restrictions
for async listeners/continuations. Am I missing something else?

On Wed, Mar 17, 2021 at 10:16 AM Denis Garus  wrote:

> Hello!
>
> As I understood, we don't try to solve the problem mentioned in the IEP:
> there is unexpected behavior,
> users should carefully read the docs to know about this, and so on.
> A thread that executes an incorrect listener hung in any case,
> and the suggested solution is to change the pool which owns this thread.
> Did you consider an option to execute user-defined listeners with a
> timeout?
> I think that implicit using java pools to run a code that can be hung is a
> questionable solution.
>
> вт, 16 мар. 2021 г. в 23:30, Alexey Kukushkin :
>
> > Pavel,
> >
> > So what you are saying is submitting a continuation to .NET Thread Pool
> > already respects current  SynchronizationContext and ConfigureAwait. In
> > this case the IEP looks complete (for some reason I thought that we
> should
> > take care about the SynchronizationContext inside the Ignite.NET
> > implementation).
> >
> > --
> > Best regards,
> > Alexey
> >
>

Re: IEP-70: Async Continuation Executor

[Denis Garus]

Hello!

As I understood, we don't try to solve the problem mentioned in the IEP:
there is unexpected behavior,
users should carefully read the docs to know about this, and so on.
A thread that executes an incorrect listener hung in any case,
and the suggested solution is to change the pool which owns this thread.
Did you consider an option to execute user-defined listeners with a timeout?
I think that implicit using java pools to run a code that can be hung is a
questionable solution.

вт, 16 мар. 2021 г. в 23:30, Alexey Kukushkin :

> Pavel,
>
> So what you are saying is submitting a continuation to .NET Thread Pool
> already respects current  SynchronizationContext and ConfigureAwait. In
> this case the IEP looks complete (for some reason I thought that we should
> take care about the SynchronizationContext inside the Ignite.NET
> implementation).
>
> --
> Best regards,
> Alexey
>

Re: IEP-70: Async Continuation Executor

[Alexey Kukushkin]

Pavel,

So what you are saying is submitting a continuation to .NET Thread Pool
already respects current  SynchronizationContext and ConfigureAwait. In
this case the IEP looks complete (for some reason I thought that we should
take care about the SynchronizationContext inside the Ignite.NET
implementation).

-- 
Best regards,
Alexey

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Raymond,

> If it is present in memory, then a Sync operation will
> be more performant. Is it possible to do a two step operation like this
> 'Cache.GetIfInMemory() ?? await Cache.GetAsync()'

Ignite already does that - if the key is present locally (primary, backup,
near cache, platform cache),
the operation will be performed synchronously and you'll get a completed
Task back;
there won't be any thread jumps.

We'll make sure this does not change with the IEP - I'll add tests.

Having said that, you can do a two-step op:
cache.TryLocalPeek(1, out var v) ? v : await cache.GetAsync(1)

This performs better for local entries, because we avoid
async method overhead, and worse for non-local entries, because we do two
calls.

On Tue, Mar 16, 2021 at 10:58 PM Pavel Tupitsyn 
wrote:

> Alexey,
>
> I'm not sure your understanding is entirely correct.
> The proposal won't break or change anything for UI apps and other apps
> with SynchronizationContext.
> The code in Button1_Click works now and will work after the proposal:
> continuations are routed to SynchronizationContext automatically when it
> is present,
> we don't have to worry about that in our code [1].
>
> > Sometimes you know that your continuation is really fast and safe and you
> > want to avoid switching threads to improve performance
>
> This is a valid point, that's why the new behavior is configurable - safe
> by default,
> but can be overridden when you know what you are doing.
>
> > In this case you use ConfigureAwait(false)
>
> Not correct: ConfigureAwait() does nothing when there is no
> SyncronizationContext:
> in ASP.NET Core apps, in Console apps, basically everywhere except WPF,
> WinForms, and Legacy ASP.NET.
>
>
> [1] https://devblogs.microsoft.com/dotnet/configureawait-faq/
>
> On Tue, Mar 16, 2021 at 10:54 PM Alexey Kukushkin <
> kukushkinale...@gmail.com> wrote:
>
>> Raymond,
>>
>> The article you referenced
>>  is great! It
>> seems to me the article rather proved what I suggested: an async operation
>> implementation:
>>
>>- Uses the async operation thread (Ignite's thread) if ConfigureAwait
>> is
>>false (by default it is true)
>>- Uses caller's current SynchornizationContext if it is specified
>>- Uses caller's TaskScheduler.Current if current
>>SynchornizationContext is null
>>
>> In the application code (outside framework callbacks) the
>> SynchornizationContext.Current will be null and TaskScheduler.Current is
>> the .NET's fork-join thread pool. Thus, normally the .NET thread pool
>> would
>> be used for continuations as Pavel suggested in the IEP.
>>
>> Executing Async operation in a context where
>> SynchornizationContext.Current is not null means some framework explicitly
>> configured the context and that means it is important to execute the
>> continuation in that context (like UI or xUnit main thread)
>>
>> I agree that routing back to original context might result in waiting,
>> which is very dangerous for an Ignite thread. We can create a worker
>> thread
>> to route continuation to original context.
>>
>>
>> вт, 16 мар. 2021 г. в 21:40, Raymond Wilson :
>>
>> > There is a (long) discussion here (
>> > https://devblogs.microsoft.com/dotnet/configureawait-faq/) regarding
>> use
>> > of
>> > ConfigureAwait().
>> >
>> > Putting edge cases aside, there is a general use case for
>> > .ConfigureAwait(true) in application UI contexts to ensure the
>> continuation
>> > joins a UI thread (for example), where failing to do so results in an
>> > error.
>> >
>> > The other general use case relates more to library code where you are
>> > typically running your tasks on the managed thread pool (assuming no
>> custom
>> > task schedulers etc). In this case the .library is agnostic about which
>> > thread pool thread picks up the continuation, so forcing the
>> continuation
>> > to join to the original thread may be both a performance penalty from
>> the
>> > join overhead, but also may add latency waiting for that thread to
>> become
>> > available again.
>> >
>> > I don't have good insight into how the striped thread pool is used in
>> > Ignite, but in highly concurrent environments letting those threads
>> escape
>> > into the calling client context seems like a bad idea in general.
>> >
>> > Stepping back a little, the Cache Async operations are good for when
>> there
>> > will be an IO operation incurred because the requested element is not
>> > present in memory. If it is present in memory, then a Sync operation
>> will
>> > be more performant. Is it possible to do a two step operation like this
>> > 'Cache.GetIfInMemory() ?? await Cache.GetAsync()' to allow the calling
>> > context to optimise the calling model dynamically?
>> >
>> > Thanks,
>> > Raymond.
>> >
>> >
>> > On Wed, Mar 17, 2021 at 6:14 AM Alexey Kukushkin <
>> > kukushkinale...@gmail.com>
>> > wrote:
>> >
>> > > Pavel,
>> > >
>> > > My understanding might be wrong but I think the best 

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Alexey,

I'm not sure your understanding is entirely correct.
The proposal won't break or change anything for UI apps and other apps with
SynchronizationContext.
The code in Button1_Click works now and will work after the proposal:
continuations are routed to SynchronizationContext automatically when it is
present,
we don't have to worry about that in our code [1].

> Sometimes you know that your continuation is really fast and safe and you
> want to avoid switching threads to improve performance

This is a valid point, that's why the new behavior is configurable - safe
by default,
but can be overridden when you know what you are doing.

> In this case you use ConfigureAwait(false)

Not correct: ConfigureAwait() does nothing when there is no
SyncronizationContext:
in ASP.NET Core apps, in Console apps, basically everywhere except WPF,
WinForms, and Legacy ASP.NET.


[1] https://devblogs.microsoft.com/dotnet/configureawait-faq/

On Tue, Mar 16, 2021 at 10:54 PM Alexey Kukushkin 
wrote:

> Raymond,
>
> The article you referenced
>  is great! It
> seems to me the article rather proved what I suggested: an async operation
> implementation:
>
>- Uses the async operation thread (Ignite's thread) if ConfigureAwait is
>false (by default it is true)
>- Uses caller's current SynchornizationContext if it is specified
>- Uses caller's TaskScheduler.Current if current
>SynchornizationContext is null
>
> In the application code (outside framework callbacks) the
> SynchornizationContext.Current will be null and TaskScheduler.Current is
> the .NET's fork-join thread pool. Thus, normally the .NET thread pool would
> be used for continuations as Pavel suggested in the IEP.
>
> Executing Async operation in a context where
> SynchornizationContext.Current is not null means some framework explicitly
> configured the context and that means it is important to execute the
> continuation in that context (like UI or xUnit main thread)
>
> I agree that routing back to original context might result in waiting,
> which is very dangerous for an Ignite thread. We can create a worker thread
> to route continuation to original context.
>
>
> вт, 16 мар. 2021 г. в 21:40, Raymond Wilson :
>
> > There is a (long) discussion here (
> > https://devblogs.microsoft.com/dotnet/configureawait-faq/) regarding use
> > of
> > ConfigureAwait().
> >
> > Putting edge cases aside, there is a general use case for
> > .ConfigureAwait(true) in application UI contexts to ensure the
> continuation
> > joins a UI thread (for example), where failing to do so results in an
> > error.
> >
> > The other general use case relates more to library code where you are
> > typically running your tasks on the managed thread pool (assuming no
> custom
> > task schedulers etc). In this case the .library is agnostic about which
> > thread pool thread picks up the continuation, so forcing the continuation
> > to join to the original thread may be both a performance penalty from the
> > join overhead, but also may add latency waiting for that thread to become
> > available again.
> >
> > I don't have good insight into how the striped thread pool is used in
> > Ignite, but in highly concurrent environments letting those threads
> escape
> > into the calling client context seems like a bad idea in general.
> >
> > Stepping back a little, the Cache Async operations are good for when
> there
> > will be an IO operation incurred because the requested element is not
> > present in memory. If it is present in memory, then a Sync operation will
> > be more performant. Is it possible to do a two step operation like this
> > 'Cache.GetIfInMemory() ?? await Cache.GetAsync()' to allow the calling
> > context to optimise the calling model dynamically?
> >
> > Thanks,
> > Raymond.
> >
> >
> > On Wed, Mar 17, 2021 at 6:14 AM Alexey Kukushkin <
> > kukushkinale...@gmail.com>
> > wrote:
> >
> > > Pavel,
> > >
> > > My understanding might be wrong but I think the best practice (or even
> > > strongly recommended way) to implement async methods in .NET is to
> > execute
> > > continuation on the caller's thread if ConfigureAwait(false) was not
> > > specified. Pseudo-code might look like:
> > >
> > > async Task PutAsync(K k, V v)
> > > {
> > > var continuationExecutor = configureAwait
> > > ? (SynchronizationContext.Current ?? TaskScheduler.Current)
> > > : null;
> > >
> > > await <>
> > >
> > > continuationExecutor.Post(continuation);
> > > }
> > >
> > > I got this understanding from reading some blog
> > > about SynchronizationContext lots of time ago. They were saying they
> > > created SynchronizationContext specifically to allow posting
> > continuations
> > > to the caller's thread.
> > >
> > > The reason for that is to simplify the user's code to avoid routing in
> > the
> > > code. Suppose you have a UI (like WPF or WinForms) event handler that
> > must
> > > be processed on the U 

Re: IEP-70: Async Continuation Executor

[Alexey Kukushkin]

Raymond,

The article you referenced
 is great! It
seems to me the article rather proved what I suggested: an async operation
implementation:

   - Uses the async operation thread (Ignite's thread) if ConfigureAwait is
   false (by default it is true)
   - Uses caller's current SynchornizationContext if it is specified
   - Uses caller's TaskScheduler.Current if current
   SynchornizationContext is null

In the application code (outside framework callbacks) the
SynchornizationContext.Current will be null and TaskScheduler.Current is
the .NET's fork-join thread pool. Thus, normally the .NET thread pool would
be used for continuations as Pavel suggested in the IEP.

Executing Async operation in a context where
SynchornizationContext.Current is not null means some framework explicitly
configured the context and that means it is important to execute the
continuation in that context (like UI or xUnit main thread)

I agree that routing back to original context might result in waiting,
which is very dangerous for an Ignite thread. We can create a worker thread
to route continuation to original context.


вт, 16 мар. 2021 г. в 21:40, Raymond Wilson :

> There is a (long) discussion here (
> https://devblogs.microsoft.com/dotnet/configureawait-faq/) regarding use
> of
> ConfigureAwait().
>
> Putting edge cases aside, there is a general use case for
> .ConfigureAwait(true) in application UI contexts to ensure the continuation
> joins a UI thread (for example), where failing to do so results in an
> error.
>
> The other general use case relates more to library code where you are
> typically running your tasks on the managed thread pool (assuming no custom
> task schedulers etc). In this case the .library is agnostic about which
> thread pool thread picks up the continuation, so forcing the continuation
> to join to the original thread may be both a performance penalty from the
> join overhead, but also may add latency waiting for that thread to become
> available again.
>
> I don't have good insight into how the striped thread pool is used in
> Ignite, but in highly concurrent environments letting those threads escape
> into the calling client context seems like a bad idea in general.
>
> Stepping back a little, the Cache Async operations are good for when there
> will be an IO operation incurred because the requested element is not
> present in memory. If it is present in memory, then a Sync operation will
> be more performant. Is it possible to do a two step operation like this
> 'Cache.GetIfInMemory() ?? await Cache.GetAsync()' to allow the calling
> context to optimise the calling model dynamically?
>
> Thanks,
> Raymond.
>
>
> On Wed, Mar 17, 2021 at 6:14 AM Alexey Kukushkin <
> kukushkinale...@gmail.com>
> wrote:
>
> > Pavel,
> >
> > My understanding might be wrong but I think the best practice (or even
> > strongly recommended way) to implement async methods in .NET is to
> execute
> > continuation on the caller's thread if ConfigureAwait(false) was not
> > specified. Pseudo-code might look like:
> >
> > async Task PutAsync(K k, V v)
> > {
> > var continuationExecutor = configureAwait
> > ? (SynchronizationContext.Current ?? TaskScheduler.Current)
> > : null;
> >
> > await <>
> >
> > continuationExecutor.Post(continuation);
> > }
> >
> > I got this understanding from reading some blog
> > about SynchronizationContext lots of time ago. They were saying they
> > created SynchronizationContext specifically to allow posting
> continuations
> > to the caller's thread.
> >
> > The reason for that is to simplify the user's code to avoid routing in
> the
> > code. Suppose you have a UI (like WPF or WinForms) event handler that
> must
> > be processed on the U thread:
> >
> > async Task Button1_Click(EventArgs args)
> > {
> > ignite.PutAsync(args.Key, args.Value);
> > Button1.Disabled = true;
> > }
> >
> > Executing the "Button1.Disabled = true" on a ForkJoinPool pool would
> cause
> > a "Trying to modify UI on a non-UI thread" exception. But if you
> > capture SynchronizationContext.Current in PutAsync and then route
> > continuation to the captured context then the code would work.
> >
> > I think the users really expect the continuations to be executed on the
> > caller's thread.
> >
> > Sometimes you know that your continuation is really fast and safe and you
> > want to avoid switching threads to improve performance. In this case you
> > use ConfigureAwait(false) like
> >
> > ignite.PutAsync(args.Key, args.Value).ConfigureAwat(false);
> >
> > In this case the continuation executes on the Ignite thread without
> routing
> > to the caller's thread.
> >
> > вт, 16 мар. 2021 г. в 18:49, Pavel Tupitsyn :
> >
> > > Alexey,
> > >
> > > .NET thick API delegates to Java directly.
> > >
> > > When you do ICache.PutAsync():
> > > * Future is created on Java side, .listen() is called
> > > * TaskCompletionSource is created on .NET 

Re: IEP-70: Async Continuation Executor

[Raymond Wilson]

There is a (long) discussion here (
https://devblogs.microsoft.com/dotnet/configureawait-faq/) regarding use of
ConfigureAwait().

Putting edge cases aside, there is a general use case for
.ConfigureAwait(true) in application UI contexts to ensure the continuation
joins a UI thread (for example), where failing to do so results in an error.

The other general use case relates more to library code where you are
typically running your tasks on the managed thread pool (assuming no custom
task schedulers etc). In this case the .library is agnostic about which
thread pool thread picks up the continuation, so forcing the continuation
to join to the original thread may be both a performance penalty from the
join overhead, but also may add latency waiting for that thread to become
available again.

I don't have good insight into how the striped thread pool is used in
Ignite, but in highly concurrent environments letting those threads escape
into the calling client context seems like a bad idea in general.

Stepping back a little, the Cache Async operations are good for when there
will be an IO operation incurred because the requested element is not
present in memory. If it is present in memory, then a Sync operation will
be more performant. Is it possible to do a two step operation like this
'Cache.GetIfInMemory() ?? await Cache.GetAsync()' to allow the calling
context to optimise the calling model dynamically?

Thanks,
Raymond.


On Wed, Mar 17, 2021 at 6:14 AM Alexey Kukushkin 
wrote:

> Pavel,
>
> My understanding might be wrong but I think the best practice (or even
> strongly recommended way) to implement async methods in .NET is to execute
> continuation on the caller's thread if ConfigureAwait(false) was not
> specified. Pseudo-code might look like:
>
> async Task PutAsync(K k, V v)
> {
> var continuationExecutor = configureAwait
> ? (SynchronizationContext.Current ?? TaskScheduler.Current)
> : null;
>
> await <>
>
> continuationExecutor.Post(continuation);
> }
>
> I got this understanding from reading some blog
> about SynchronizationContext lots of time ago. They were saying they
> created SynchronizationContext specifically to allow posting continuations
> to the caller's thread.
>
> The reason for that is to simplify the user's code to avoid routing in the
> code. Suppose you have a UI (like WPF or WinForms) event handler that must
> be processed on the U thread:
>
> async Task Button1_Click(EventArgs args)
> {
> ignite.PutAsync(args.Key, args.Value);
> Button1.Disabled = true;
> }
>
> Executing the "Button1.Disabled = true" on a ForkJoinPool pool would cause
> a "Trying to modify UI on a non-UI thread" exception. But if you
> capture SynchronizationContext.Current in PutAsync and then route
> continuation to the captured context then the code would work.
>
> I think the users really expect the continuations to be executed on the
> caller's thread.
>
> Sometimes you know that your continuation is really fast and safe and you
> want to avoid switching threads to improve performance. In this case you
> use ConfigureAwait(false) like
>
> ignite.PutAsync(args.Key, args.Value).ConfigureAwat(false);
>
> In this case the continuation executes on the Ignite thread without routing
> to the caller's thread.
>
> вт, 16 мар. 2021 г. в 18:49, Pavel Tupitsyn :
>
> > Alexey,
> >
> > .NET thick API delegates to Java directly.
> >
> > When you do ICache.PutAsync():
> > * Future is created on Java side, .listen() is called
> > * TaskCompletionSource is created on .NET side, its Task is returned to
> the
> > user
> > * Operation completes, Future listener is called on the Java side
> > * Listener invokes JNI callback to .NET, where
> > TaskCompletionSource.SetResult is called
> >
> > Therefore, .NET user code (in ContinueWith or after await) will be
> executed
> > on the Java
> > thread that invokes the future listener.
> >
> > After the proposed fix, future listeners will be invoked on
> > ForkJoinPool#commonPool (instead of striped pool).
> > So .NET continuations will end up in commonPool as well, which solves the
> > problem for .NET automatically, no changes required.
> >
> > Does that make sense?
> >
> > On Tue, Mar 16, 2021 at 1:52 PM Alexey Kukushkin <
> > kukushkinale...@gmail.com>
> > wrote:
> >
> > > Hi Pavel,
> > >
> > > Extending Java async API with additional Executor parameters looks OK
> to
> > > me.
> > >
> > > It is not clear from the IEP how you are going to do that for .NET
> async
> > > API. My understanding is in .NET we do not add any Executors. Instead,
> > the
> > > Ignite Async API should use (SynchronizationContext.Current ??
> > > TaskScheduler.Current) by default and it should have exciting behavior
> > (use
> > > Ignite striped pool) if ConfigureAwait(false) was specified for the
> Task
> > > result.
> > >
> > > Is my understanding correct?
> > >
> > >
> > > пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :
> > >
> > > > Igniters,
> > > >
> > > > Please review 

Re: IEP-70: Async Continuation Executor

[Alexey Kukushkin]

Pavel,

My understanding might be wrong but I think the best practice (or even
strongly recommended way) to implement async methods in .NET is to execute
continuation on the caller's thread if ConfigureAwait(false) was not
specified. Pseudo-code might look like:

async Task PutAsync(K k, V v)
{
var continuationExecutor = configureAwait
? (SynchronizationContext.Current ?? TaskScheduler.Current)
: null;

await <>

continuationExecutor.Post(continuation);
}

I got this understanding from reading some blog
about SynchronizationContext lots of time ago. They were saying they
created SynchronizationContext specifically to allow posting continuations
to the caller's thread.

The reason for that is to simplify the user's code to avoid routing in the
code. Suppose you have a UI (like WPF or WinForms) event handler that must
be processed on the U thread:

async Task Button1_Click(EventArgs args)
{
ignite.PutAsync(args.Key, args.Value);
Button1.Disabled = true;
}

Executing the "Button1.Disabled = true" on a ForkJoinPool pool would cause
a "Trying to modify UI on a non-UI thread" exception. But if you
capture SynchronizationContext.Current in PutAsync and then route
continuation to the captured context then the code would work.

I think the users really expect the continuations to be executed on the
caller's thread.

Sometimes you know that your continuation is really fast and safe and you
want to avoid switching threads to improve performance. In this case you
use ConfigureAwait(false) like

ignite.PutAsync(args.Key, args.Value).ConfigureAwat(false);

In this case the continuation executes on the Ignite thread without routing
to the caller's thread.

вт, 16 мар. 2021 г. в 18:49, Pavel Tupitsyn :

> Alexey,
>
> .NET thick API delegates to Java directly.
>
> When you do ICache.PutAsync():
> * Future is created on Java side, .listen() is called
> * TaskCompletionSource is created on .NET side, its Task is returned to the
> user
> * Operation completes, Future listener is called on the Java side
> * Listener invokes JNI callback to .NET, where
> TaskCompletionSource.SetResult is called
>
> Therefore, .NET user code (in ContinueWith or after await) will be executed
> on the Java
> thread that invokes the future listener.
>
> After the proposed fix, future listeners will be invoked on
> ForkJoinPool#commonPool (instead of striped pool).
> So .NET continuations will end up in commonPool as well, which solves the
> problem for .NET automatically, no changes required.
>
> Does that make sense?
>
> On Tue, Mar 16, 2021 at 1:52 PM Alexey Kukushkin <
> kukushkinale...@gmail.com>
> wrote:
>
> > Hi Pavel,
> >
> > Extending Java async API with additional Executor parameters looks OK to
> > me.
> >
> > It is not clear from the IEP how you are going to do that for .NET async
> > API. My understanding is in .NET we do not add any Executors. Instead,
> the
> > Ignite Async API should use (SynchronizationContext.Current ??
> > TaskScheduler.Current) by default and it should have exciting behavior
> (use
> > Ignite striped pool) if ConfigureAwait(false) was specified for the Task
> > result.
> >
> > Is my understanding correct?
> >
> >
> > пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :
> >
> > > Igniters,
> > >
> > > Please review the IEP [1] and let me know your thoughts.
> > >
> > > [1]
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
> > >
> >
> >
> > --
> > Best regards,
> > Alexey
> >
>


-- 
Best regards,
Alexey

Re: IEP-70: Async Continuation Executor

[Igor Sapego]

Pavel, I like the proposal,

+1 from me

Best Regards,
Igor


On Tue, Mar 16, 2021 at 6:49 PM Pavel Tupitsyn  wrote:

> Alexey,
>
> .NET thick API delegates to Java directly.
>
> When you do ICache.PutAsync():
> * Future is created on Java side, .listen() is called
> * TaskCompletionSource is created on .NET side, its Task is returned to the
> user
> * Operation completes, Future listener is called on the Java side
> * Listener invokes JNI callback to .NET, where
> TaskCompletionSource.SetResult is called
>
> Therefore, .NET user code (in ContinueWith or after await) will be executed
> on the Java
> thread that invokes the future listener.
>
> After the proposed fix, future listeners will be invoked on
> ForkJoinPool#commonPool (instead of striped pool).
> So .NET continuations will end up in commonPool as well, which solves the
> problem for .NET automatically, no changes required.
>
> Does that make sense?
>
> On Tue, Mar 16, 2021 at 1:52 PM Alexey Kukushkin <
> kukushkinale...@gmail.com>
> wrote:
>
> > Hi Pavel,
> >
> > Extending Java async API with additional Executor parameters looks OK to
> > me.
> >
> > It is not clear from the IEP how you are going to do that for .NET async
> > API. My understanding is in .NET we do not add any Executors. Instead,
> the
> > Ignite Async API should use (SynchronizationContext.Current ??
> > TaskScheduler.Current) by default and it should have exciting behavior
> (use
> > Ignite striped pool) if ConfigureAwait(false) was specified for the Task
> > result.
> >
> > Is my understanding correct?
> >
> >
> > пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :
> >
> > > Igniters,
> > >
> > > Please review the IEP [1] and let me know your thoughts.
> > >
> > > [1]
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
> > >
> >
> >
> > --
> > Best regards,
> > Alexey
> >
>

Re: IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Alexey,

.NET thick API delegates to Java directly.

When you do ICache.PutAsync():
* Future is created on Java side, .listen() is called
* TaskCompletionSource is created on .NET side, its Task is returned to the
user
* Operation completes, Future listener is called on the Java side
* Listener invokes JNI callback to .NET, where
TaskCompletionSource.SetResult is called

Therefore, .NET user code (in ContinueWith or after await) will be executed
on the Java
thread that invokes the future listener.

After the proposed fix, future listeners will be invoked on
ForkJoinPool#commonPool (instead of striped pool).
So .NET continuations will end up in commonPool as well, which solves the
problem for .NET automatically, no changes required.

Does that make sense?

On Tue, Mar 16, 2021 at 1:52 PM Alexey Kukushkin 
wrote:

> Hi Pavel,
>
> Extending Java async API with additional Executor parameters looks OK to
> me.
>
> It is not clear from the IEP how you are going to do that for .NET async
> API. My understanding is in .NET we do not add any Executors. Instead, the
> Ignite Async API should use (SynchronizationContext.Current ??
> TaskScheduler.Current) by default and it should have exciting behavior (use
> Ignite striped pool) if ConfigureAwait(false) was specified for the Task
> result.
>
> Is my understanding correct?
>
>
> пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :
>
> > Igniters,
> >
> > Please review the IEP [1] and let me know your thoughts.
> >
> > [1]
> >
> >
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
> >
>
>
> --
> Best regards,
> Alexey
>

Re: IEP-70: Async Continuation Executor

[Alexey Kukushkin]

Hi Pavel,

Extending Java async API with additional Executor parameters looks OK to me.

It is not clear from the IEP how you are going to do that for .NET async
API. My understanding is in .NET we do not add any Executors. Instead, the
Ignite Async API should use (SynchronizationContext.Current ??
TaskScheduler.Current) by default and it should have exciting behavior (use
Ignite striped pool) if ConfigureAwait(false) was specified for the Task
result.

Is my understanding correct?


пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn :

> Igniters,
>
> Please review the IEP [1] and let me know your thoughts.
>
> [1]
>
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
>


-- 
Best regards,
Alexey

IEP-70: Async Continuation Executor

[Pavel Tupitsyn]

Igniters,

Please review the IEP [1] and let me know your thoughts.

[1]
https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor