Hello!
> Let's say a user started a compute with fut = compute.runAsync(task);
> and now calls fut.complete(someVal); Does this mean that Ignite no longer
needs to execute the task?
> If the task is currently running, does it need to be canceled?
Yes, this case looks like Ignite should cancel computations because a user
wants to complete the future. Why not?
If there will be an opportunity to cancel a future, why is it a bad option
to finish a future through a complete() method?
> If you look at Ignite-2 code, you may found a number of places where we
return e.g. exchange futures or partition release futures.
> Assume the impact if we will return CompletableFuture instead, which can
be completed in 3-rd party plugin by mistake?
If exchange futures or partition release futures can be returned to 3-rd
party plugin by mistake, it is poor encapsulation.
And if it will be IgniteFuter rather than CompletedFuture, anyway, this can
harm.
вт, 30 мар. 2021 г. в 13:14, Andrey Mashenkov <andrey.mashen...@gmail.com>:
> Guys,
>
> I want to remember there is one more point to pay attention to.
> Extending Future and CompletableStage is more than just prevents unexpected
> behavior if a user completed the future.
>
> First of all, it helps us to write safer code as we won't a method contract
> exposed such methods as to a user as to a developer.
> If you look at Ignite-2 code, you may found a number of places where we
> return e.g. exchange futures or partition release futures.
> Assume the impact if we will return CompletableFuture instead, which can be
> completed in 3-rd party plugin by mistake?
>
> The suggested approach allows us to don't bother if a CompletableFuture has
> to be wrapped or not.
>
>
> On Tue, Mar 30, 2021 at 12:22 PM Alexey Goncharuk <
> alexey.goncha...@gmail.com> wrote:
>
> > Ivan,
> >
> > My concern with the concept of a user completing the future returned from
> > Ignite public API is that it is unclear how to interpret this action
> (this
> > backs Val's message).
> > Let's say a user started a compute with fut = compute.runAsync(task); and
> > now calls fut.complete(someVal); Does this mean that Ignite no longer
> needs
> > to execute the task? If the task is currently running, does it need to be
> > canceled?
> >
> > Using CompletableFuture.anyOf() is a good instrument in this case because
> > it makes the 'first future wins' contract explicit in the code. Besides
> > that, the point regarding the cancel() method is valid, and we will need
> > some custom mechanics to cancel a computation, so a custom interface that
> > simply extends both Future and CompletableStage seems reasonable to me.
> >
> > --AG
> >
> > пн, 29 мар. 2021 г. в 09:12, Ivan Pavlukhin <vololo...@gmail.com>:
> >
> > > Val,
> > >
> > > There were enough hype around Reactive programming past years. I
> > > remind a lot of talks about RxJava. And I suppose it worth to consider
> > > it. But it requires some time to study modern trends to make a choice.
> > > So far I am not ready to facilitate Reactive API for Ignite 3.
> > >
> > > Regarding CompletableFuture.
> > >
> > > > The point is that currently a future returned from any of Ignite's
> > async
> > > > operations is supposed to be completed with a value only by Ignite
> > > itself,
> > > > not by the user. If we follow the same approach in Ignite 3,
> returning
> > > > CompletableFuture is surely wrong in my view.
> > >
> > > My first thoughts was similar. But later I thought what a user would
> > > like do with returned future. And one of cases I imagined was a case
> > > of alternative result. E.g. a user uses Ignite and another data source
> > > in his application. He wants to use a value arrived faster. He
> > > combines 2 futures like CompletableFuture.anyOf(...). Consequently
> > > even if we prohibit CompletableFuture.complete(...) explicitly then it
> > > will be possible to create a combination that will allow premature
> > > future completion. After all generally CompletableFuture is a
> > > placeholder for async computaion result and if a user wants to
> > > substitute result returned from Ignite why should we disallow him to
> > > do it?
> > >
> > > Also I found one more suspicious thing with CompletableFuture. As it
> > > is a concrete class it implements a cancel() method. And as I see the
> > > implementation does not try to cancel underlying computations. Is not
> > > it a problem?
> > >
> > > 2021-03-29 7:30 GMT+03:00, Valentin Kulichenko <
> > > valentin.kuliche...@gmail.com>:
> > > > Ivan,
> > > >
> > > > It's not really about the "harm", but more about "what should we do
> if
> > > this
> > > > method is called?". Imagine the following code:
> > > >
> > > > CompletableFuture<String> fut = cache.getAsync(key);
> > > > fut.complete("something");
> > > >
> > > > What should happen in this case?
> > > >
> > > > The point is that currently a future returned from any of Ignite's
> > async
> > > > operations is supposed to be completed with a value only by Ignite
> > > itself,
> > > > not by the user. If we follow the same approach in Ignite 3,
> returning
> > > > CompletableFuture is surely wrong in my view.
> > > >
> > > > At the same time, if we take a fundamentally different route with the
> > > async
> > > > APIs, this whole discussion might become irrelevant. For example, can
> > you
> > > > elaborate on your thinking around the reactive API? Do you have any
> > > > specifics in mind?
> > > >
> > > > -Val
> > > >
> > > > On Sat, Mar 27, 2021 at 9:18 PM Ivan Pavlukhin <vololo...@gmail.com>
> > > wrote:
> > > >
> > > >> > The methods below shouldn't be accessible for user:
> > > >> > complete()
> > > >> > completeExceptionaly()
> > > >>
> > > >> Folks, in case of user-facing API, do you think there is a real harm
> > > >> in allowing a user to manually "complete" a future? I suppose a user
> > > >> employs some post-processing for future results and potentially
> wants
> > > >> to have control of these results as well. E.g. premature completion
> in
> > > >> case when a result is no longer needed is possible usage.
> > > >>
> > > >> Also I thinkg it might be a good time to ponder about Future/Promise
> > > >> APIs in general. Why such API is our choice? Can we choose e.g.
> > > >> Reactive API style instead?
> > > >>
> > > >> 2021-03-27 0:33 GMT+03:00, Valentin Kulichenko <
> > > >> valentin.kuliche...@gmail.com>:
> > > >> > Andrey,
> > > >> >
> > > >> > I see. So in a nutshell, you're saying that we want to return a
> > future
> > > >> that
> > > >> > the user's code is not allowed to complete. In this case, I think
> > it's
> > > >> > clear that CompletableFuture is not what we need. We actually
> need a
> > > >> > NonCompletableFuture :)
> > > >> >
> > > >> > My vote is for the custom interface.
> > > >> >
> > > >> > -Val
> > > >> >
> > > >> > On Fri, Mar 26, 2021 at 2:25 AM Andrey Mashenkov
> > > >> > <andrey.mashen...@gmail.com>
> > > >> > wrote:
> > > >> >
> > > >> >> Val,
> > > >> >>
> > > >> >> The methods below shouldn't be accessible for user:
> > > >> >> complete()
> > > >> >> completeExceptionaly()
> > > >> >>
> > > >> >> Returning CompletableFuture we must always make a copy to prevent
> > the
> > > >> >> original future from being completed by mistake.
> > > >> >> I think it will NOT be enough to do that returing the future to
> the
> > > >> >> end-user, but from every critical module to the outside of the
> > > module,
> > > >> >> e.g. to plugins. The impact of disclosing ExchangeFuture,
> > > >> >> PartitionReleaseFuture to plugins may be serious.
> > > >> >>
> > > >> >> IgniteFuture<T> extends Future<T>, CompletionStage<T> which
> > > >> >> implementation
> > > >> >> will just wrap CompletableFuture these issues will be resolved in
> > > >> natural
> > > >> >> way.
> > > >> >> In addition we can force toCompletableFuture() method to return a
> > > >> >> defensive
> > > >> >> copy(), that resolves the last concern.
> > > >> >>
> > > >> >>
> > > >> >> On Fri, Mar 26, 2021 at 11:38 AM Konstantin Orlov
> > > >> >> <kor...@gridgain.com>
> > > >> >> wrote:
> > > >> >>
> > > >> >> > CompletableFuture seems a better option to me.
> > > >> >> >
> > > >> >> > --
> > > >> >> > Regards,
> > > >> >> > Konstantin Orlov
> > > >> >> >
> > > >> >> >
> > > >> >> >
> > > >> >> >
> > > >> >> > > On 26 Mar 2021, at 11:07, Pavel Tupitsyn <
> ptupit...@apache.org
> > >
> > > >> >> > > wrote:
> > > >> >> > >
> > > >> >> > > On the one hand, I agree with Alexey.
> > > >> >> > > CompletableFuture has complete* methods which should not be
> > > >> available
> > > >> >> to
> > > >> >> > > the user code.
> > > >> >> > > This can be solved with a simple interface like we do in Thin
> > > >> Client:
> > > >> >> > > IgniteClientFuture<T> extends Future<T>, CompletionStage<T>
> > > >> >> > >
> > > >> >> > >
> > > >> >> > > On the other hand:
> > > >> >> > > - CompletionStage has toCompletableFuture anyway (rather
> weird)
> > > >> >> > > - Other libraries use CompletableFuture and it seems to be
> fine
> > > >> >> > > - Using CompletableFuture is the simplest approach
> > > >> >> > >
> > > >> >> > >
> > > >> >> > > So I lean towards CompletableFuture too.
> > > >> >> > >
> > > >> >> > > On Fri, Mar 26, 2021 at 10:46 AM Alexey Kukushkin <
> > > >> >> > kukushkinale...@gmail.com>
> > > >> >> > > wrote:
> > > >> >> > >
> > > >> >> > >> I do not like Java's CompletableFuture and prefer our own
> > Future
> > > >> >> > (revised
> > > >> >> > >> IgniteFuture).
> > > >> >> > >>
> > > >> >> > >> My understanding of the Future (or Promise) pattern in
> general
> > > is
> > > >> >> having
> > > >> >> > >> two separate APIs:
> > > >> >> > >>
> > > >> >> > >> 1. Server-side: create, set result, raise error, cancel
> from
> > > >> >> > >> server.
> > > >> >> > >> 2. Client-side: get result, handle error, cancel from
> client
> > > >> >> > >>
> > > >> >> > >> Java's CompletableFuture looks like both the client-side and
> > > >> >> > >> server-side API. The "Completeable" prefix in the name is
> > > already
> > > >> >> > confusing
> > > >> >> > >> for a client since it cannot "complete" an operation, only a
> > > >> >> > >> server
> > > >> >> can.
> > > >> >> > >>
> > > >> >> > >> I would create our own IgniteFuture adding client-side
> > > >> functionality
> > > >> >> we
> > > >> >> > >> currently miss (like client-side cancellation).
> > > >> >> > >>
> > > >> >> > >>
> > > >> >> > >> пт, 26 мар. 2021 г. в 01:08, Valentin Kulichenko <
> > > >> >> > >> valentin.kuliche...@gmail.com>:
> > > >> >> > >>
> > > >> >> > >>> Andrey,
> > > >> >> > >>>
> > > >> >> > >>> Can you compile a full list of these risky methods, and
> > > >> >> > >>> elaborate
> > > >> >> > >>> on
> > > >> >> > what
> > > >> >> > >>> the risks are?
> > > >> >> > >>>
> > > >> >> > >>> Generally, CompletableFuture is a much better option,
> because
> > > >> >> > >>> it's
> > > >> >> > >>> standard. But we need to make sure it actually fits our
> needs
> > > >> >> > >>> and
> > > >> >> > doesn't
> > > >> >> > >>> do more harm than good.
> > > >> >> > >>>
> > > >> >> > >>> -Val
> > > >> >> > >>>
> > > >> >> > >>> On Thu, Mar 25, 2021 at 12:23 PM Alexei Scherbakov <
> > > >> >> > >>> alexey.scherbak...@gmail.com> wrote:
> > > >> >> > >>>
> > > >> >> > >>>> I think both options are fine, but personally lean toward
> > > >> >> > >>>> CompletableFuture.
> > > >> >> > >>>>
> > > >> >> > >>>> чт, 25 мар. 2021 г. в 17:56, Atri Sharma <a...@apache.org
> >:
> > > >> >> > >>>>
> > > >> >> > >>>>> I would suggest using CompletableFuture -- I don't see a
> > need
> > > >> for
> > > >> >> > >>>>> a
> > > >> >> > >>>> custom
> > > >> >> > >>>>> interface that is unique to us.
> > > >> >> > >>>>>
> > > >> >> > >>>>> It also allows a lower barrier for new contributors for
> > > >> >> understanding
> > > >> >> > >>>>> existing code
> > > >> >> > >>>>>
> > > >> >> > >>>>> On Thu, 25 Mar 2021, 20:18 Andrey Mashenkov, <
> > > >> >> > >>> andrey.mashen...@gmail.com
> > > >> >> > >>>>>
> > > >> >> > >>>>> wrote:
> > > >> >> > >>>>>
> > > >> >> > >>>>>> Hi Igniters,
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> I'd like to start a discussion about replacing our
> custom
> > > >> >> > >>> IgniteFuture
> > > >> >> > >>>>>> class with CompletableFuture - existed JDK class
> > > >> >> > >>>>>> or rework it's implementation (like some other products
> > > done)
> > > >> to
> > > >> >> > >>>>>> a
> > > >> >> > >>>>>> composition of CompletionStage and Future interfaces.
> > > >> >> > >>>>>> or maybe other option if you have any ideas. Do you?
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> 1. The first approach pros and cons are
> > > >> >> > >>>>>> + Well-known JDK class
> > > >> >> > >>>>>> + Already implemented
> > > >> >> > >>>>>> - It is a class, not an interface.
> > > >> >> > >>>>>> - Expose some potentially harmful methods like
> > "complete()".
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> On the other side, it has copy() method to create
> > defensive
> > > >> copy
> > > >> >> > >> and
> > > >> >> > >>>>>> minimalCompletionStage() to restrict harmful method
> usage.
> > > >> >> > >>>>>> Thus, this look like an applicable solution, but we
> should
> > > be
> > > >> >> > >> careful
> > > >> >> > >>>>>> exposing internal future to the outside.
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> 2. The second approach is to implement our own interface
> > > like
> > > >> >> > >>>>>> the
> > > >> >> > >>> next
> > > >> >> > >>>>> one:
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> interface IgniteFuture<T> extends CompletableStage<T>,
> > > >> Future<T>
> > > >> >> > >>>>>> {
> > > >> >> > >>>>>> }
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> Pros and cons are
> > > >> >> > >>>>>> + Our interfaces/classes contracts will expose an
> > interface
> > > >> >> > >>>>>> rather
> > > >> >> > >>> than
> > > >> >> > >>>>>> concrete implementation.
> > > >> >> > >>>>>> + All methods are safe.
> > > >> >> > >>>>>> - Some implementation is required.
> > > >> >> > >>>>>> - CompletableStage has a method toCompletableFuture()
> and
> > > can
> > > >> be
> > > >> >> > >>>>> converted
> > > >> >> > >>>>>> to CompletableFuture. This should be supported.
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> However, we still could wrap CompletableFuture and don't
> > > >> >> > >>>>>> bother
> > > >> >> > >> about
> > > >> >> > >>>>>> creating a defensive copy.
> > > >> >> > >>>>>>
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> Other project experience:
> > > >> >> > >>>>>> * Spotify uses CompletableFuture directly [1].
> > > >> >> > >>>>>> * Redis goes the second approach [2]
> > > >> >> > >>>>>> * Vertx explicitly extends CompletableFuture [3].
> However,
> > > >> >> > >>>>>> they
> > > >> >> > >> have
> > > >> >> > >>>>> custom
> > > >> >> > >>>>>> future classes and a number of helpers that could be
> > > replaced
> > > >> >> > >>>>>> with
> > > >> >> > >>>>>> CompletableStage. Maybe it is just a legacy.'
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> Any thoughts?
> > > >> >> > >>>>>>
> > > >> >> > >>>>>> [1]
> > > >> >> > >>>>>>
> > > >> >> > >>>>>>
> > > >> >> > >>>>>
> > > >> >> > >>>>
> > > >> >> > >>>
> > > >> >> > >>
> > > >> >> >
> > > >> >>
> > > >>
> > >
> >
> https://spotify.github.io/completable-futures/apidocs/com/spotify/futures/ConcurrencyReducer.html
> > > >> >> > >>>>>> [2]
> > > >> >> > >>>>>>
> > > >> >> > >>>>>>
> > > >> >> > >>>>>
> > > >> >> > >>>>
> > > >> >> > >>>
> > > >> >> > >>
> > > >> >> >
> > > >> >>
> > > >>
> > >
> >
> https://lettuce.io/lettuce-4/release/api/com/lambdaworks/redis/RedisFuture.html
> > > >> >> > >>>>>> [3]
> > > >> >> > >>>>>>
> > > >> >> > >>>>>>
> > > >> >> > >>>>>
> > > >> >> > >>>>
> > > >> >> > >>>
> > > >> >> > >>
> > > >> >> >
> > > >> >>
> > > >>
> > >
> >
> https://javadoc.io/static/org.jspare.vertx/vertx-jspare/1.1.0-M03/org/jspare/vertx/concurrent/VertxCompletableFuture.html
> > > >> >> > >>>>>> --
> > > >> >> > >>>>>> Best regards,
> > > >> >> > >>>>>> Andrey V. Mashenkov
> > > >> >> > >>>>>>
> > > >> >> > >>>>>
> > > >> >> > >>>>
> > > >> >> > >>>>
> > > >> >> > >>>> --
> > > >> >> > >>>>
> > > >> >> > >>>> Best regards,
> > > >> >> > >>>> Alexei Scherbakov
> > > >> >> > >>>>
> > > >> >> > >>>
> > > >> >> > >>
> > > >> >> > >>
> > > >> >> > >> --
> > > >> >> > >> Best regards,
> > > >> >> > >> Alexey
> > > >> >> > >>
> > > >> >> >
> > > >> >> >
> > > >> >>
> > > >> >> --
> > > >> >> Best regards,
> > > >> >> Andrey V. Mashenkov
> > > >> >>
> > > >> >
> > > >>
> > > >>
> > > >> --
> > > >>
> > > >> Best regards,
> > > >> Ivan Pavlukhin
> > > >>
> > > >
> > >
> > >
> > > --
> > >
> > > Best regards,
> > > Ivan Pavlukhin
> > >
> >
>
>
> --
> Best regards,
> Andrey V. Mashenkov
>
