RE: Semaphore action

[Andrey Kornev]

Yakov,

Thank you for your feedback.

I agree with the first point. My thinking was based on previous unfortunate 
experiences with user callbacks being executed on Ignite system threads while 
some internal locks are held, which made the Ignite API non-reentrant (a 
salient example in 1.4 is the Event API's listeners handling the discovery 
events). The Semaphore API must guarantee the Ignite API's reentrancy and 
clearly document the semantics. In such case, the overload with the Executor is 
redundant.

Wrt the second point, the original signature correctly unwraps the action's 
result type as T. Your suggestion would result in the method's return type 
becoming IgniteFuture<IgniteFuture<...>>  which is undesirable. Besides, the 
original signature makes the asynchronous contract explicit -- the action must 
return a future in order to be asynchronous. 

I only wish IgniteFuture extended JUC's Future! Is there any particular reason 
it doesn't?

Regards
Andrey

> Date: Mon, 18 Jan 2016 14:28:24 +0300
> Subject: Re: Semaphore action
> From: yzhda...@apache.org
> To: dev@ignite.apache.org
> 
> I like the idea. Couple of comments though.
> 
> I would leave only  IgniteFuture acquireAndExecute(Callable
> action); Overload with executor can be omitted because callable
> implementation can use any executor. As far as moving acquire logic to
> another thread pool - I think we'd better go without it same as for any
> other Ignite operations. Andrey, what was the pool intended for?
> 
> This T> IgniteFuture acquireAndExecuteAsyncAction(Callable<IgniteFuture>
> action); can be implemented with   IgniteFuture
> acquireAndExecute(Callable action); where T is IgniteFuture for
> operations initiated and returned by callable. Makes sense?
> 
> 
> --Yakov
> 
> 2016-01-18 0:39 GMT+03:00 Andrey Kornev <andrewkor...@hotmail.com>:
> 
> > Just to clarify, 7.4 below refers to GridGain 7.4, which is Ignite 1.4.
> > _
> > From: Andrey Kornev <andrewkor...@hotmail.com>
> > Sent: Sunday, January 17, 2016 10:27 AM
> > Subject: RE: Semaphore action
> > To:  <dev@ignite.apache.org>
> >
> >
> >Vladisav,
> >
> >  It would be great if you could implement the enhancements!
> >
> >  And while we're at it, here's something else I'd like us to consider. The
> > proposed API only gets us half way there: there is no longer a blocking
> > wait for the permit, but the action must be blocking. I really think it
> > should be possible to do the whole thing asynchronously including the
> > action and release of the permit. (In fact, this is what I need for my
> > project. In 7.4, I was able to implement this feature using just the public
> > APIs, but I'd really like to drop that code and start using the Semaphore
> > API.)
> >
> >  For example, upon the permit acquisition, the action could fire off a
> > distributed compute and rather than block waiting for the results to come
> > back, it would simply return the future provided by Ignite's withAsync().
> > The semaphore implementation would not release the permit immediately upon
> > the action return. Instead, it'd do so only when the action future
> > completes.
> >
> >  Here's how the API might look like:
> >
> >   IgniteFuture
> > acquireAndExecuteAsyncAction(Callable<IgniteFuture> action);
> >
> >  This API makes it possible to execute a protected action with no blocking
> > whatsoever! It's a lot more efficient, reduces the pressure on Ignite's
> > thread pools and prevents thread starvation, and so on.
> >
> >  Regards
> >  Andrey
> >
> >  > Date: Sun, 17 Jan 2016 10:40:53 +0100
> >  > Subject: Re: Semaphore action
> >  > From: vladis...@gmail.com
> >  > To: dev@ignite.apache.org
> >  >
> >  > It does sounds like a useful addition,
> >  > and I believe it could be implemented easily.
> >  > I can do it if the community agrees on changing the API of the
> > Semaphore.
> >  >
> >  > Best regards,
> >  > VJ
> >  >
> >  > On Sun, Jan 17, 2016 at 5:53 AM, Andrey Kornev <
> > andrewkor...@hotmail.com>
> >  > wrote:
> >  >
> >  > > Dmitriy,I don't believe it makes sense to have the action execute
> >  > > remotely. At least I didn't think of it.In my mind, the action is
> > always
> >  > > local (that's the reason I suggested the Java's Callable in the API
> > and not
> >  > > serializable IgniteCallable, in the first place). What the action
> > does is
> >  > > 

Re: Semaphore action

[Yakov Zhdanov]

I like the idea. Couple of comments though.

I would leave only  IgniteFuture acquireAndExecute(Callable
action); Overload with executor can be omitted because callable
implementation can use any executor. As far as moving acquire logic to
another thread pool - I think we'd better go without it same as for any
other Ignite operations. Andrey, what was the pool intended for?

This T> IgniteFuture acquireAndExecuteAsyncAction(Callable<IgniteFuture>
action); can be implemented with   IgniteFuture
acquireAndExecute(Callable action); where T is IgniteFuture for
operations initiated and returned by callable. Makes sense?


--Yakov

2016-01-18 0:39 GMT+03:00 Andrey Kornev <andrewkor...@hotmail.com>:

> Just to clarify, 7.4 below refers to GridGain 7.4, which is Ignite 1.4.
> _
> From: Andrey Kornev <andrewkor...@hotmail.com>
> Sent: Sunday, January 17, 2016 10:27 AM
> Subject: RE: Semaphore action
> To:  <dev@ignite.apache.org>
>
>
>Vladisav,
>
>  It would be great if you could implement the enhancements!
>
>  And while we're at it, here's something else I'd like us to consider. The
> proposed API only gets us half way there: there is no longer a blocking
> wait for the permit, but the action must be blocking. I really think it
> should be possible to do the whole thing asynchronously including the
> action and release of the permit. (In fact, this is what I need for my
> project. In 7.4, I was able to implement this feature using just the public
> APIs, but I'd really like to drop that code and start using the Semaphore
> API.)
>
>  For example, upon the permit acquisition, the action could fire off a
> distributed compute and rather than block waiting for the results to come
> back, it would simply return the future provided by Ignite's withAsync().
> The semaphore implementation would not release the permit immediately upon
> the action return. Instead, it'd do so only when the action future
> completes.
>
>  Here's how the API might look like:
>
>   IgniteFuture
> acquireAndExecuteAsyncAction(Callable<IgniteFuture> action);
>
>  This API makes it possible to execute a protected action with no blocking
> whatsoever! It's a lot more efficient, reduces the pressure on Ignite's
> thread pools and prevents thread starvation, and so on.
>
>  Regards
>  Andrey
>
>  > Date: Sun, 17 Jan 2016 10:40:53 +0100
>  > Subject: Re: Semaphore action
>  > From: vladis...@gmail.com
>  > To: dev@ignite.apache.org
>  >
>  > It does sounds like a useful addition,
>  > and I believe it could be implemented easily.
>  > I can do it if the community agrees on changing the API of the
> Semaphore.
>  >
>  > Best regards,
>  > VJ
>  >
>  > On Sun, Jan 17, 2016 at 5:53 AM, Andrey Kornev <
> andrewkor...@hotmail.com>
>  > wrote:
>  >
>  > > Dmitriy,I don't believe it makes sense to have the action execute
>  > > remotely. At least I didn't think of it.In my mind, the action is
> always
>  > > local (that's the reason I suggested the Java's Callable in the API
> and not
>  > > serializable IgniteCallable, in the first place). What the action
> does is
>  > > entirely up to the user. One could fire off a remote compute if
> that's what
>  > > one wants.I hope I make myself clear.ThanksAndrey
>  > > _
>  > > From: Dmitriy Setrakyan <dsetrak...@apache.org>
>  > > Sent: Saturday, January 16, 2016 7:54 AM
>  > > Subject: Re: Semaphore action
>  > > To:  <dev@ignite.apache.org>
>  > >
>  > >
>  > >Andrey,
>  > >
>  > >  In general this seems like a good addition. However, I do not
> understand
>  > >  how you can specify an executor, given that execution could happen
>  > >  remotely. Can you please clarify?
>  > >
>  > >  D.
>  > >
>  > >  On Fri, Jan 15, 2016 at 1:00 PM, Andrey Kornev <
> andrewkor...@hotmail.com>
>  > >  wrote:
>  > >
>  > >  > Hi there,
>  > >  >
>  > >  > The Semaphore feature was a great addition to Ignite's
> synchronization
>  > >  > primitive toolkit. I'd like to propose an enhancement to make
> Semaphore
>  > > API
>  > >  > even more useful.
>  > >  >
>  > >  > My biggest complaint about the current API is its blocking nature.
> For
>  > >  > example, the only way to acquire a permit is by using the blocking
>  > >  > acquire() method (tryAcquire() is not very useful in the cases

Re: Semaphore action

[Vladisav Jelisavcic]

I created a ticket from this discussion:

https://issues.apache.org/jira/browse/IGNITE-2399

Can someone please review it (and edit if needed)?

Best regrads,
VJ

On Mon, Jan 18, 2016 at 12:28 PM, Yakov Zhdanov <yzhda...@apache.org> wrote:

> I like the idea. Couple of comments though.
>
> I would leave only  IgniteFuture acquireAndExecute(Callable
> action); Overload with executor can be omitted because callable
> implementation can use any executor. As far as moving acquire logic to
> another thread pool - I think we'd better go without it same as for any
> other Ignite operations. Andrey, what was the pool intended for?
>
> This T> IgniteFuture
> acquireAndExecuteAsyncAction(Callable<IgniteFuture>
> action); can be implemented with   IgniteFuture
> acquireAndExecute(Callable action); where T is IgniteFuture for
> operations initiated and returned by callable. Makes sense?
>
>
> --Yakov
>
> 2016-01-18 0:39 GMT+03:00 Andrey Kornev <andrewkor...@hotmail.com>:
>
> > Just to clarify, 7.4 below refers to GridGain 7.4, which is Ignite 1.4.
> > _
> > From: Andrey Kornev <andrewkor...@hotmail.com>
> > Sent: Sunday, January 17, 2016 10:27 AM
> > Subject: RE: Semaphore action
> > To:  <dev@ignite.apache.org>
> >
> >
> >Vladisav,
> >
> >  It would be great if you could implement the enhancements!
> >
> >  And while we're at it, here's something else I'd like us to consider.
> The
> > proposed API only gets us half way there: there is no longer a blocking
> > wait for the permit, but the action must be blocking. I really think it
> > should be possible to do the whole thing asynchronously including the
> > action and release of the permit. (In fact, this is what I need for my
> > project. In 7.4, I was able to implement this feature using just the
> public
> > APIs, but I'd really like to drop that code and start using the Semaphore
> > API.)
> >
> >  For example, upon the permit acquisition, the action could fire off a
> > distributed compute and rather than block waiting for the results to come
> > back, it would simply return the future provided by Ignite's withAsync().
> > The semaphore implementation would not release the permit immediately
> upon
> > the action return. Instead, it'd do so only when the action future
> > completes.
> >
> >  Here's how the API might look like:
> >
> >   IgniteFuture
> > acquireAndExecuteAsyncAction(Callable<IgniteFuture> action);
> >
> >  This API makes it possible to execute a protected action with no
> blocking
> > whatsoever! It's a lot more efficient, reduces the pressure on Ignite's
> > thread pools and prevents thread starvation, and so on.
> >
> >  Regards
> >  Andrey
> >
> >  > Date: Sun, 17 Jan 2016 10:40:53 +0100
> >  > Subject: Re: Semaphore action
> >  > From: vladis...@gmail.com
> >  > To: dev@ignite.apache.org
> >  >
> >  > It does sounds like a useful addition,
> >  > and I believe it could be implemented easily.
> >  > I can do it if the community agrees on changing the API of the
> > Semaphore.
> >  >
> >  > Best regards,
> >  > VJ
> >  >
> >  > On Sun, Jan 17, 2016 at 5:53 AM, Andrey Kornev <
> > andrewkor...@hotmail.com>
> >  > wrote:
> >  >
> >  > > Dmitriy,I don't believe it makes sense to have the action execute
> >  > > remotely. At least I didn't think of it.In my mind, the action is
> > always
> >  > > local (that's the reason I suggested the Java's Callable in the API
> > and not
> >  > > serializable IgniteCallable, in the first place). What the action
> > does is
> >  > > entirely up to the user. One could fire off a remote compute if
> > that's what
> >  > > one wants.I hope I make myself clear.ThanksAndrey
> >  > > _
> >  > > From: Dmitriy Setrakyan <dsetrak...@apache.org>
> >  > > Sent: Saturday, January 16, 2016 7:54 AM
> >  > > Subject: Re: Semaphore action
> >  > > To:  <dev@ignite.apache.org>
> >  > >
> >  > >
> >  > >Andrey,
> >  > >
> >  > >  In general this seems like a good addition. However, I do not
> > understand
> >  > >  how you can specify an executor, given that execution could happen
> >  > >  remotely. Can you please clarify?
> >  > >
> >  > >  D.
> >

RE: Semaphore action

[Andrey Kornev]

Just to clarify, 7.4 below refers to GridGain 7.4, which is Ignite 1.4.
_
From: Andrey Kornev <andrewkor...@hotmail.com>
Sent: Sunday, January 17, 2016 10:27 AM
Subject: RE: Semaphore action
To:  <dev@ignite.apache.org>


   Vladisav,   

 It would be great if you could implement the enhancements!

 And while we're at it, here's something else I'd like us to consider. The 
proposed API only gets us half way there: there is no longer a blocking wait 
for the permit, but the action must be blocking. I really think it should be 
possible to do the whole thing asynchronously including the action and release 
of the permit. (In fact, this is what I need for my project. In 7.4, I was able 
to implement this feature using just the public APIs, but I'd really like to 
drop that code and start using the Semaphore API.)   

 For example, upon the permit acquisition, the action could fire off a 
distributed compute and rather than block waiting for the results to come back, 
it would simply return the future provided by Ignite's withAsync(). The 
semaphore implementation would not release the permit immediately upon the 
action return. Instead, it'd do so only when the action future completes.   

 Here's how the API might look like:   

  IgniteFuture acquireAndExecuteAsyncAction(Callable<IgniteFuture> 
action);   

 This API makes it possible to execute a protected action with no blocking 
whatsoever! It's a lot more efficient, reduces the pressure on Ignite's thread 
pools and prevents thread starvation, and so on.   

 Regards   
 Andrey   

 > Date: Sun, 17 Jan 2016 10:40:53 +0100   
 > Subject: Re: Semaphore action   
 > From: vladis...@gmail.com   
 > To: dev@ignite.apache.org   
 >
 > It does sounds like a useful addition,   
 > and I believe it could be implemented easily.   
 > I can do it if the community agrees on changing the API of the Semaphore.   
 >
 > Best regards,   
 > VJ   
 >
 > On Sun, Jan 17, 2016 at 5:53 AM, Andrey Kornev <andrewkor...@hotmail.com>   
 > wrote:   
 >
 > > Dmitriy,I don't believe it makes sense to have the action execute   
 > > remotely. At least I didn't think of it.In my mind, the action is always   
 > > local (that's the reason I suggested the Java's Callable in the API and 
 > > not   
 > > serializable IgniteCallable, in the first place). What the action does is  
 > >  
 > > entirely up to the user. One could fire off a remote compute if that's 
 > > what   
 > > one wants.I hope I make myself clear.ThanksAndrey   
 > > _________   
 > > From: Dmitriy Setrakyan <dsetrak...@apache.org>   
 > > Sent: Saturday, January 16, 2016 7:54 AM   
 > > Subject: Re: Semaphore action   
 > > To:  <dev@ignite.apache.org>   
 > >   
 > >   
 > >    Andrey,   
 > >   
 > >  In general this seems like a good addition. However, I do not understand  
 > > 
 > >  how you can specify an executor, given that execution could happen   
 > >  remotely. Can you please clarify?   
 > >   
 > >  D.   
 > >   
 > >  On Fri, Jan 15, 2016 at 1:00 PM, Andrey Kornev <andrewkor...@hotmail.com> 
 > >  
 > >  wrote:   
 > >   
 > >  > Hi there,   
 > >  >   
 > >  > The Semaphore feature was a great addition to Ignite's synchronization  
 > > 
 > >  > primitive toolkit. I'd like to propose an enhancement to make Semaphore 
 > >  
 > > API   
 > >  > even more useful.   
 > >  >   
 > >  > My biggest complaint about the current API is its blocking nature. For  
 > > 
 > >  > example, the only way to acquire a permit is by using the blocking   
 > >  > acquire() method (tryAcquire() is not very useful in the cases when you 
 > >  
 > >  > MUST acquire before you can proceed). I believe that in the 21st 
 > >century   
 > >  > blocking APIs is anachronism. :))) It's a bit embarrassing even. I'm   
 > > sure   
 > >  > Ignite can do better than that! :)   
 > >  >   
 > >  > Usually a permit acquisition is followed by an action, followed by a   
 > >  > release of the permit. I propose a simple enhancement to the Semaphore  
 > > 
 > > API   
 > >  > that will reduce the boilerplate and make it possible to do all of that 
 > >  
 > >  > asynchronously! The new method might look like this:   
 > >  >   
 > >  >  IgniteFuture acquireAndExecute(Callable action);   
 > >  >  IgniteFuture acquireAndExecute(Callable action, Executor   
 > >  > executor);   
 > >  &

RE: Semaphore action

[Andrey Kornev]

Vladisav,

It would be great if you could implement the enhancements! 

And while we're at it, here's something else I'd like us to consider. The 
proposed API only gets us half way there: there is no longer a blocking wait 
for the permit, but the action must be blocking. I really think it should be 
possible to do the whole thing asynchronously including the action and release 
of the permit. (In fact, this is what I need for my project. In 7.4, I was able 
to implement this feature using just the public APIs, but I'd really like to 
drop that code and start using the Semaphore API.)

For example, upon the permit acquisition, the action could fire off a 
distributed compute and rather than block waiting for the results to come back, 
it would simply return the future provided by Ignite's withAsync(). The 
semaphore implementation would not release the permit immediately upon the 
action return. Instead, it'd do so only when the action future completes.

Here's how the API might look like:

 IgniteFuture acquireAndExecuteAsyncAction(Callable<IgniteFuture> 
action);

This API makes it possible to execute a protected action with no blocking 
whatsoever! It's a lot more efficient, reduces the pressure on Ignite's thread 
pools and prevents thread starvation, and so on.

Regards
Andrey

> Date: Sun, 17 Jan 2016 10:40:53 +0100
> Subject: Re: Semaphore action
> From: vladis...@gmail.com
> To: dev@ignite.apache.org
> 
> It does sounds like a useful addition,
> and I believe it could be implemented easily.
> I can do it if the community agrees on changing the API of the Semaphore.
> 
> Best regards,
> VJ
> 
> On Sun, Jan 17, 2016 at 5:53 AM, Andrey Kornev <andrewkor...@hotmail.com>
> wrote:
> 
> > Dmitriy,I don't believe it makes sense to have the action execute
> > remotely. At least I didn't think of it.In my mind, the action is always
> > local (that's the reason I suggested the Java's Callable in the API and not
> > serializable IgniteCallable, in the first place). What the action does is
> > entirely up to the user. One could fire off a remote compute if that's what
> > one wants.I hope I make myself clear.ThanksAndrey
> > _
> > From: Dmitriy Setrakyan <dsetrak...@apache.org>
> > Sent: Saturday, January 16, 2016 7:54 AM
> > Subject: Re: Semaphore action
> > To:  <dev@ignite.apache.org>
> >
> >
> >Andrey,
> >
> >  In general this seems like a good addition. However, I do not understand
> >  how you can specify an executor, given that execution could happen
> >  remotely. Can you please clarify?
> >
> >  D.
> >
> >  On Fri, Jan 15, 2016 at 1:00 PM, Andrey Kornev <andrewkor...@hotmail.com>
> >  wrote:
> >
> >  > Hi there,
> >  >
> >  > The Semaphore feature was a great addition to Ignite's synchronization
> >  > primitive toolkit. I'd like to propose an enhancement to make Semaphore
> > API
> >  > even more useful.
> >  >
> >  > My biggest complaint about the current API is its blocking nature. For
> >  > example, the only way to acquire a permit is by using the blocking
> >  > acquire() method (tryAcquire() is not very useful in the cases when you
> >  > MUST acquire before you can proceed). I believe that in the 21st century
> >  > blocking APIs is anachronism. :))) It's a bit embarrassing even. I'm
> > sure
> >  > Ignite can do better than that! :)
> >  >
> >  > Usually a permit acquisition is followed by an action, followed by a
> >  > release of the permit. I propose a simple enhancement to the Semaphore
> > API
> >  > that will reduce the boilerplate and make it possible to do all of that
> >  > asynchronously! The new method might look like this:
> >  >
> >  >  IgniteFuture acquireAndExecute(Callable action);
> >  >  IgniteFuture acquireAndExecute(Callable action, Executor
> >  > executor);
> >  >
> >  > (The name could be improved, of course...) The first method would
> >  > immediately return a future to be later completed by the action's
> > result.
> >  > The action will be executed on a thread from the Ignite's public thread
> >  > pool once a permit has been acquired. Optionally, the user could
> > specify a
> >  > different executor using the second signature.
> >  >
> >  > Would the community be interested in such enhancement?
> >  >
> >  > Thanks
> >  > Andrey
> >  >
> >  > PS. A subject for a separate discussion, but I believe that in the next
> >  > major release Ignite should just drop the clumsy withAsync() API and
> > make
> >  > all blocking calls explicitly async by returning a future. In my
> > opinion it
> >  > would greatly simplify the API and make it more modern. Besides, I'm
> > pretty
> >  > sure that internally Ignite already creates futures for pretty much
> > most of
> >  > the API calls, so it should not cause any performance issue.
> >  >
> >
> >
> >
> >
> >
  

Re: Semaphore action

[Dmitriy Setrakyan]

Andrey,

In general this seems like a good addition. However, I do not understand
how you can specify an executor, given that execution could happen
remotely. Can you please clarify?

D.

On Fri, Jan 15, 2016 at 1:00 PM, Andrey Kornev 
wrote:

> Hi there,
>
> The Semaphore feature was a great addition to Ignite's synchronization
> primitive toolkit. I'd like to propose an enhancement to make Semaphore API
> even more useful.
>
> My biggest complaint about the current API is its blocking nature. For
> example, the only way to acquire a permit is by using the blocking
> acquire() method (tryAcquire() is not very useful in the cases when you
> MUST acquire before you can proceed). I believe that in the 21st century
> blocking APIs is anachronism. :))) It's a bit embarrassing even. I'm sure
> Ignite can do better than that! :)
>
> Usually a permit acquisition is followed by an action, followed by a
> release of the permit. I propose a simple enhancement to the Semaphore API
> that will reduce the boilerplate and make it possible to do all of that
> asynchronously! The new method might look like this:
>
>  IgniteFuture acquireAndExecute(Callable action);
>  IgniteFuture acquireAndExecute(Callable action, Executor
> executor);
>
> (The name could be improved, of course...) The first method would
> immediately return a future to be later completed by the action's result.
> The action will be executed on a thread from the Ignite's public thread
> pool once a permit has been acquired. Optionally, the user could specify a
> different executor using the second signature.
>
> Would the community be interested in such enhancement?
>
> Thanks
> Andrey
>
> PS. A subject for a separate discussion, but I believe that in the next
> major release Ignite should just drop the clumsy withAsync() API and make
> all blocking calls explicitly async by returning a future. In my opinion it
> would greatly simplify the API and make it more modern. Besides, I'm pretty
> sure that internally Ignite already creates futures for pretty much most of
> the API calls, so it should not cause any performance issue.
>