Hi Andrey,
Please see below.
-Val
On Wed, Mar 31, 2021 at 7:55 AM Andrey Mashenkov <andrey.mashen...@gmail.com>
wrote:
> CompletableFuture cancellation will not work as many users expected.
> Javadoc says:
> /* Since (unlike {@link FutureTask}) this class has no direct
> * control over the computation that causes it to be completed,
> * cancellation is treated as just another form of exceptional
> * completion.
> */
>
Let's not make assumptions about the expectations of the users. That's
exactly why I initially leaned towards a custom interface as well, but
that's a mistake.
Indeed, this contract might look weird to us, because the current version
of Ignite behaves differently. However, there are much more developers
using CompletableFuture and other standard async frameworks, than
developers using the async functionality of Ignite. Therefore, our
intuitions can easily be wrong.
> Completion of a child future doesn't trigger the completion of a parent.
> So, we will need to extend the future anyway.
>
First of all, as Pavel mentioned, you can attach your own listener before
returning a CompletableFuture to the user. You don't need to extend.
Second of all, there is still a discussion to be had on whether the parent
needs to be completed. I don't actually think it's obvious, and most likely
it's case by case. With CompletableFuture you have enough flexibility to
control the behavior.
>
> Also, cancel() method contract differs from IgniteFuture in 2.0,
> Completable method return true if the future was cancelled,
> but IgniteFuture return true only if it wasn't cancel prior the call.
> Thus, if cancel() was called twice we will have different results for
> CompletableFuture and IgniteFuture,
> that makes CompletableFuture barely usable for our internal purposes.
>
It doesn't really matter how IgniteFuture in 2.0 behaves. It was created
long before continuations and other async concepts became mainstream (in
Java world at least).
Also, we don't have to use it for internal purposes, of course. I'm only
talking about public APIs.
>
> BTW, CompletableFuture.anyOf() still can be used, see
> CompletionStage.toCompletableFuture() contract.
>
In my view, this actually kills the idea of a custom future. Basically,
the proposal is to introduce a custom entity to restrict access to some of
the CompletableFuture methods, but then allow to convert this custom entity
to a CompletableFuture that has all the methods. This makes zero sense to
me.
>
> The more I learn about CompletableFuture the stronger my opinion about
> overriding it.
>
>
> On Wed, Mar 31, 2021 at 9:31 AM Denis Garus <garus....@gmail.com> wrote:
>
> > > Stripping them from such functionality, which they are used to, is most
> > likely a bad idea.
> >
> > Completely agree with this point of view.
> > Moreover, a user can pass CompletableFuture to another library to do any
> > manipulations.
> > So if we want to introduce our class instead of the java class, we should
> > have solid arguments;
> > otherwise, it can be a reason for irritation.
> >
> > ср, 31 мар. 2021 г. в 09:06, Pavel Tupitsyn <ptupit...@apache.org>:
> >
> > > Val,
> > >
> > > > we can have an IgniteFuture that extends CompletableFuture.
> > > > This might be useful if want the cancel() operation to cancel the
> > > > underlying operation
> > >
> > > I think we can subscribe to the cancellation of the CompletableFuture
> > > and cancel the underlying operation without an extra class,
> > > something like
> > >
> > > fut.exceptionally(t -> {
> > > if (t instanceof CancellationException) {
> > > // Cancel Ignite operation
> > > }
> > > });
> > >
> > > On Wed, Mar 31, 2021 at 7:45 AM Valentin Kulichenko <
> > > valentin.kuliche...@gmail.com> wrote:
> > >
> > > > These are actually some interesting points. As I'm thinking more
> about
> > > > this, I'm leaning towards changing my opinion and voting for the
> > > > CompletableFuture. Here is my reasoning.
> > > >
> > > > First, it's important to keep in mind that CompletableFuture is not
> an
> > > > interface that we will implement, it's an implemented class.
> Therefore,
> > > > some of the concerns around complete() and cancel() method are not
> > really
> > > > relevant -- it's not up to us how these methods behave, they're
> already
> > > > implemented.
> > > >
> > > > Second, CompletableFuture does provide some useful functionality
> (anyOf
> > > is
> > > > one of the examples). I can even envision users wanting to complete
> the
> > > > future under certain circumstances, e.g. after a timeout, using
> > > > the completeOnTimeout method. Stripping them from such functionality,
> > > which
> > > > they are used to, is most likely a bad idea.
> > > >
> > > > And finally, we can have an IgniteFuture that extends
> > CompletableFuture.
> > > > This might be useful if want the cancel() operation to cancel the
> > > > underlying operation. This way we keep all the functionality of
> > > > CompletableFuture while keeping a certain amount of flexibility for
> > > > specific cases.
> > > >
> > > > Thoughts?
> > > >
> > > > -Val
> > > >
> > > >
> > > > On Tue, Mar 30, 2021 at 5:36 AM Denis Garus <garus....@gmail.com>
> > wrote:
> > > >
> > > > > > 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
> > > > > >
> > > > >
> > > >
> > >
> >
>
>
> --
> Best regards,
> Andrey V. Mashenkov
>
