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 <T> IgniteFuture<T> acquireAndExecute(Callable<T>
> 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<T> acquireAndExecuteAsyncAction(Callable<IgniteFuture<T>>
> action); can be implemented with <T> IgniteFuture<T>
> acquireAndExecute(Callable<T> 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:
> >
> > <T> IgniteFuture<T>
> > acquireAndExecuteAsyncAction(Callable<IgniteFuture<T>> 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:
> > > > >
> > > > > <T> IgniteFuture<T> acquireAndExecute(Callable<T> action);
> > > > > <T> IgniteFuture<T> acquireAndExecute(Callable<T> 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.
> > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> >
> >
> >
> >
> >
