>
> 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?
Future has cancel() method for this.
Completing future from outside will never respect other subscribers that
may expect other guatantees.
>
> 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.
>
IgniteFuture contract will not have complete() or similar methods, but we a
free to pass any implementation.
Obviously, if a user casts the future to CompletableFuture or uses a
reflection to access IgniteFutureImpl hidden methods -
that means the user breaks the contract and CompletableFuture defensive
copy will not help either.
On Tue, Mar 30, 2021 at 2:28 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
> >
>
--
Best regards,
Andrey V. Mashenkov
