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
>
