You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Vladimir Ozerov <vo...@gridgain.com> on 2016/12/20 08:56:08 UTC
Make async API great again
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.
Re: Make async API great again
Posted by 李玉...@163,
18...@163.com.
+1
\u5728 2016/12/20 18:05, Pavel Tupitsyn \u5199\u9053:
> Huge +1 on this. Current API is very confusing.
>
> On Tue, Dec 20, 2016 at 11:56 AM, Vladimir Ozerov <vo...@gridgain.com>
> 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.
>>
Re: Make async API great again
Posted by Pavel Tupitsyn <pt...@apache.org>.
Huge +1 on this. Current API is very confusing.
On Tue, Dec 20, 2016 at 11:56 AM, Vladimir Ozerov <vo...@gridgain.com>
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.
>
Re: Make async API great again
Posted by Sergei Egorov <bs...@gmail.com>.
Cool!
Please check my neighboring thread about special .to() method on
IgniteFuture to make it even better :)
On Mon, Mar 27, 2017 at 1:32 PM Vladimir Ozerov <vo...@gridgain.com>
wrote:
> Igniters,
>
> I am glad to announce that old async-style is finally deprecated and now
> all asynchronous methods are defined explicitly! Woohoo! Thanks to Taras
> for this important contribution.
>
> On Mon, Jan 23, 2017 at 6:12 PM, Taras Ledkov <tl...@gridgain.com>
> wrote:
>
> > Each compute method produces task. For 'run', 'call' etc. methods the
> > classes of tasks are internal.
> > There are tests checking the task session by ComputeTaskFuture for these
> > methods.
> > I think we have to leave real class of future is
> > ComputeTaskInternalFuture#ComputeFuture to compatibility.
> >
> > The class of the future at the public API may be changed.
> >
> >
> >
> > On 20.01.2017 17:54, Vladimir Ozerov wrote:
> >
> >> IgniteCompute was designed this way initially. See
> IgniteCompute.future()
> >> override. May be it makes sense to leave this type only for execute(...)
> >> methods.
> >>
> >> On Fri, Jan 20, 2017 at 5:45 PM, Semyon Boikov <sb...@gridgain.com>
> >> wrote:
> >>
> >> Hi Taras,
> >>>
> >>> Why 'async' methods return ComputeTaskFuture, not just IgniteFuture? It
> >>> seems that ComputeTaskFuture is needed only for tasks?
> >>>
> >>>
> >>>
> >>>
> >>> On Fri, Jan 20, 2017 at 5:18 PM, Taras Ledkov <tl...@gridgain.com>
> >>> 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: tledkov@gridgain.com
> >>>>
> >>>>
> >>>>
> > --
> > Taras Ledkov
> > Mail-To: tledkov@gridgain.com
> >
> >
>
Re: Make async API great again
Posted by Denis Magda <dm...@apache.org>.
Promising beginning of the week! So many talks in the past and finally we came to the logical end.
Thanks to both of you Taras and Vovan to making this real ;)
Could any of you update the Migration Guide simplifying the life for those who will be migrating from AI 1.x?
https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+2.0+Migration+Guide <https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+2.0+Migration+Guide>
—
Denis
> On Mar 27, 2017, at 3:24 AM, Vladimir Ozerov <vo...@gridgain.com> wrote:
>
> Igniters,
>
> I am glad to announce that old async-style is finally deprecated and now
> all asynchronous methods are defined explicitly! Woohoo! Thanks to Taras
> for this important contribution.
>
> On Mon, Jan 23, 2017 at 6:12 PM, Taras Ledkov <tl...@gridgain.com> wrote:
>
>> Each compute method produces task. For 'run', 'call' etc. methods the
>> classes of tasks are internal.
>> There are tests checking the task session by ComputeTaskFuture for these
>> methods.
>> I think we have to leave real class of future is
>> ComputeTaskInternalFuture#ComputeFuture to compatibility.
>>
>> The class of the future at the public API may be changed.
>>
>>
>>
>> On 20.01.2017 17:54, Vladimir Ozerov wrote:
>>
>>> IgniteCompute was designed this way initially. See IgniteCompute.future()
>>> override. May be it makes sense to leave this type only for execute(...)
>>> methods.
>>>
>>> On Fri, Jan 20, 2017 at 5:45 PM, Semyon Boikov <sb...@gridgain.com>
>>> wrote:
>>>
>>> Hi Taras,
>>>>
>>>> Why 'async' methods return ComputeTaskFuture, not just IgniteFuture? It
>>>> seems that ComputeTaskFuture is needed only for tasks?
>>>>
>>>>
>>>>
>>>>
>>>> On Fri, Jan 20, 2017 at 5:18 PM, Taras Ledkov <tl...@gridgain.com>
>>>> 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: tledkov@gridgain.com
>>>>>
>>>>>
>>>>>
>> --
>> Taras Ledkov
>> Mail-To: tledkov@gridgain.com
>>
>>
Re: Make async API great again
Posted by Vladimir Ozerov <vo...@gridgain.com>.
Igniters,
I am glad to announce that old async-style is finally deprecated and now
all asynchronous methods are defined explicitly! Woohoo! Thanks to Taras
for this important contribution.
On Mon, Jan 23, 2017 at 6:12 PM, Taras Ledkov <tl...@gridgain.com> wrote:
> Each compute method produces task. For 'run', 'call' etc. methods the
> classes of tasks are internal.
> There are tests checking the task session by ComputeTaskFuture for these
> methods.
> I think we have to leave real class of future is
> ComputeTaskInternalFuture#ComputeFuture to compatibility.
>
> The class of the future at the public API may be changed.
>
>
>
> On 20.01.2017 17:54, Vladimir Ozerov wrote:
>
>> IgniteCompute was designed this way initially. See IgniteCompute.future()
>> override. May be it makes sense to leave this type only for execute(...)
>> methods.
>>
>> On Fri, Jan 20, 2017 at 5:45 PM, Semyon Boikov <sb...@gridgain.com>
>> wrote:
>>
>> Hi Taras,
>>>
>>> Why 'async' methods return ComputeTaskFuture, not just IgniteFuture? It
>>> seems that ComputeTaskFuture is needed only for tasks?
>>>
>>>
>>>
>>>
>>> On Fri, Jan 20, 2017 at 5:18 PM, Taras Ledkov <tl...@gridgain.com>
>>> 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: tledkov@gridgain.com
>>>>
>>>>
>>>>
> --
> Taras Ledkov
> Mail-To: tledkov@gridgain.com
>
>
Re: Make async API great again
Posted by Taras Ledkov <tl...@gridgain.com>.
Each compute method produces task. For 'run', 'call' etc. methods the
classes of tasks are internal.
There are tests checking the task session by ComputeTaskFuture for these
methods.
I think we have to leave real class of future is
ComputeTaskInternalFuture#ComputeFuture to compatibility.
The class of the future at the public API may be changed.
On 20.01.2017 17:54, Vladimir Ozerov wrote:
> IgniteCompute was designed this way initially. See IgniteCompute.future()
> override. May be it makes sense to leave this type only for execute(...)
> methods.
>
> On Fri, Jan 20, 2017 at 5:45 PM, Semyon Boikov <sb...@gridgain.com> wrote:
>
>> Hi Taras,
>>
>> Why 'async' methods return ComputeTaskFuture, not just IgniteFuture? It
>> seems that ComputeTaskFuture is needed only for tasks?
>>
>>
>>
>>
>> On Fri, Jan 20, 2017 at 5:18 PM, Taras Ledkov <tl...@gridgain.com>
>> 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: tledkov@gridgain.com
>>>
>>>
--
Taras Ledkov
Mail-To: tledkov@gridgain.com
Re: Make async API great again
Posted by Vladimir Ozerov <vo...@gridgain.com>.
IgniteCompute was designed this way initially. See IgniteCompute.future()
override. May be it makes sense to leave this type only for execute(...)
methods.
On Fri, Jan 20, 2017 at 5:45 PM, Semyon Boikov <sb...@gridgain.com> wrote:
> Hi Taras,
>
> Why 'async' methods return ComputeTaskFuture, not just IgniteFuture? It
> seems that ComputeTaskFuture is needed only for tasks?
>
>
>
>
> On Fri, Jan 20, 2017 at 5:18 PM, Taras Ledkov <tl...@gridgain.com>
> 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: tledkov@gridgain.com
> >
> >
>
Re: Make async API great again
Posted by Semyon Boikov <sb...@gridgain.com>.
Hi Taras,
Why 'async' methods return ComputeTaskFuture, not just IgniteFuture? It
seems that ComputeTaskFuture is needed only for tasks?
On Fri, Jan 20, 2017 at 5:18 PM, Taras Ledkov <tl...@gridgain.com> 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: tledkov@gridgain.com
>
>
Re: Make async API great again
Posted by Vladimir Ozerov <vo...@gridgain.com>.
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 <tl...@gridgain.com> 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: tledkov@gridgain.com
>
>
Re: Make async API great again
Posted by Taras Ledkov <tl...@gridgain.com>.
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: tledkov@gridgain.com
Re: Make async API great again
Posted by Yakov Zhdanov <yz...@apache.org>.
me too.
--Yakov
Re: Make async API great again
Posted by Vladimir Ozerov <vo...@gridgain.com>.
Answered.
On Mon, Dec 26, 2016 at 12:51 PM, Yakov Zhdanov <yz...@apache.org> 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 <vo...@gridgain.com>:
>
> > 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 <vo...@gridgain.com>
> > 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 <
> > dsetrakyan@apache.org>
> > > 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 <
> vozerov@gridgain.com
> > >
> > >> 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 <
> > >> dsetrakyan@apache.org
> > >> > >
> > >> > 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 <
> > >> vozerov@gridgain.com>
> > >> > > 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 <
> > >> > > dsetrakyan@apache.org>
> > >> > > > wrote:
> > >> > > >
> > >> > > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> > >> > > > sergi.vladykin@gmail.com
> > >> > > > > >
> > >> > > > > 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?
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>
Re: Make async API great again
Posted by Yakov Zhdanov <yz...@apache.org>.
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 <vo...@gridgain.com>:
> 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 <vo...@gridgain.com>
> 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 <
> dsetrakyan@apache.org>
> > 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 <vozerov@gridgain.com
> >
> >> 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 <
> >> dsetrakyan@apache.org
> >> > >
> >> > 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 <
> >> vozerov@gridgain.com>
> >> > > 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 <
> >> > > dsetrakyan@apache.org>
> >> > > > wrote:
> >> > > >
> >> > > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> >> > > > sergi.vladykin@gmail.com
> >> > > > > >
> >> > > > > 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?
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>
Re: Make async API great again
Posted by Vladimir Ozerov <vo...@gridgain.com>.
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 <vo...@gridgain.com>
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 <ds...@apache.org>
> 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 <vo...@gridgain.com>
>> 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 <
>> dsetrakyan@apache.org
>> > >
>> > 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 <
>> vozerov@gridgain.com>
>> > > 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 <
>> > > dsetrakyan@apache.org>
>> > > > wrote:
>> > > >
>> > > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
>> > > > sergi.vladykin@gmail.com
>> > > > > >
>> > > > > 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?
>> > > > >
>> > > >
>> > >
>> >
>>
>
>
Re: Make async API great again
Posted by Vladimir Ozerov <vo...@gridgain.com>.
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 <ds...@apache.org>
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 <vo...@gridgain.com>
> 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 <
> dsetrakyan@apache.org
> > >
> > 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 <
> vozerov@gridgain.com>
> > > 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 <
> > > dsetrakyan@apache.org>
> > > > wrote:
> > > >
> > > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> > > > sergi.vladykin@gmail.com
> > > > > >
> > > > > 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?
> > > > >
> > > >
> > >
> >
>
Re: Make async API great again
Posted by Dmitriy Setrakyan <ds...@apache.org>.
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 <vo...@gridgain.com>
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 <dsetrakyan@apache.org
> >
> 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 <vo...@gridgain.com>
> > 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 <
> > dsetrakyan@apache.org>
> > > wrote:
> > >
> > > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> > > sergi.vladykin@gmail.com
> > > > >
> > > > 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?
> > > >
> > >
> >
>
Re: Make async API great again
Posted by Vladimir Ozerov <vo...@gridgain.com>.
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 <ds...@apache.org>
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 <vo...@gridgain.com>
> 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 <
> dsetrakyan@apache.org>
> > wrote:
> >
> > > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> > sergi.vladykin@gmail.com
> > > >
> > > 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?
> > >
> >
>
Re: Make async API great again
Posted by Dmitriy Setrakyan <ds...@apache.org>.
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 <vo...@gridgain.com>
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 <ds...@apache.org>
> wrote:
>
> > On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <
> sergi.vladykin@gmail.com
> > >
> > 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?
> >
>
Re: Make async API great again
Posted by Vladimir Ozerov <vo...@gridgain.com>.
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 <ds...@apache.org>
wrote:
> On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <sergi.vladykin@gmail.com
> >
> 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?
>
Re: Make async API great again
Posted by Sergi Vladykin <se...@gmail.com>.
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 <ds...@apache.org>:
> On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <sergi.vladykin@gmail.com
> >
> 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?
>
Re: Make async API great again
Posted by Dmitriy Setrakyan <ds...@apache.org>.
On Tue, Dec 20, 2016 at 10:28 AM, Sergi Vladykin <se...@gmail.com>
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?
Re: Make async API great again
Posted by Sergi Vladykin <se...@gmail.com>.
+1 For removing withAsync. It is a broken design.
Sergi
2016-12-20 20:38 GMT+03:00 Dmitriy Setrakyan <ds...@apache.org>:
> On Tue, Dec 20, 2016 at 9:13 AM, Pavel Tupitsyn <pt...@apache.org>
> 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 <
> dsetrakyan@apache.org>
> > 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 <
> vozerov@gridgain.com>
> > > 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.
> > > >
> > >
> >
>
Re: Make async API great again
Posted by Dmitriy Setrakyan <ds...@apache.org>.
On Tue, Dec 20, 2016 at 9:13 AM, Pavel Tupitsyn <pt...@apache.org>
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 <ds...@apache.org>
> 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 <vo...@gridgain.com>
> > 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.
> > >
> >
>
Re: Make async API great again
Posted by Pavel Tupitsyn <pt...@apache.org>.
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 <ds...@apache.org>
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 <vo...@gridgain.com>
> 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.
> >
>
Re: Make async API great again
Posted by Dmitriy Setrakyan <ds...@apache.org>.
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 <vo...@gridgain.com>
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.
>