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
