> Completing future from outside will never respect other subscribers that
> may expect other guatantees.
For example, if we talk about public API like IgniteCache, what subscribers
may expect other guatantees?
IMHO, the best solution is to get the well-known standard interface to a
user, and he will be happy.
But when we talk about internal classes like "exchange future" they could
be custom futures if convenient.
вт, 30 мар. 2021 г. в 15:25, Atri Sharma <a...@apache.org>:
> IMO the only way Ignite should cancel computations is iff cancel method is
> invoked, not implicitly if complete is invoked.
>
> On Tue, 30 Mar 2021 at 4:58 PM, Denis Garus <garus....@gmail.com> wrote:
>
> > 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
> > >
> >
> --
> Regards,
>
> Atri
> Apache Concerted
>
