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