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

Reply via email to