I've changed the IEP and added a new future interface to the POC: interface IgniteClientFuture<T> extends Future<T>, CompletionStage<T>
The implementation simply wraps the CompletableFuture. On Fri, Aug 21, 2020 at 11:22 PM Valentin Kulichenko < [email protected]> wrote: > Hi Pavel, > > Interesting findings :) Agree that we should not use the CompletableFuture > - it clearly has a different purpose. > > I think that the approach taken by Redis makes more sense. I don't like > that it requires a custom interface, but I think we can live with that. > > I would be glad to hear other opinions though. > > -Val > > On Fri, Aug 21, 2020 at 1:02 AM Pavel Tupitsyn <[email protected]> > wrote: > > > Val, > > > > The problems with CompletableFuture in public API are: > > * It is a class, not an interface > > * It is completable - anyone can call .complete(), which is not what we > > want > > > > There seems to be no clear guidance in Java world on async API design; > > however, it is often recommended to return CompletionStage instead of > > CompletableFuture > > from the public APIs [1] [2], and some products follow this [3]. > > > > Other products return their own future interface that extends both Future > > and CompletionStage, > > which seems to be a better alternative to me [4]. > > > > Thoughts? > > > > [1] > > > > > https://stackoverflow.com/questions/47571117/what-is-the-difference-between-completionstage-and-completablefuture > > [2] > > > > > https://stackoverflow.com/questions/34930840/should-i-return-completablefuture-or-future-when-defining-api > > < > > > https://stackoverflow.com/questions/34930840/should-i-return-completablefuture-or-future-when-defining-api#:~:text=by%20returning%20a%20CompletableFuture%2C%20you,API%2C%20which%20is%20not%20good > > .> > > [3] > > > > > https://docs.hazelcast.org/docs/latest/javadoc/com/hazelcast/cache/ICache.html > > [4] > > > > > https://lettuce.io/lettuce-4/release/api/com/lambdaworks/redis/RedisFuture.html > > > > On Fri, Aug 21, 2020 at 10:28 AM Alex Plehanov <[email protected]> > > wrote: > > > > > Pavel, > > > > > > Thanks for the discussion, I've also faced with the necessity of having > > > async calls while implementing POC for thin client data streamer [1] > and > > > solve it similarly (but in my case it's required only for internal > > > implementation, so I've only changed the internal API). > > > > > > I want to note that described in IEP approach (and implemented in POC) > is > > > not fully async, since "send" is still used in the user's thread. To > make > > > it fully async we need additional sending thread (since blocking IO is > > used > > > for communication with the server). If partition awareness is enabled > > there > > > will be 2 threads per each server connection, perhaps we should think > > about > > > moving to NIO and introducing some kind of communication thread pool. > > > > > > [1]: https://github.com/apache/ignite/pull/8175 > > > > > > пт, 21 авг. 2020 г. в 03:35, Valentin Kulichenko < > > > [email protected]>: > > > > > > > Sounds good. I've added this to the 3.0 roadmap: > > > > https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+3.0 > > > > > > > > Unless there are any objections from others, let's stick with the > > > > CompletableFuture for any future development, including the thin > > client. > > > > > > > > -Val > > > > > > > > On Thu, Aug 20, 2020 at 9:30 AM Pavel Tupitsyn <[email protected] > > > > > > wrote: > > > > > > > > > Val, no objections from my side. > > > > > As noted above, the only benefit of IgniteFuture is consistency > > across > > > > > thin/thick APIs, > > > > > which is probably not so important. > > > > > > > > > > On Thu, Aug 20, 2020 at 6:28 PM Valentin Kulichenko < > > > > > [email protected]> wrote: > > > > > > > > > > > Hi Pavel, > > > > > > > > > > > > Are there any benefits of IgniteFuture over CompletableFuture? > > > > > > > > > > > > IgniteFuture was created long ago, during the time when > > > > CompletableFuture > > > > > > did not exist. There is a big chance that IgniteFuture actually > > > became > > > > > > redundant at the moment we transitioned to Java8. If that's the > > > case, I > > > > > > would prefer using CompletableFuture in the thin client and > getting > > > rid > > > > > of > > > > > > IgniteFuture altogether in 3.0. > > > > > > > > > > > > What do you think? > > > > > > > > > > > > -Val > > > > > > > > > > > > On Thu, Aug 20, 2020 at 7:19 AM Pavel Tupitsyn < > > [email protected] > > > > > > > > > > wrote: > > > > > > > > > > > > > Igniters, > > > > > > > > > > > > > > I've prepared an IEP [1], please review and let me know what > you > > > > think. > > > > > > > > > > > > > > In particular, I'd like to discuss the Future interface to be > > used: > > > > > > > * IgniteFuture is the first candidate - Thin APIs will be > > > consistent > > > > > with > > > > > > > Thick APIs, probably better for existing Ignite users. > > > > > > > * CompletableFuture is the standard for async Java APIs. Many > > users > > > > may > > > > > > > prefer that instead of a custom IgniteFuture. > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-51%3A+Java+Thin+Client+Async+API > > > > > > > > > > > > > > > > > > > > > > > > > > > >
