Make async API great again

[Vladimir Ozerov]

Igniters,

We have complaints about design of our async API, and upcoming Ignite 2.0
release is a good candidate to fix it.

Problems:

1) API is verbose and error prone:

Ignite asyncCache = cache.withAsync();
asyncCache.get(key);
IgniteFuture<V> fut = asyncCache.future();

2) Hard to reason which methods support async execution and which are not.
@IgniteAsyncSupported is not user friendly because it forces users to read
JavaDocs thoroughly.

3) No control on where IgniteFuture.listen() and IgniteFuture.chain()
methods are executed. User can easily inadvertently add some code which
will lead to starvation. E.g.:

IgniteFuture<V> fut = asyncCache.future();
fut.listen((fut) -> { cache.put(...); }); // Starvation because callback
might be executed in sys pool.

4) We have incorrect IO optimization: if message is to be send to local
node, it will be executed synchronously. See GridIoManager.send() method.
This is wrong, because it breaks pool semantics. E.g. cache operations must
be executed in sys pool, but can be executed in public pool.

I propose to fix all that problems in Apache Ignite 2.0:

1) Let's have simple and straightforward API:
IgniteFuture<V> fut = cache.getAsync(key);

2) Fix local node "optimization" in GridIoManager - messages should not be
processed synchronously.

3) User continuations must never be executed synchronously, always delegate
them to public pool by default (or may be to Java FJP?).

4) Allow users to specify thread pool for their continuations:
IgniteFuture.listen(Closure, ExecutorService);
IgniteFufure.chain(Closure, ExecutorService);

See Akka "Dispatcher" [1] as example of similar concept.

Thoughts?

[1] http://doc.akka.io/docs/akka/current/scala/dispatchers.html

Vladimir.

[Pavel Tupitsyn]

Huge +1 on this. Current API is very confusing.

On Tue, Dec 20, 2016 at 11:56 AM, Vladimir Ozerov <[hidden email]>
wrote:

> Igniters,
>
> We have complaints about design of our async API, and upcoming Ignite 2.0
> release is a good candidate to fix it.
>
> Problems:
>
> 1) API is verbose and error prone:
>
> Ignite asyncCache = cache.withAsync();
> asyncCache.get(key);
> IgniteFuture<V> fut = asyncCache.future();
>
> 2) Hard to reason which methods support async execution and which are not.
> @IgniteAsyncSupported is not user friendly because it forces users to read
> JavaDocs thoroughly.
>
> 3) No control on where IgniteFuture.listen() and IgniteFuture.chain()
> methods are executed. User can easily inadvertently add some code which
> will lead to starvation. E.g.:
>
> IgniteFuture<V> fut = asyncCache.future();
> fut.listen((fut) -> { cache.put(...); }); // Starvation because callback
> might be executed in sys pool.
>
> 4) We have incorrect IO optimization: if message is to be send to local
> node, it will be executed synchronously. See GridIoManager.send() method.
> This is wrong, because it breaks pool semantics. E.g. cache operations must
> be executed in sys pool, but can be executed in public pool.
>
> I propose to fix all that problems in Apache Ignite 2.0:
>
> 1) Let's have simple and straightforward API:
> IgniteFuture<V> fut = cache.getAsync(key);
>
> 2) Fix local node "optimization" in GridIoManager - messages should not be
> processed synchronously.
>
> 3) User continuations must never be executed synchronously, always delegate
> them to public pool by default (or may be to Java FJP?).
>
> 4) Allow users to specify thread pool for their continuations:
> IgniteFuture.listen(Closure, ExecutorService);
> IgniteFufure.chain(Closure, ExecutorService);
>
> See Akka "Dispatcher" [1] as example of similar concept.
>
> Thoughts?
>
> [1] http://doc.akka.io/docs/akka/current/scala/dispatchers.html
>
> Vladimir.
>

+1


在 2016/12/20 18:05, Pavel Tupitsyn 写道:

> Huge +1 on this. Current API is very confusing.
>
> On Tue, Dec 20, 2016 at 11:56 AM, Vladimir Ozerov <[hidden email]>
> wrote:
>
>> Igniters,
>>
>> We have complaints about design of our async API, and upcoming Ignite 2.0
>> release is a good candidate to fix it.
>>
>> Problems:
>>
>> 1) API is verbose and error prone:
>>
>> Ignite asyncCache = cache.withAsync();
>> asyncCache.get(key);
>> IgniteFuture<V> fut = asyncCache.future();
>>
>> 2) Hard to reason which methods support async execution and which are not.
>> @IgniteAsyncSupported is not user friendly because it forces users to read
>> JavaDocs thoroughly.
>>
>> 3) No control on where IgniteFuture.listen() and IgniteFuture.chain()
>> methods are executed. User can easily inadvertently add some code which
>> will lead to starvation. E.g.:
>>
>> IgniteFuture<V> fut = asyncCache.future();
>> fut.listen((fut) -> { cache.put(...); }); // Starvation because callback
>> might be executed in sys pool.
>>
>> 4) We have incorrect IO optimization: if message is to be send to local
>> node, it will be executed synchronously. See GridIoManager.send() method.
>> This is wrong, because it breaks pool semantics. E.g. cache operations must
>> be executed in sys pool, but can be executed in public pool.
>>
>> I propose to fix all that problems in Apache Ignite 2.0:
>>
>> 1) Let's have simple and straightforward API:
>> IgniteFuture<V> fut = cache.getAsync(key);
>>
>> 2) Fix local node "optimization" in GridIoManager - messages should not be
>> processed synchronously.
>>
>> 3) User continuations must never be executed synchronously, always delegate
>> them to public pool by default (or may be to Java FJP?).
>>
>> 4) Allow users to specify thread pool for their continuations:
>> IgniteFuture.listen(Closure, ExecutorService);
>> IgniteFufure.chain(Closure, ExecutorService);
>>
>> See Akka "Dispatcher" [1] as example of similar concept.
>>
>> Thoughts?
>>
>> [1] http://doc.akka.io/docs/akka/current/scala/dispatchers.html
>>
>> Vladimir.
>>

[dsetrakyan]

The impact of this change seems quite significant. Also, some of the
issues, like starvation, will not be fixed with introduction of the new
API, as far as I can tell.

I would be against removing the current async support from the API in one
motion. I think we can deprecate it right now, in 2.0, and completely
remove it in 3.0.

As far as the new async API, I would propose something like
IgniteCacheAsync with all the async methods. Otherwise, we face serious
method proliferation on the existing API, essentially doubling it in size.

D.

On Tue, Dec 20, 2016 at 12:56 AM, Vladimir Ozerov <[hidden email]>
wrote:

> Igniters,
>
> We have complaints about design of our async API, and upcoming Ignite 2.0
> release is a good candidate to fix it.
>
> Problems:
>
> 1) API is verbose and error prone:
>
> Ignite asyncCache = cache.withAsync();
> asyncCache.get(key);
> IgniteFuture<V> fut = asyncCache.future();
>
> 2) Hard to reason which methods support async execution and which are not.
> @IgniteAsyncSupported is not user friendly because it forces users to read
> JavaDocs thoroughly.
>
> 3) No control on where IgniteFuture.listen() and IgniteFuture.chain()
> methods are executed. User can easily inadvertently add some code which
> will lead to starvation. E.g.:
>
> IgniteFuture<V> fut = asyncCache.future();
> fut.listen((fut) -> { cache.put(...); }); // Starvation because callback
> might be executed in sys pool.
>
> 4) We have incorrect IO optimization: if message is to be send to local
> node, it will be executed synchronously. See GridIoManager.send() method.
> This is wrong, because it breaks pool semantics. E.g. cache operations must
> be executed in sys pool, but can be executed in public pool.
>
> I propose to fix all that problems in Apache Ignite 2.0:
>
> 1) Let's have simple and straightforward API:
> IgniteFuture<V> fut = cache.getAsync(key);
>
> 2) Fix local node "optimization" in GridIoManager - messages should not be
> processed synchronously.
>
> 3) User continuations must never be executed synchronously, always delegate
> them to public pool by default (or may be to Java FJP?).
>
> 4) Allow users to specify thread pool for their continuations:
> IgniteFuture.listen(Closure, ExecutorService);
> IgniteFufure.chain(Closure, ExecutorService);
>
> See Akka "Dispatcher" [1] as example of similar concept.
>
> Thoughts?
>
> [1] http://doc.akka.io/docs/akka/current/scala/dispatchers.html
>
> Vladimir.
>

[Pavel Tupitsyn]

I don't think that increased method number in the API is an issue.
Modern IDEs have sophisticated auto-complete features that make it easy to
find the right one.

As an API user, I would prefer sync and async methods side by side in the
same interface:
Use types 'cache.get(' and the IDE would show both sync and sync methods.
* Easy to discover async APIs even if you are not aware of them yet
* Easy to tell which methods have async counterparts

With a separate interface this is more verbose and complicated from the
user POV.

PS Link to prior discussion for Ignite.NET:
http://apache-ignite-developers.2346864.n4.nabble.com/Net-separate-methods-for-async-operations-td3901.html

Pavel


On Tue, Dec 20, 2016 at 7:57 PM, Dmitriy Setrakyan <[hidden email]>
wrote:

> The impact of this change seems quite significant. Also, some of the
> issues, like starvation, will not be fixed with introduction of the new
> API, as far as I can tell.
>
> I would be against removing the current async support from the API in one
> motion. I think we can deprecate it right now, in 2.0, and completely
> remove it in 3.0.
>
> As far as the new async API, I would propose something like
> IgniteCacheAsync with all the async methods. Otherwise, we face serious
> method proliferation on the existing API, essentially doubling it in size.
>
> D.
>
> On Tue, Dec 20, 2016 at 12:56 AM, Vladimir Ozerov <[hidden email]>
> wrote:
>
> > Igniters,
> >
> > We have complaints about design of our async API, and upcoming Ignite 2.0
> > release is a good candidate to fix it.
> >
> > Problems:
> >
> > 1) API is verbose and error prone:
> >
> > Ignite asyncCache = cache.withAsync();
> > asyncCache.get(key);
> > IgniteFuture<V> fut = asyncCache.future();
> >
> > 2) Hard to reason which methods support async execution and which are
> not.
> > @IgniteAsyncSupported is not user friendly because it forces users to
> read
> > JavaDocs thoroughly.
> >
> > 3) No control on where IgniteFuture.listen() and IgniteFuture.chain()
> > methods are executed. User can easily inadvertently add some code which
> > will lead to starvation. E.g.:
> >
> > IgniteFuture<V> fut = asyncCache.future();
> > fut.listen((fut) -> { cache.put(...); }); // Starvation because callback
> > might be executed in sys pool.
> >
> > 4) We have incorrect IO optimization: if message is to be send to local
> > node, it will be executed synchronously. See GridIoManager.send() method.
> > This is wrong, because it breaks pool semantics. E.g. cache operations
> must
> > be executed in sys pool, but can be executed in public pool.
> >
> > I propose to fix all that problems in Apache Ignite 2.0:
> >
> > 1) Let's have simple and straightforward API:
> > IgniteFuture<V> fut = cache.getAsync(key);
> >
> > 2) Fix local node "optimization" in GridIoManager - messages should not
> be
> > processed synchronously.
> >
> > 3) User continuations must never be executed synchronously, always
> delegate
> > them to public pool by default (or may be to Java FJP?).
> >
> > 4) Allow users to specify thread pool for their continuations:
> > IgniteFuture.listen(Closure, ExecutorService);
> > IgniteFufure.chain(Closure, ExecutorService);
> >
> > See Akka "Dispatcher" [1] as example of similar concept.
> >
> > Thoughts?
> >
> > [1] http://doc.akka.io/docs/akka/current/scala/dispatchers.html
> >
> > Vladimir.
> >
>

[dsetrakyan]

On Tue, Dec 20, 2016 at 9:13 AM, Pavel Tupitsyn <[hidden email]>
wrote:

> I don't think that increased method number in the API is an issue.
> Modern IDEs have sophisticated auto-complete features that make it easy to
> find the right one.
>

It is not an issue of IDE support. It is an issue of API complexity and
learning curve for new users.

By the way, do we have many examples of users complaining about the current
async approach in Ignite? I personally do not see any harm of leaving the
async API unchanged.

>
> On Tue, Dec 20, 2016 at 7:57 PM, Dmitriy Setrakyan <[hidden email]>
> wrote:
>
> > The impact of this change seems quite significant. Also, some of the
> > issues, like starvation, will not be fixed with introduction of the new
> > API, as far as I can tell.
> >
> > I would be against removing the current async support from the API in one
> > motion. I think we can deprecate it right now, in 2.0, and completely
> > remove it in 3.0.
> >
> > As far as the new async API, I would propose something like
> > IgniteCacheAsync with all the async methods. Otherwise, we face serious
> > method proliferation on the existing API, essentially doubling it in
> size.
> >
> > D.
> >
> > On Tue, Dec 20, 2016 at 12:56 AM, Vladimir Ozerov <[hidden email]>
> > wrote:
> >
> > > Igniters,
> > >
> > > We have complaints about design of our async API, and upcoming Ignite
> 2.0
> > > release is a good candidate to fix it.
> > >
> > > Problems:
> > >
> > > 1) API is verbose and error prone:
> > >
> > > Ignite asyncCache = cache.withAsync();
> > > asyncCache.get(key);
> > > IgniteFuture<V> fut = asyncCache.future();
> > >
> > > 2) Hard to reason which methods support async execution and which are
> > not.
> > > @IgniteAsyncSupported is not user friendly because it forces users to
> > read
> > > JavaDocs thoroughly.
> > >
> > > 3) No control on where IgniteFuture.listen() and IgniteFuture.chain()
> > > methods are executed. User can easily inadvertently add some code which
> > > will lead to starvation. E.g.:
> > >
> > > IgniteFuture<V> fut = asyncCache.future();
> > > fut.listen((fut) -> { cache.put(...); }); // Starvation because
> callback
> > > might be executed in sys pool.
> > >
> > > 4) We have incorrect IO optimization: if message is to be send to local
> > > node, it will be executed synchronously. See GridIoManager.send()
> method.
> > > This is wrong, because it breaks pool semantics. E.g. cache operations
> > must
> > > be executed in sys pool, but can be executed in public pool.
> > >
> > > I propose to fix all that problems in Apache Ignite 2.0:
> > >
> > > 1) Let's have simple and straightforward API:
> > > IgniteFuture<V> fut = cache.getAsync(key);
> > >
> > > 2) Fix local node "optimization" in GridIoManager - messages should not
> > be
> > > processed synchronously.
> > >
> > > 3) User continuations must never be executed synchronously, always
> > delegate
> > > them to public pool by default (or may be to Java FJP?).
> > >
> > > 4) Allow users to specify thread pool for their continuations:
> > > IgniteFuture.listen(Closure, ExecutorService);
> > > IgniteFufure.chain(Closure, ExecutorService);
> > >
> > > See Akka "Dispatcher" [1] as example of similar concept.
> > >
> > > Thoughts?
> > >
> > > [1] http://doc.akka.io/docs/akka/current/scala/dispatchers.html
> > >
> > > Vladimir.
> > >
> >
>

[Sergi]

+1 For removing withAsync. It is a broken design.

Sergi

2016-12-20 20:38 GMT+03:00 Dmitriy Setrakyan <[hidden email]>:

> On Tue, Dec 20, 2016 at 9:13 AM, Pavel Tupitsyn <[hidden email]>
> wrote:
>
> > I don't think that increased method number in the API is an issue.
> > Modern IDEs have sophisticated auto-complete features that make it easy
> to
> > find the right one.
> >
>
> It is not an issue of IDE support. It is an issue of API complexity and
> learning curve for new users.
>
> By the way, do we have many examples of users complaining about the current
> async approach in Ignite? I personally do not see any harm of leaving the
> async API unchanged.
>
>
>
> >
> > On Tue, Dec 20, 2016 at 7:57 PM, Dmitriy Setrakyan <
> [hidden email]>
> > wrote:
> >
> > > The impact of this change seems quite significant. Also, some of the
> > > issues, like starvation, will not be fixed with introduction of the new
> > > API, as far as I can tell.
> > >
> > > I would be against removing the current async support from the API in
> one
> > > motion. I think we can deprecate it right now, in 2.0, and completely
> > > remove it in 3.0.
> > >
> > > As far as the new async API, I would propose something like
> > > IgniteCacheAsync with all the async methods. Otherwise, we face serious
> > > method proliferation on the existing API, essentially doubling it in
> > size.
> > >
> > > D.
> > >
> > > On Tue, Dec 20, 2016 at 12:56 AM, Vladimir Ozerov <
> [hidden email]>
> > > wrote:
> > >
> > > > Igniters,
> > > >
> > > > We have complaints about design of our async API, and upcoming Ignite
> > 2.0
> > > > release is a good candidate to fix it.
> > > >
> > > > Problems:
> > > >
> > > > 1) API is verbose and error prone:
> > > >
> > > > Ignite asyncCache = cache.withAsync();
> > > > asyncCache.get(key);
> > > > IgniteFuture<V> fut = asyncCache.future();
> > > >
> > > > 2) Hard to reason which methods support async execution and which are
> > > not.
> > > > @IgniteAsyncSupported is not user friendly because it forces users to
> > > read
> > > > JavaDocs thoroughly.
> > > >
> > > > 3) No control on where IgniteFuture.listen() and IgniteFuture.chain()
> > > > methods are executed. User can easily inadvertently add some code
> which
> > > > will lead to starvation. E.g.:
> > > >
> > > > IgniteFuture<V> fut = asyncCache.future();
> > > > fut.listen((fut) -> { cache.put(...); }); // Starvation because
> > callback
> > > > might be executed in sys pool.
> > > >
> > > > 4) We have incorrect IO optimization: if message is to be send to
> local
> > > > node, it will be executed synchronously. See GridIoManager.send()
> > method.
> > > > This is wrong, because it breaks pool semantics. E.g. cache
> operations
> > > must
> > > > be executed in sys pool, but can be executed in public pool.
> > > >
> > > > I propose to fix all that problems in Apache Ignite 2.0:
> > > >
> > > > 1) Let's have simple and straightforward API:
> > > > IgniteFuture<V> fut = cache.getAsync(key);
> > > >
> > > > 2) Fix local node "optimization" in GridIoManager - messages should
> not
> > > be
> > > > processed synchronously.
> > > >
> > > > 3) User continuations must never be executed synchronously, always
> > > delegate
> > > > them to public pool by default (or may be to Java FJP?).
> > > >
> > > > 4) Allow users to specify thread pool for their continuations:
> > > > IgniteFuture.listen(Closure, ExecutorService);
> > > > IgniteFufure.chain(Closure, ExecutorService);
> > > >
> > > > See Akka "Dispatcher" [1] as example of similar concept.
> > > >
> > > > Thoughts?
> > > >
> > > > [1] http://doc.akka.io/docs/akka/current/scala/dispatchers.html
> > > >
> > > > Vladimir.
> > > >
> > >
> >
>

[dsetrakyan]

On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <[hidden email]>
wrote:

> +1 For removing withAsync. It is a broken design.
>

Sergi, do you also want to add all the async methods to the main API or do
you have some other design in mind?

[Sergi]

Me personal preference is to have a separate interface with all the
supported async methods. Basically in 2.0 we can have the same withAsync()
method but it will return this new interface.

Sergi

2016-12-20 21:31 GMT+03:00 Dmitriy Setrakyan <[hidden email]>:

> On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <[hidden email]
> >
> wrote:
>
> > +1 For removing withAsync. It is a broken design.
> >
>
> Sergi, do you also want to add all the async methods to the main API or do
> you have some other design in mind?
>

[Vladimir Ozerov]

We already discussed this several months ago in other thread.

"Async" methods is the most simple and straight API possible. .NET world
goes this way all over their frameworks and nobody died. Hazelcast also
goes this way. Java goes this way (see CompletableFuture). This is common
and well-known practice. The most impacted part of our API will be cache,
+33 new methods. Though, I do not see how it can affect learning curve.

Agree that we should deprecate AsyncSupport gradually and remove it no
earlier than in Apache Ignite 3.0.

On Tue, Dec 20, 2016 at 9:31 PM, Dmitriy Setrakyan <[hidden email]>
wrote:

> On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <[hidden email]
> >
> wrote:
>
> > +1 For removing withAsync. It is a broken design.
> >
>
> Sergi, do you also want to add all the async methods to the main API or do
> you have some other design in mind?
>

[dsetrakyan]

How difficult is this change? Does not look like it can be done overnight.

On Tue, Dec 20, 2016 at 10:46 AM, Vladimir Ozerov <[hidden email]>
wrote:

> We already discussed this several months ago in other thread.
>
> "Async" methods is the most simple and straight API possible. .NET world
> goes this way all over their frameworks and nobody died. Hazelcast also
> goes this way. Java goes this way (see CompletableFuture). This is common
> and well-known practice. The most impacted part of our API will be cache,
> +33 new methods. Though, I do not see how it can affect learning curve.
>
> Agree that we should deprecate AsyncSupport gradually and remove it no
> earlier than in Apache Ignite 3.0.
>
> On Tue, Dec 20, 2016 at 9:31 PM, Dmitriy Setrakyan <[hidden email]>
> wrote:
>
> > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> [hidden email]
> > >
> > wrote:
> >
> > > +1 For removing withAsync. It is a broken design.
> > >
> >
> > Sergi, do you also want to add all the async methods to the main API or
> do
> > you have some other design in mind?
> >
>

[Vladimir Ozerov]

Async API rework is mechanical addition of ~100 methods through copy-paste.
Should not take more than a day to implement and more than another day to
rework tests.

On Tue, Dec 20, 2016 at 10:00 PM, Dmitriy Setrakyan <[hidden email]>
wrote:

> How difficult is this change? Does not look like it can be done overnight.
>
> On Tue, Dec 20, 2016 at 10:46 AM, Vladimir Ozerov <[hidden email]>
> wrote:
>
> > We already discussed this several months ago in other thread.
> >
> > "Async" methods is the most simple and straight API possible. .NET world
> > goes this way all over their frameworks and nobody died. Hazelcast also
> > goes this way. Java goes this way (see CompletableFuture). This is common
> > and well-known practice. The most impacted part of our API will be cache,
> > +33 new methods. Though, I do not see how it can affect learning curve.
> >
> > Agree that we should deprecate AsyncSupport gradually and remove it no
> > earlier than in Apache Ignite 3.0.
> >
> > On Tue, Dec 20, 2016 at 9:31 PM, Dmitriy Setrakyan <
> [hidden email]>
> > wrote:
> >
> > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> > [hidden email]
> > > >
> > > wrote:
> > >
> > > > +1 For removing withAsync. It is a broken design.
> > > >
> > >
> > > Sergi, do you also want to add all the async methods to the main API or
> > do
> > > you have some other design in mind?
> > >
> >
>

[dsetrakyan]

Would be nice if someone would prototype a new cache API and post the
generated javadoc here. I think we all will benefit from reviewing it.

On Tue, Dec 20, 2016 at 12:17 PM, Vladimir Ozerov <[hidden email]>
wrote:

> Async API rework is mechanical addition of ~100 methods through copy-paste.
> Should not take more than a day to implement and more than another day to
> rework tests.
>
> On Tue, Dec 20, 2016 at 10:00 PM, Dmitriy Setrakyan <[hidden email]
> >
> wrote:
>
> > How difficult is this change? Does not look like it can be done
> overnight.
> >
> > On Tue, Dec 20, 2016 at 10:46 AM, Vladimir Ozerov <[hidden email]>
> > wrote:
> >
> > > We already discussed this several months ago in other thread.
> > >
> > > "Async" methods is the most simple and straight API possible. .NET
> world
> > > goes this way all over their frameworks and nobody died. Hazelcast also
> > > goes this way. Java goes this way (see CompletableFuture). This is
> common
> > > and well-known practice. The most impacted part of our API will be
> cache,
> > > +33 new methods. Though, I do not see how it can affect learning curve.
> > >
> > > Agree that we should deprecate AsyncSupport gradually and remove it no
> > > earlier than in Apache Ignite 3.0.
> > >
> > > On Tue, Dec 20, 2016 at 9:31 PM, Dmitriy Setrakyan <
> > [hidden email]>
> > > wrote:
> > >
> > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> > > [hidden email]
> > > > >
> > > > wrote:
> > > >
> > > > > +1 For removing withAsync. It is a broken design.
> > > > >
> > > >
> > > > Sergi, do you also want to add all the async methods to the main API
> or
> > > do
> > > > you have some other design in mind?
> > > >
> > >
> >
>

[Vladimir Ozerov]

Gents,

I created tickets for all proposed improvements:
1) Nice async API: https://issues.apache.org/jira/browse/IGNITE-4475
2) Do not process IO messages synchronously for local node:
https://issues.apache.org/jira/browse/IGNITE-4476
3) Better IgniteFuture API and callback semantics:
https://issues.apache.org/jira/browse/IGNITE-4477

Please review it and let me know if you have any comments.

Vladimir.

On Wed, Dec 21, 2016 at 4:32 AM, Dmitriy Setrakyan <[hidden email]>
wrote:

> Would be nice if someone would prototype a new cache API and post the
> generated javadoc here. I think we all will benefit from reviewing it.
>
> On Tue, Dec 20, 2016 at 12:17 PM, Vladimir Ozerov <[hidden email]>
> wrote:
>
> > Async API rework is mechanical addition of ~100 methods through
> copy-paste.
> > Should not take more than a day to implement and more than another day to
> > rework tests.
> >
> > On Tue, Dec 20, 2016 at 10:00 PM, Dmitriy Setrakyan <
> [hidden email]
> > >
> > wrote:
> >
> > > How difficult is this change? Does not look like it can be done
> > overnight.
> > >
> > > On Tue, Dec 20, 2016 at 10:46 AM, Vladimir Ozerov <
> [hidden email]>
> > > wrote:
> > >
> > > > We already discussed this several months ago in other thread.
> > > >
> > > > "Async" methods is the most simple and straight API possible. .NET
> > world
> > > > goes this way all over their frameworks and nobody died. Hazelcast
> also
> > > > goes this way. Java goes this way (see CompletableFuture). This is
> > common
> > > > and well-known practice. The most impacted part of our API will be
> > cache,
> > > > +33 new methods. Though, I do not see how it can affect learning
> curve.
> > > >
> > > > Agree that we should deprecate AsyncSupport gradually and remove it
> no
> > > > earlier than in Apache Ignite 3.0.
> > > >
> > > > On Tue, Dec 20, 2016 at 9:31 PM, Dmitriy Setrakyan <
> > > [hidden email]>
> > > > wrote:
> > > >
> > > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> > > > [hidden email]
> > > > > >
> > > > > wrote:
> > > > >
> > > > > > +1 For removing withAsync. It is a broken design.
> > > > > >
> > > > >
> > > > > Sergi, do you also want to add all the async methods to the main
> API
> > or
> > > > do
> > > > > you have some other design in mind?
> > > > >
> > > >
> > >
> >
>

[Vladimir Ozerov]

And several more improvements to future API:
1) Remove startTime() and duration() methods:
https://issues.apache.org/jira/browse/IGNITE-4479
2) Fix broken cancellation semantics:
https://issues.apache.org/jira/browse/IGNITE-4480

On Thu, Dec 22, 2016 at 1:40 PM, Vladimir Ozerov <[hidden email]>
wrote:

> Gents,
>
> I created tickets for all proposed improvements:
> 1) Nice async API: https://issues.apache.org/jira/browse/IGNITE-4475
> 2) Do not process IO messages synchronously for local node:
> https://issues.apache.org/jira/browse/IGNITE-4476
> 3) Better IgniteFuture API and callback semantics: https://issues.
> apache.org/jira/browse/IGNITE-4477
>
> Please review it and let me know if you have any comments.
>
> Vladimir.
>
> On Wed, Dec 21, 2016 at 4:32 AM, Dmitriy Setrakyan <[hidden email]>
> wrote:
>
>> Would be nice if someone would prototype a new cache API and post the
>> generated javadoc here. I think we all will benefit from reviewing it.
>>
>> On Tue, Dec 20, 2016 at 12:17 PM, Vladimir Ozerov <[hidden email]>
>> wrote:
>>
>> > Async API rework is mechanical addition of ~100 methods through
>> copy-paste.
>> > Should not take more than a day to implement and more than another day
>> to
>> > rework tests.
>> >
>> > On Tue, Dec 20, 2016 at 10:00 PM, Dmitriy Setrakyan <
>> [hidden email]
>> > >
>> > wrote:
>> >
>> > > How difficult is this change? Does not look like it can be done
>> > overnight.
>> > >
>> > > On Tue, Dec 20, 2016 at 10:46 AM, Vladimir Ozerov <
>> [hidden email]>
>> > > wrote:
>> > >
>> > > > We already discussed this several months ago in other thread.
>> > > >
>> > > > "Async" methods is the most simple and straight API possible. .NET
>> > world
>> > > > goes this way all over their frameworks and nobody died. Hazelcast
>> also
>> > > > goes this way. Java goes this way (see CompletableFuture). This is
>> > common
>> > > > and well-known practice. The most impacted part of our API will be
>> > cache,
>> > > > +33 new methods. Though, I do not see how it can affect learning
>> curve.
>> > > >
>> > > > Agree that we should deprecate AsyncSupport gradually and remove it
>> no
>> > > > earlier than in Apache Ignite 3.0.
>> > > >
>> > > > On Tue, Dec 20, 2016 at 9:31 PM, Dmitriy Setrakyan <
>> > > [hidden email]>
>> > > > wrote:
>> > > >
>> > > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
>> > > > [hidden email]
>> > > > > >
>> > > > > wrote:
>> > > > >
>> > > > > > +1 For removing withAsync. It is a broken design.
>> > > > > >
>> > > > >
>> > > > > Sergi, do you also want to add all the async methods to the main
>> API
>> > or
>> > > > do
>> > > > > you have some other design in mind?
>> > > > >
>> > > >
>> > >
>> >
>>
>
>

[yzhdanov]

Vladimir, I commented in https://issues.apache.org/jira/browse/IGNITE-4480
and https://issues.apache.org/jira/browse/IGNITE-4479 and
https://issues.apache.org/jira/browse/IGNITE-4476

I agree that current approach for async API is not good at all and needs to
be fixed.

As far as callback semantics, it does not seem to be a solution since user
may not pass executor parameter and will get the same starvation again. We
just need to add proper documentation and maybe implement smarter
starvation checker similar to one in striped pool.



--Yakov

2016-12-22 14:09 GMT+03:00 Vladimir Ozerov <[hidden email]>:

> And several more improvements to future API:
> 1) Remove startTime() and duration() methods:
> https://issues.apache.org/jira/browse/IGNITE-4479
> 2) Fix broken cancellation semantics:
> https://issues.apache.org/jira/browse/IGNITE-4480
>
> On Thu, Dec 22, 2016 at 1:40 PM, Vladimir Ozerov <[hidden email]>
> wrote:
>
> > Gents,
> >
> > I created tickets for all proposed improvements:
> > 1) Nice async API: https://issues.apache.org/jira/browse/IGNITE-4475
> > 2) Do not process IO messages synchronously for local node:
> > https://issues.apache.org/jira/browse/IGNITE-4476
> > 3) Better IgniteFuture API and callback semantics: https://issues.
> > apache.org/jira/browse/IGNITE-4477
> >
> > Please review it and let me know if you have any comments.
> >
> > Vladimir.
> >
> > On Wed, Dec 21, 2016 at 4:32 AM, Dmitriy Setrakyan <
> [hidden email]>
> > wrote:
> >
> >> Would be nice if someone would prototype a new cache API and post the
> >> generated javadoc here. I think we all will benefit from reviewing it.
> >>
> >> On Tue, Dec 20, 2016 at 12:17 PM, Vladimir Ozerov <[hidden email]
> >
> >> wrote:
> >>
> >> > Async API rework is mechanical addition of ~100 methods through
> >> copy-paste.
> >> > Should not take more than a day to implement and more than another day
> >> to
> >> > rework tests.
> >> >
> >> > On Tue, Dec 20, 2016 at 10:00 PM, Dmitriy Setrakyan <
> >> [hidden email]
> >> > >
> >> > wrote:
> >> >
> >> > > How difficult is this change? Does not look like it can be done
> >> > overnight.
> >> > >
> >> > > On Tue, Dec 20, 2016 at 10:46 AM, Vladimir Ozerov <
> >> [hidden email]>
> >> > > wrote:
> >> > >
> >> > > > We already discussed this several months ago in other thread.
> >> > > >
> >> > > > "Async" methods is the most simple and straight API possible. .NET
> >> > world
> >> > > > goes this way all over their frameworks and nobody died. Hazelcast
> >> also
> >> > > > goes this way. Java goes this way (see CompletableFuture). This is
> >> > common
> >> > > > and well-known practice. The most impacted part of our API will be
> >> > cache,
> >> > > > +33 new methods. Though, I do not see how it can affect learning
> >> curve.
> >> > > >
> >> > > > Agree that we should deprecate AsyncSupport gradually and remove
> it
> >> no
> >> > > > earlier than in Apache Ignite 3.0.
> >> > > >
> >> > > > On Tue, Dec 20, 2016 at 9:31 PM, Dmitriy Setrakyan <
> >> > > [hidden email]>
> >> > > > wrote:
> >> > > >
> >> > > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> >> > > > [hidden email]
> >> > > > > >
> >> > > > > wrote:
> >> > > > >
> >> > > > > > +1 For removing withAsync. It is a broken design.
> >> > > > > >
> >> > > > >
> >> > > > > Sergi, do you also want to add all the async methods to the main
> >> API
> >> > or
> >> > > > do
> >> > > > > you have some other design in mind?
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>

[Vladimir Ozerov]

Answered.

On Mon, Dec 26, 2016 at 12:51 PM, Yakov Zhdanov <[hidden email]> wrote:

> Vladimir, I commented in https://issues.apache.org/jira/browse/IGNITE-4480
> and https://issues.apache.org/jira/browse/IGNITE-4479 and
> https://issues.apache.org/jira/browse/IGNITE-4476
>
> I agree that current approach for async API is not good at all and needs to
> be fixed.
>
> As far as callback semantics, it does not seem to be a solution since user
> may not pass executor parameter and will get the same starvation again. We
> just need to add proper documentation and maybe implement smarter
> starvation checker similar to one in striped pool.
>
>
>
> --Yakov
>
> 2016-12-22 14:09 GMT+03:00 Vladimir Ozerov <[hidden email]>:
>
> > And several more improvements to future API:
> > 1) Remove startTime() and duration() methods:
> > https://issues.apache.org/jira/browse/IGNITE-4479
> > 2) Fix broken cancellation semantics:
> > https://issues.apache.org/jira/browse/IGNITE-4480
> >
> > On Thu, Dec 22, 2016 at 1:40 PM, Vladimir Ozerov <[hidden email]>
> > wrote:
> >
> > > Gents,
> > >
> > > I created tickets for all proposed improvements:
> > > 1) Nice async API: https://issues.apache.org/jira/browse/IGNITE-4475
> > > 2) Do not process IO messages synchronously for local node:
> > > https://issues.apache.org/jira/browse/IGNITE-4476
> > > 3) Better IgniteFuture API and callback semantics: https://issues.
> > > apache.org/jira/browse/IGNITE-4477
> > >
> > > Please review it and let me know if you have any comments.
> > >
> > > Vladimir.
> > >
> > > On Wed, Dec 21, 2016 at 4:32 AM, Dmitriy Setrakyan <
> > [hidden email]>
> > > wrote:
> > >
> > >> Would be nice if someone would prototype a new cache API and post the
> > >> generated javadoc here. I think we all will benefit from reviewing it.
> > >>
> > >> On Tue, Dec 20, 2016 at 12:17 PM, Vladimir Ozerov <
> [hidden email]
> > >
> > >> wrote:
> > >>
> > >> > Async API rework is mechanical addition of ~100 methods through
> > >> copy-paste.
> > >> > Should not take more than a day to implement and more than another
> day
> > >> to
> > >> > rework tests.
> > >> >
> > >> > On Tue, Dec 20, 2016 at 10:00 PM, Dmitriy Setrakyan <
> > >> [hidden email]
> > >> > >
> > >> > wrote:
> > >> >
> > >> > > How difficult is this change? Does not look like it can be done
> > >> > overnight.
> > >> > >
> > >> > > On Tue, Dec 20, 2016 at 10:46 AM, Vladimir Ozerov <
> > >> [hidden email]>
> > >> > > wrote:
> > >> > >
> > >> > > > We already discussed this several months ago in other thread.
> > >> > > >
> > >> > > > "Async" methods is the most simple and straight API possible.
> .NET
> > >> > world
> > >> > > > goes this way all over their frameworks and nobody died.
> Hazelcast
> > >> also
> > >> > > > goes this way. Java goes this way (see CompletableFuture). This
> is
> > >> > common
> > >> > > > and well-known practice. The most impacted part of our API will
> be
> > >> > cache,
> > >> > > > +33 new methods. Though, I do not see how it can affect learning
> > >> curve.
> > >> > > >
> > >> > > > Agree that we should deprecate AsyncSupport gradually and remove
> > it
> > >> no
> > >> > > > earlier than in Apache Ignite 3.0.
> > >> > > >
> > >> > > > On Tue, Dec 20, 2016 at 9:31 PM, Dmitriy Setrakyan <
> > >> > > [hidden email]>
> > >> > > > wrote:
> > >> > > >
> > >> > > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> > >> > > > [hidden email]
> > >> > > > > >
> > >> > > > > wrote:
> > >> > > > >
> > >> > > > > > +1 For removing withAsync. It is a broken design.
> > >> > > > > >
> > >> > > > >
> > >> > > > > Sergi, do you also want to add all the async methods to the
> main
> > >> API
> > >> > or
> > >> > > > do
> > >> > > > > you have some other design in mind?
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>

[yzhdanov]

me too.

--Yakov

[Taras Ledkov]

Gents

I've done changes of the IgniteCompute as a subtask of the whole async
API refactoring (https://issues.apache.org/jira/browse/IGNITE-4580).
Please check the new version of the public API
(https://github.com/gridgain/apache-ignite/blob/b81621bf2e8a35b20989f95ff52c0f6d91dd75d6/modules/core/src/main/java/org/apache/ignite/IgniteCompute.java)

Please look through the new API and let me know any comments.

--
Taras Ledkov
Mail-To: [hidden email]

[Vladimir Ozerov]

Very cool! Clean and straightforward.

P.S.: If someone doesn't understand the fix - please look at new methods
with "async" suffix [1].

[1] https://github.com/gridgain/apache-ignite/blob/b81621bf2e8a
35b20989f95ff52c0f6d91dd75d6/modules/core/src/main/java/
org/apache/ignite/IgniteCompute.java

On Fri, Jan 20, 2017 at 5:18 PM, Taras Ledkov <[hidden email]> wrote:

> Gents
>
> I've done changes of the IgniteCompute as a subtask of the whole async API
> refactoring (https://issues.apache.org/jira/browse/IGNITE-4580).
> Please check the new version of the public API (
> https://github.com/gridgain/apache-ignite/blob/b81621bf2e8a
> 35b20989f95ff52c0f6d91dd75d6/modules/core/src/main/java/
> org/apache/ignite/IgniteCompute.java)
>
> Please look through the new API and let me know any comments.
>
> --
> Taras Ledkov
> Mail-To: [hidden email]
>
>