> 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
