Apache Ignite Developers - Legacy Mail Archive

Rework "withAsync" in Apache 2.0

Classic

List

Threaded

13 messages Options

Vladimir Ozerov

Rework "withAsync" in Apache 2.0

Hi folks,

There was a discussion about not very good "withAsync()" API design [1]. As
we are approaching Ignite 2.0 it is a good time to rework it. My
considerations on how good API should look like and how should it be
implemented:

1) Sync and async must be *different interfaces*.
2) All async methods must return *futures*. No thread-locals.
3) Sync operations normally* should not* be implemented through async. This
is a long story - if we delegate to async, then we have to bother with
additional threads, associated back-pressure control and all that crap.
Sync call must be sync unless there is a very strong reason to go through
async path.
4) Last, but not least - we should re-design our futures to be more like
Java 8 *CompletableFuture*. It should be able to accept Executors to
continue computation, it should have lots of methods to deal with
continuations and joins, etc.. Ideally, it would be cool if could remove it
and use CompletableFuture, but it is clearly too early to drop Java 7
support. So we should at least try making our futures similar to
CompletableFuture.

I will create relevant tickets soon.

Thoughts?

[1]
http://apache-ignite-developers.2346864.n4.nabble.com/Net-separate-methods-for-async-operations-td3901.html

Alexey Goncharuk

Re: Rework "withAsync" in Apache 2.0

Big +1 on this in general.

I would also relax our guarantees on operations submitted from the same
thread. Currently we guarantee that sequential invocations of async
operations happen in the same order. I think that if a user wants such
guarantees, he must define these dependencies explicitly by calling chain()
on returning futures.

This change will significantly improve cache operations performance in
async mode.

3) Sync operations normally* should not* be implemented through async. This
> is a long story - if we delegate to async, then we have to bother with
> additional threads, associated back-pressure control and all that crap.
> Sync call must be sync unless there is a very strong reason to go through
> async path.
>
Not sure about this, though. In most cases a cache operation implies
request/response over the network, so I think we should have explicit
synchronous counterparts only for methods that are guaranteed to be local.

yzhdanov

Re: Rework "withAsync" in Apache 2.0

Agree with Alex. Vova, please go on with issues taking Alex's comments into
consideration.

Thanks!

--Yakov

2016-07-21 10:43 GMT+03:00 Alexey Goncharuk <[hidden email]>:

> Big +1 on this in general.
>
> I would also relax our guarantees on operations submitted from the same
> thread. Currently we guarantee that sequential invocations of async
> operations happen in the same order. I think that if a user wants such
> guarantees, he must define these dependencies explicitly by calling chain()
> on returning futures.
>
> This change will significantly improve cache operations performance in
> async mode.
>
> 3) Sync operations normally* should not* be implemented through async. This
> > is a long story - if we delegate to async, then we have to bother with
> > additional threads, associated back-pressure control and all that crap.
> > Sync call must be sync unless there is a very strong reason to go through
> > async path.
> >
> Not sure about this, though. In most cases a cache operation implies
> request/response over the network, so I think we should have explicit
> synchronous counterparts only for methods that are guaranteed to be local.
>

Semyon Boikov

Re: Rework "withAsync" in Apache 2.0

Another issue which usually confuses users is Ignite 'implementation
details' of asynchronous execution: it operation is local it can be
executed from calling thread (for example, if 'async put' is executed in
atomic cache from primary node then cache store will be updated from
calling thread). Does it make sense to fix this as well?

On Thu, Jul 21, 2016 at 10:55 AM, Yakov Zhdanov <[hidden email]> wrote:

> Agree with Alex. Vova, please go on with issues taking Alex's comments into
> consideration.
>
> Thanks!
>
> --Yakov
>
> 2016-07-21 10:43 GMT+03:00 Alexey Goncharuk <[hidden email]>:
>
> > Big +1 on this in general.
> >
> > I would also relax our guarantees on operations submitted from the same
> > thread. Currently we guarantee that sequential invocations of async
> > operations happen in the same order. I think that if a user wants such
> > guarantees, he must define these dependencies explicitly by calling
> chain()
> > on returning futures.
> >
> > This change will significantly improve cache operations performance in
> > async mode.
> >
> > 3) Sync operations normally* should not* be implemented through async.
> This
> > > is a long story - if we delegate to async, then we have to bother with
> > > additional threads, associated back-pressure control and all that crap.
> > > Sync call must be sync unless there is a very strong reason to go
> through
> > > async path.
> > >
> > Not sure about this, though. In most cases a cache operation implies
> > request/response over the network, so I think we should have explicit
> > synchronous counterparts only for methods that are guaranteed to be
> local.
> >
>

Vladimir Ozerov

Re: Rework "withAsync" in Apache 2.0

Alex.

Of course, some distributed operations will involve some kind of asynchrony
even in synchronous mode. My point is that we should not blindly do things
like that:

V get(K key) {
return getAsync(key),get();
}

Instead, get() has it's own path, getAsync() another path. But of course
they could share some common places. E.g. I remember we already fixed some
cache operations in this regard when users hit async semaphore limit when
calling synchronous gets.

Another point is that async instances may possibly accept user-provided
Executor.

Vladimir,

On Thu, Jul 21, 2016 at 11:04 AM, Semyon Boikov <[hidden email]>
wrote:

> Another issue which usually confuses users is Ignite 'implementation
> details' of asynchronous execution: it operation is local it can be
> executed from calling thread (for example, if 'async put' is executed in
> atomic cache from primary node then cache store will be updated from
> calling thread). Does it make sense to fix this as well?
>
>
> On Thu, Jul 21, 2016 at 10:55 AM, Yakov Zhdanov <[hidden email]>
> wrote:
>
> > Agree with Alex. Vova, please go on with issues taking Alex's comments
> into
> > consideration.
> >
> > Thanks!
> >
> > --Yakov
> >
> > 2016-07-21 10:43 GMT+03:00 Alexey Goncharuk <[hidden email]
> >:
> >
> > > Big +1 on this in general.
> > >
> > > I would also relax our guarantees on operations submitted from the same
> > > thread. Currently we guarantee that sequential invocations of async
> > > operations happen in the same order. I think that if a user wants such
> > > guarantees, he must define these dependencies explicitly by calling
> > chain()
> > > on returning futures.
> > >
> > > This change will significantly improve cache operations performance in
> > > async mode.
> > >
> > > 3) Sync operations normally* should not* be implemented through async.
> > This
> > > > is a long story - if we delegate to async, then we have to bother
> with
> > > > additional threads, associated back-pressure control and all that
> crap.
> > > > Sync call must be sync unless there is a very strong reason to go
> > through
> > > > async path.
> > > >
> > > Not sure about this, though. In most cases a cache operation implies
> > > request/response over the network, so I think we should have explicit
> > > synchronous counterparts only for methods that are guaranteed to be
> > local.
> > >
> >
>

Sergi

Re: Rework "withAsync" in Apache 2.0

+1

Finally it is time to drop this "cool feature" from Ignite!

Sergi

On Thu, Jul 21, 2016 at 11:13 AM, Vladimir Ozerov <[hidden email]>
wrote:

> Alex.
>
> Of course, some distributed operations will involve some kind of asynchrony
> even in synchronous mode. My point is that we should not blindly do things
> like that:
>
> V get(K key) {
> return getAsync(key),get();
> }
>
> Instead, get() has it's own path, getAsync() another path. But of course
> they could share some common places. E.g. I remember we already fixed some
> cache operations in this regard when users hit async semaphore limit when
> calling synchronous gets.
>
> Another point is that async instances may possibly accept user-provided
> Executor.
>
> Vladimir,
>
> On Thu, Jul 21, 2016 at 11:04 AM, Semyon Boikov <[hidden email]>
> wrote:
>
> > Another issue which usually confuses users is Ignite 'implementation
> > details' of asynchronous execution: it operation is local it can be
> > executed from calling thread (for example, if 'async put' is executed in
> > atomic cache from primary node then cache store will be updated from
> > calling thread). Does it make sense to fix this as well?
> >
> >
> > On Thu, Jul 21, 2016 at 10:55 AM, Yakov Zhdanov <[hidden email]>
> > wrote:
> >
> > > Agree with Alex. Vova, please go on with issues taking Alex's comments
> > into
> > > consideration.
> > >
> > > Thanks!
> > >
> > > --Yakov
> > >
> > > 2016-07-21 10:43 GMT+03:00 Alexey Goncharuk <
> [hidden email]
> > >:
> > >
> > > > Big +1 on this in general.
> > > >
> > > > I would also relax our guarantees on operations submitted from the
> same
> > > > thread. Currently we guarantee that sequential invocations of async
> > > > operations happen in the same order. I think that if a user wants
> such
> > > > guarantees, he must define these dependencies explicitly by calling
> > > chain()
> > > > on returning futures.
> > > >
> > > > This change will significantly improve cache operations performance
> in
> > > > async mode.
> > > >
> > > > 3) Sync operations normally* should not* be implemented through
> async.
> > > This
> > > > > is a long story - if we delegate to async, then we have to bother
> > with
> > > > > additional threads, associated back-pressure control and all that
> > crap.
> > > > > Sync call must be sync unless there is a very strong reason to go
> > > through
> > > > > async path.
> > > > >
> > > > Not sure about this, though. In most cases a cache operation implies
> > > > request/response over the network, so I think we should have explicit
> > > > synchronous counterparts only for methods that are guaranteed to be
> > > local.
> > > >
> > >
> >
>

dsetrakyan

Re: Rework "withAsync" in Apache 2.0

Do I understand correctly that the community is proposing to have 2
identical interfaces, one for sync operations and another for async
operations?

On Thu, Jul 21, 2016 at 12:15 PM, Sergi Vladykin <[hidden email]>
wrote:

> +1
>
> Finally it is time to drop this "cool feature" from Ignite!
>
> Sergi
>
> On Thu, Jul 21, 2016 at 11:13 AM, Vladimir Ozerov <[hidden email]>
> wrote:
>
> > Alex.
> >
> > Of course, some distributed operations will involve some kind of
> asynchrony
> > even in synchronous mode. My point is that we should not blindly do
> things
> > like that:
> >
> > V get(K key) {
> > return getAsync(key),get();
> > }
> >
> > Instead, get() has it's own path, getAsync() another path. But of course
> > they could share some common places. E.g. I remember we already fixed
> some
> > cache operations in this regard when users hit async semaphore limit when
> > calling synchronous gets.
> >
> > Another point is that async instances may possibly accept user-provided
> > Executor.
> >
> > Vladimir,
> >
> > On Thu, Jul 21, 2016 at 11:04 AM, Semyon Boikov <[hidden email]>
> > wrote:
> >
> > > Another issue which usually confuses users is Ignite 'implementation
> > > details' of asynchronous execution: it operation is local it can be
> > > executed from calling thread (for example, if 'async put' is executed
> in
> > > atomic cache from primary node then cache store will be updated from
> > > calling thread). Does it make sense to fix this as well?
> > >
> > >
> > > On Thu, Jul 21, 2016 at 10:55 AM, Yakov Zhdanov <[hidden email]>
> > > wrote:
> > >
> > > > Agree with Alex. Vova, please go on with issues taking Alex's
> comments
> > > into
> > > > consideration.
> > > >
> > > > Thanks!
> > > >
> > > > --Yakov
> > > >
> > > > 2016-07-21 10:43 GMT+03:00 Alexey Goncharuk <
> > [hidden email]
> > > >:
> > > >
> > > > > Big +1 on this in general.
> > > > >
> > > > > I would also relax our guarantees on operations submitted from the
> > same
> > > > > thread. Currently we guarantee that sequential invocations of async
> > > > > operations happen in the same order. I think that if a user wants
> > such
> > > > > guarantees, he must define these dependencies explicitly by calling
> > > > chain()
> > > > > on returning futures.
> > > > >
> > > > > This change will significantly improve cache operations performance
> > in
> > > > > async mode.
> > > > >
> > > > > 3) Sync operations normally* should not* be implemented through
> > async.
> > > > This
> > > > > > is a long story - if we delegate to async, then we have to bother
> > > with
> > > > > > additional threads, associated back-pressure control and all that
> > > crap.
> > > > > > Sync call must be sync unless there is a very strong reason to go
> > > > through
> > > > > > async path.
> > > > > >
> > > > > Not sure about this, though. In most cases a cache operation
> implies
> > > > > request/response over the network, so I think we should have
> explicit
> > > > > synchronous counterparts only for methods that are guaranteed to be
> > > > local.
> > > > >
> > > >
> > >
> >
>

Vladimir Ozerov

Re: Rework "withAsync" in Apache 2.0

It is a matter of taste. In .NET we have "Async" methods near synchronous
methods because it is common practice in .NET world:
V Get(K key);
Task<V> GetAsync(K key);

For Java we can go the same way, or introduce separate interface. It will
not be identical to synchronous, because it will have different return
types and probably will contain only those methods which support async
execution.

Personally I like .NET approach. It is simple and straightforward. User do
not have to bother about whether current instance is sync or async (like
now), and will not have to perform additional steps like
*IgniteCache.withAsync().get()*. Do you want to call GET asynchronously? No
problem, *IgniteCache.getAsync()*. Simple, expressive, straightforward.

The drawback is that our interfaces will become "heavier". But it is 2016
year, we all have IDEs. I do not see any problems if we will have 62 + 36 =
98 methods instead of current 62 on cache API.

Vladimir.

On Thu, Jul 21, 2016 at 12:21 PM, Dmitriy Setrakyan <[hidden email]>
wrote:

> Do I understand correctly that the community is proposing to have 2
> identical interfaces, one for sync operations and another for async
> operations?
>
> On Thu, Jul 21, 2016 at 12:15 PM, Sergi Vladykin <[hidden email]
> >
> wrote:
>
> > +1
> >
> > Finally it is time to drop this "cool feature" from Ignite!
> >
> > Sergi
> >
> > On Thu, Jul 21, 2016 at 11:13 AM, Vladimir Ozerov <[hidden email]>
> > wrote:
> >
> > > Alex.
> > >
> > > Of course, some distributed operations will involve some kind of
> > asynchrony
> > > even in synchronous mode. My point is that we should not blindly do
> > things
> > > like that:
> > >
> > > V get(K key) {
> > > return getAsync(key),get();
> > > }
> > >
> > > Instead, get() has it's own path, getAsync() another path. But of
> course
> > > they could share some common places. E.g. I remember we already fixed
> > some
> > > cache operations in this regard when users hit async semaphore limit
> when
> > > calling synchronous gets.
> > >
> > > Another point is that async instances may possibly accept user-provided
> > > Executor.
> > >
> > > Vladimir,
> > >
> > > On Thu, Jul 21, 2016 at 11:04 AM, Semyon Boikov <[hidden email]>
> > > wrote:
> > >
> > > > Another issue which usually confuses users is Ignite 'implementation
> > > > details' of asynchronous execution: it operation is local it can be
> > > > executed from calling thread (for example, if 'async put' is executed
> > in
> > > > atomic cache from primary node then cache store will be updated from
> > > > calling thread). Does it make sense to fix this as well?
> > > >
> > > >
> > > > On Thu, Jul 21, 2016 at 10:55 AM, Yakov Zhdanov <[hidden email]
> >
> > > > wrote:
> > > >
> > > > > Agree with Alex. Vova, please go on with issues taking Alex's
> > comments
> > > > into
> > > > > consideration.
> > > > >
> > > > > Thanks!
> > > > >
> > > > > --Yakov
> > > > >
> > > > > 2016-07-21 10:43 GMT+03:00 Alexey Goncharuk <
> > > [hidden email]
> > > > >:
> > > > >
> > > > > > Big +1 on this in general.
> > > > > >
> > > > > > I would also relax our guarantees on operations submitted from
> the
> > > same
> > > > > > thread. Currently we guarantee that sequential invocations of
> async
> > > > > > operations happen in the same order. I think that if a user wants
> > > such
> > > > > > guarantees, he must define these dependencies explicitly by
> calling
> > > > > chain()
> > > > > > on returning futures.
> > > > > >
> > > > > > This change will significantly improve cache operations
> performance
> > > in
> > > > > > async mode.
> > > > > >
> > > > > > 3) Sync operations normally* should not* be implemented through
> > > async.
> > > > > This
> > > > > > > is a long story - if we delegate to async, then we have to
> bother
> > > > with
> > > > > > > additional threads, associated back-pressure control and all
> that
> > > > crap.
> > > > > > > Sync call must be sync unless there is a very strong reason to
> go
> > > > > through
> > > > > > > async path.
> > > > > > >
> > > > > > Not sure about this, though. In most cases a cache operation
> > implies
> > > > > > request/response over the network, so I think we should have
> > explicit
> > > > > > synchronous counterparts only for methods that are guaranteed to
> be
> > > > > local.
> > > > > >
> > > > >
> > > >
> > >
> >
>

Vladimir Ozerov

Re: Rework "withAsync" in Apache 2.0

Moreover, have a look at *CompletableFuture *interface:

handle
handle*Async*

thenApply
thenApply*Async*

And so on. One more argument to return methods with "Async" suffix from
good old GridGain days.

On Thu, Jul 21, 2016 at 12:38 PM, Vladimir Ozerov <[hidden email]>
wrote:

> It is a matter of taste. In .NET we have "Async" methods near synchronous
> methods because it is common practice in .NET world:
> V Get(K key);
> Task<V> GetAsync(K key);
>
> For Java we can go the same way, or introduce separate interface. It will
> not be identical to synchronous, because it will have different return
> types and probably will contain only those methods which support async
> execution.
>
> Personally I like .NET approach. It is simple and straightforward. User do
> not have to bother about whether current instance is sync or async (like
> now), and will not have to perform additional steps like
> *IgniteCache.withAsync().get()*. Do you want to call GET asynchronously?
> No problem, *IgniteCache.getAsync()*. Simple, expressive, straightforward.
>
> The drawback is that our interfaces will become "heavier". But it is 2016
> year, we all have IDEs. I do not see any problems if we will have 62 + 36 =
> 98 methods instead of current 62 on cache API.
>
> Vladimir.
>
>
> On Thu, Jul 21, 2016 at 12:21 PM, Dmitriy Setrakyan <[hidden email]
> > wrote:
>
>> Do I understand correctly that the community is proposing to have 2
>> identical interfaces, one for sync operations and another for async
>> operations?
>>
>> On Thu, Jul 21, 2016 at 12:15 PM, Sergi Vladykin <
>> [hidden email]>
>> wrote:
>>
>> > +1
>> >
>> > Finally it is time to drop this "cool feature" from Ignite!
>> >
>> > Sergi
>> >
>> > On Thu, Jul 21, 2016 at 11:13 AM, Vladimir Ozerov <[hidden email]
>> >
>> > wrote:
>> >
>> > > Alex.
>> > >
>> > > Of course, some distributed operations will involve some kind of
>> > asynchrony
>> > > even in synchronous mode. My point is that we should not blindly do
>> > things
>> > > like that:
>> > >
>> > > V get(K key) {
>> > > return getAsync(key),get();
>> > > }
>> > >
>> > > Instead, get() has it's own path, getAsync() another path. But of
>> course
>> > > they could share some common places. E.g. I remember we already fixed
>> > some
>> > > cache operations in this regard when users hit async semaphore limit
>> when
>> > > calling synchronous gets.
>> > >
>> > > Another point is that async instances may possibly accept
>> user-provided
>> > > Executor.
>> > >
>> > > Vladimir,
>> > >
>> > > On Thu, Jul 21, 2016 at 11:04 AM, Semyon Boikov <[hidden email]
>> >
>> > > wrote:
>> > >
>> > > > Another issue which usually confuses users is Ignite 'implementation
>> > > > details' of asynchronous execution: it operation is local it can be
>> > > > executed from calling thread (for example, if 'async put' is
>> executed
>> > in
>> > > > atomic cache from primary node then cache store will be updated from
>> > > > calling thread). Does it make sense to fix this as well?
>> > > >
>> > > >
>> > > > On Thu, Jul 21, 2016 at 10:55 AM, Yakov Zhdanov <
>> [hidden email]>
>> > > > wrote:
>> > > >
>> > > > > Agree with Alex. Vova, please go on with issues taking Alex's
>> > comments
>> > > > into
>> > > > > consideration.
>> > > > >
>> > > > > Thanks!
>> > > > >
>> > > > > --Yakov
>> > > > >
>> > > > > 2016-07-21 10:43 GMT+03:00 Alexey Goncharuk <
>> > > [hidden email]
>> > > > >:
>> > > > >
>> > > > > > Big +1 on this in general.
>> > > > > >
>> > > > > > I would also relax our guarantees on operations submitted from
>> the
>> > > same
>> > > > > > thread. Currently we guarantee that sequential invocations of
>> async
>> > > > > > operations happen in the same order. I think that if a user
>> wants
>> > > such
>> > > > > > guarantees, he must define these dependencies explicitly by
>> calling
>> > > > > chain()
>> > > > > > on returning futures.
>> > > > > >
>> > > > > > This change will significantly improve cache operations
>> performance
>> > > in
>> > > > > > async mode.
>> > > > > >
>> > > > > > 3) Sync operations normally* should not* be implemented through
>> > > async.
>> > > > > This
>> > > > > > > is a long story - if we delegate to async, then we have to
>> bother
>> > > > with
>> > > > > > > additional threads, associated back-pressure control and all
>> that
>> > > > crap.
>> > > > > > > Sync call must be sync unless there is a very strong reason
>> to go
>> > > > > through
>> > > > > > > async path.
>> > > > > > >
>> > > > > > Not sure about this, though. In most cases a cache operation
>> > implies
>> > > > > > request/response over the network, so I think we should have
>> > explicit
>> > > > > > synchronous counterparts only for methods that are guaranteed
>> to be
>> > > > > local.
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>

Andrey Kornev

Re: Rework "withAsync" in Apache 2.0

In reply to this post by Semyon Boikov

Totally agree with this particular suggestion. On more than just a few occasions I've been greatly confused by Ignite's "asynchronous" operations that in reality are blocking under some circumstances and non-blocking under others... Go figure! The reason I was given for such design was performance, if I remember correctly. I hope the Ignite experts on this mailing list will be able to provide more specifics if the community is interested.

In general an asynchronous operation is the one that returns a future immediately without any blocking of the calling thread. The outcome of the invocation is then communicated back exclusively via the future.

With the growing popularity of reactive programming style (a la Rx Java, etc.), I think it is crucial to get this fixed.

Regards

Andrey

________________________________
From: Semyon Boikov <[hidden email]>
Sent: Thursday, July 21, 2016 1:04 AM
To: [hidden email]
Subject: Re: Rework "withAsync" in Apache 2.0

Another issue which usually confuses users is Ignite 'implementation
details' of asynchronous execution: it operation is local it can be
executed from calling thread (for example, if 'async put' is executed in
atomic cache from primary node then cache store will be updated from
calling thread). Does it make sense to fix this as well?

On Thu, Jul 21, 2016 at 10:55 AM, Yakov Zhdanov <[hidden email]> wrote:

Sergi

Re: Rework "withAsync" in Apache 2.0

In reply to this post by Vladimir Ozerov

I'm ok with any of these ways. Probably having methods with "Async" suffix
is simpler, having a separate interface for all the async methods is a bit
cleaner. Current IgniteAsyncSupport definitely was a big mistake.

Sergi

On Thu, Jul 21, 2016 at 12:41 PM, Vladimir Ozerov <[hidden email]>
wrote:

> Moreover, have a look at *CompletableFuture *interface:
>
> handle
> handle*Async*
>
> thenApply
> thenApply*Async*
>
> And so on. One more argument to return methods with "Async" suffix from
> good old GridGain days.
>
> On Thu, Jul 21, 2016 at 12:38 PM, Vladimir Ozerov <[hidden email]>
> wrote:
>
> > It is a matter of taste. In .NET we have "Async" methods near synchronous
> > methods because it is common practice in .NET world:
> > V Get(K key);
> > Task<V> GetAsync(K key);
> >
> > For Java we can go the same way, or introduce separate interface. It will
> > not be identical to synchronous, because it will have different return
> > types and probably will contain only those methods which support async
> > execution.
> >
> > Personally I like .NET approach. It is simple and straightforward. User
> do
> > not have to bother about whether current instance is sync or async (like
> > now), and will not have to perform additional steps like
> > *IgniteCache.withAsync().get()*. Do you want to call GET asynchronously?
> > No problem, *IgniteCache.getAsync()*. Simple, expressive,
> straightforward.
> >
> > The drawback is that our interfaces will become "heavier". But it is 2016
> > year, we all have IDEs. I do not see any problems if we will have 62 +
> 36 =
> > 98 methods instead of current 62 on cache API.
> >
> > Vladimir.
> >
> >
> > On Thu, Jul 21, 2016 at 12:21 PM, Dmitriy Setrakyan <
> [hidden email]
> > > wrote:
> >
> >> Do I understand correctly that the community is proposing to have 2
> >> identical interfaces, one for sync operations and another for async
> >> operations?
> >>
> >> On Thu, Jul 21, 2016 at 12:15 PM, Sergi Vladykin <
> >> [hidden email]>
> >> wrote:
> >>
> >> > +1
> >> >
> >> > Finally it is time to drop this "cool feature" from Ignite!
> >> >
> >> > Sergi
> >> >
> >> > On Thu, Jul 21, 2016 at 11:13 AM, Vladimir Ozerov <
> [hidden email]
> >> >
> >> > wrote:
> >> >
> >> > > Alex.
> >> > >
> >> > > Of course, some distributed operations will involve some kind of
> >> > asynchrony
> >> > > even in synchronous mode. My point is that we should not blindly do
> >> > things
> >> > > like that:
> >> > >
> >> > > V get(K key) {
> >> > > return getAsync(key),get();
> >> > > }
> >> > >
> >> > > Instead, get() has it's own path, getAsync() another path. But of
> >> course
> >> > > they could share some common places. E.g. I remember we already
> fixed
> >> > some
> >> > > cache operations in this regard when users hit async semaphore limit
> >> when
> >> > > calling synchronous gets.
> >> > >
> >> > > Another point is that async instances may possibly accept
> >> user-provided
> >> > > Executor.
> >> > >
> >> > > Vladimir,
> >> > >
> >> > > On Thu, Jul 21, 2016 at 11:04 AM, Semyon Boikov <
> [hidden email]
> >> >
> >> > > wrote:
> >> > >
> >> > > > Another issue which usually confuses users is Ignite
> 'implementation
> >> > > > details' of asynchronous execution: it operation is local it can
> be
> >> > > > executed from calling thread (for example, if 'async put' is
> >> executed
> >> > in
> >> > > > atomic cache from primary node then cache store will be updated
> from
> >> > > > calling thread). Does it make sense to fix this as well?
> >> > > >
> >> > > >
> >> > > > On Thu, Jul 21, 2016 at 10:55 AM, Yakov Zhdanov <
> >> [hidden email]>
> >> > > > wrote:
> >> > > >
> >> > > > > Agree with Alex. Vova, please go on with issues taking Alex's
> >> > comments
> >> > > > into
> >> > > > > consideration.
> >> > > > >
> >> > > > > Thanks!
> >> > > > >
> >> > > > > --Yakov
> >> > > > >
> >> > > > > 2016-07-21 10:43 GMT+03:00 Alexey Goncharuk <
> >> > > [hidden email]
> >> > > > >:
> >> > > > >
> >> > > > > > Big +1 on this in general.
> >> > > > > >
> >> > > > > > I would also relax our guarantees on operations submitted from
> >> the
> >> > > same
> >> > > > > > thread. Currently we guarantee that sequential invocations of
> >> async
> >> > > > > > operations happen in the same order. I think that if a user
> >> wants
> >> > > such
> >> > > > > > guarantees, he must define these dependencies explicitly by
> >> calling
> >> > > > > chain()
> >> > > > > > on returning futures.
> >> > > > > >
> >> > > > > > This change will significantly improve cache operations
> >> performance
> >> > > in
> >> > > > > > async mode.
> >> > > > > >
> >> > > > > > 3) Sync operations normally* should not* be implemented
> through
> >> > > async.
> >> > > > > This
> >> > > > > > > is a long story - if we delegate to async, then we have to
> >> bother
> >> > > > with
> >> > > > > > > additional threads, associated back-pressure control and all
> >> that
> >> > > > crap.
> >> > > > > > > Sync call must be sync unless there is a very strong reason
> >> to go
> >> > > > > through
> >> > > > > > > async path.
> >> > > > > > >
> >> > > > > > Not sure about this, though. In most cases a cache operation
> >> > implies
> >> > > > > > request/response over the network, so I think we should have
> >> > explicit
> >> > > > > > synchronous counterparts only for methods that are guaranteed
> >> to be
> >> > > > > local.
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>

dsetrakyan

Re: Rework "withAsync" in Apache 2.0

My opinion is that if we do away with current “withAsync()” paradigm, then
we should simply add the *async* methods directly to the original
interface. At least this way it will be easier to tell which methods
support async execution, and which don’t.

I don’t think it makes sense to have 2 separate interfaces, one for sync
and another for async.

D.

On Thu, Jul 21, 2016 at 1:01 PM, Sergi Vladykin <[hidden email]>
wrote:

> I'm ok with any of these ways. Probably having methods with "Async" suffix
> is simpler, having a separate interface for all the async methods is a bit
> cleaner. Current IgniteAsyncSupport definitely was a big mistake.
>
> Sergi
>
> On Thu, Jul 21, 2016 at 12:41 PM, Vladimir Ozerov <[hidden email]>
> wrote:
>
> > Moreover, have a look at *CompletableFuture *interface:
> >
> > handle
> > handle*Async*
> >
> > thenApply
> > thenApply*Async*
> >
> > And so on. One more argument to return methods with "Async" suffix from
> > good old GridGain days.
> >
> > On Thu, Jul 21, 2016 at 12:38 PM, Vladimir Ozerov <[hidden email]>
> > wrote:
> >
> > > It is a matter of taste. In .NET we have "Async" methods near
> synchronous
> > > methods because it is common practice in .NET world:
> > > V Get(K key);
> > > Task<V> GetAsync(K key);
> > >
> > > For Java we can go the same way, or introduce separate interface. It
> will
> > > not be identical to synchronous, because it will have different return
> > > types and probably will contain only those methods which support async
> > > execution.
> > >
> > > Personally I like .NET approach. It is simple and straightforward. User
> > do
> > > not have to bother about whether current instance is sync or async
> (like
> > > now), and will not have to perform additional steps like
> > > *IgniteCache.withAsync().get()*. Do you want to call GET
> asynchronously?
> > > No problem, *IgniteCache.getAsync()*. Simple, expressive,
> > straightforward.
> > >
> > > The drawback is that our interfaces will become "heavier". But it is
> 2016
> > > year, we all have IDEs. I do not see any problems if we will have 62 +
> > 36 =
> > > 98 methods instead of current 62 on cache API.
> > >
> > > Vladimir.
> > >
> > >
> > > On Thu, Jul 21, 2016 at 12:21 PM, Dmitriy Setrakyan <
> > [hidden email]
> > > > wrote:
> > >
> > >> Do I understand correctly that the community is proposing to have 2
> > >> identical interfaces, one for sync operations and another for async
> > >> operations?
> > >>
> > >> On Thu, Jul 21, 2016 at 12:15 PM, Sergi Vladykin <
> > >> [hidden email]>
> > >> wrote:
> > >>
> > >> > +1
> > >> >
> > >> > Finally it is time to drop this "cool feature" from Ignite!
> > >> >
> > >> > Sergi
> > >> >
> > >> > On Thu, Jul 21, 2016 at 11:13 AM, Vladimir Ozerov <
> > [hidden email]
> > >> >
> > >> > wrote:
> > >> >
> > >> > > Alex.
> > >> > >
> > >> > > Of course, some distributed operations will involve some kind of
> > >> > asynchrony
> > >> > > even in synchronous mode. My point is that we should not blindly
> do
> > >> > things
> > >> > > like that:
> > >> > >
> > >> > > V get(K key) {
> > >> > > return getAsync(key),get();
> > >> > > }
> > >> > >
> > >> > > Instead, get() has it's own path, getAsync() another path. But of
> > >> course
> > >> > > they could share some common places. E.g. I remember we already
> > fixed
> > >> > some
> > >> > > cache operations in this regard when users hit async semaphore
> limit
> > >> when
> > >> > > calling synchronous gets.
> > >> > >
> > >> > > Another point is that async instances may possibly accept
> > >> user-provided
> > >> > > Executor.
> > >> > >
> > >> > > Vladimir,
> > >> > >
> > >> > > On Thu, Jul 21, 2016 at 11:04 AM, Semyon Boikov <
> > [hidden email]
> > >> >
> > >> > > wrote:
> > >> > >
> > >> > > > Another issue which usually confuses users is Ignite
> > 'implementation
> > >> > > > details' of asynchronous execution: it operation is local it can
> > be
> > >> > > > executed from calling thread (for example, if 'async put' is
> > >> executed
> > >> > in
> > >> > > > atomic cache from primary node then cache store will be updated
> > from
> > >> > > > calling thread). Does it make sense to fix this as well?
> > >> > > >
> > >> > > >
> > >> > > > On Thu, Jul 21, 2016 at 10:55 AM, Yakov Zhdanov <
> > >> [hidden email]>
> > >> > > > wrote:
> > >> > > >
> > >> > > > > Agree with Alex. Vova, please go on with issues taking Alex's
> > >> > comments
> > >> > > > into
> > >> > > > > consideration.
> > >> > > > >
> > >> > > > > Thanks!
> > >> > > > >
> > >> > > > > --Yakov
> > >> > > > >
> > >> > > > > 2016-07-21 10:43 GMT+03:00 Alexey Goncharuk <
> > >> > > [hidden email]
> > >> > > > >:
> > >> > > > >
> > >> > > > > > Big +1 on this in general.
> > >> > > > > >
> > >> > > > > > I would also relax our guarantees on operations submitted
> from
> > >> the
> > >> > > same
> > >> > > > > > thread. Currently we guarantee that sequential invocations
> of
> > >> async
> > >> > > > > > operations happen in the same order. I think that if a user
> > >> wants
> > >> > > such
> > >> > > > > > guarantees, he must define these dependencies explicitly by
> > >> calling
> > >> > > > > chain()
> > >> > > > > > on returning futures.
> > >> > > > > >
> > >> > > > > > This change will significantly improve cache operations
> > >> performance
> > >> > > in
> > >> > > > > > async mode.
> > >> > > > > >
> > >> > > > > > 3) Sync operations normally* should not* be implemented
> > through
> > >> > > async.
> > >> > > > > This
> > >> > > > > > > is a long story - if we delegate to async, then we have to
> > >> bother
> > >> > > > with
> > >> > > > > > > additional threads, associated back-pressure control and
> all
> > >> that
> > >> > > > crap.
> > >> > > > > > > Sync call must be sync unless there is a very strong
> reason
> > >> to go
> > >> > > > > through
> > >> > > > > > > async path.
> > >> > > > > > >
> > >> > > > > > Not sure about this, though. In most cases a cache operation
> > >> > implies
> > >> > > > > > request/response over the network, so I think we should have
> > >> > explicit
> > >> > > > > > synchronous counterparts only for methods that are
> guaranteed
> > >> to be
> > >> > > > > local.
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>

Andrey Kornev

Re: Rework "withAsync" in Apache 2.0

In reply to this post by Vladimir Ozerov

+1 "Async" suffix.

________________________________
From: Vladimir Ozerov <[hidden email]>
Sent: Thursday, July 21, 2016 2:41 AM
To: [hidden email]
Subject: Re: Rework "withAsync" in Apache 2.0

Moreover, have a look at *CompletableFuture *interface:

handle
handle*Async*

thenApply
thenApply*Async*

And so on. One more argument to return methods with "Async" suffix from
good old GridGain days.

On Thu, Jul 21, 2016 at 12:38 PM, Vladimir Ozerov <[hidden email]>
wrote: