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
>
