keith-turner commented on PR #2967: URL: https://github.com/apache/accumulo/pull/2967#issuecomment-1264717379
> I'm not sure it's necessary to name the method "Async()" when it returns a Future. I think that's implied. For that I was thinking its in Async method among many sync methods and using the return type is optional. So wanted to make sure anyone using knew that when the method returned it may not be done. > I think if we start adding async APIs, we should default to the ForkJoinPool, and provide some other mechanism to set an executor at the time the AccumuloClient is constructed, for use with that client, and shut down when that client is closed. I am not sure scoping the executor at the client is good. Many methods on Completable future have a version that takes an Executor and one that does not. This allows developer control over limiting how many cores an operation can use. I think this important if a developer wants to limit one scan to one core and another to 32 cores for example. If the executor was scoped to the accumulo client, then they would need to create two different clients for the scans. > My conclusion: Based on the experimentation you've done, and the state of this discussion, and thinking about how much work is still needed to design asynchronous APIs in a way that's consistent going forward, instead of just doing it as a one-off for this specific feature I agree. I also mentioned previously it would be good to do the async client API work at the same time as making the servers support async read/write pipelines so that the two could influence each in a development cycle. This is another reason to defer this to 3.0. Another little thing that I noticed while experimenting with an async API for this PR is that our existing checked exceptions would be wrapped by something like an ExecutionException when using futures. Its a very nuanced thing, but exceptions may be something else to consider in the design of async APIs. I noticed that ComletableFuture introduced a new method called [join()](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/CompletableFuture.html#join()) that is similar to [get()](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/CompletableFuture.html#get()) except it has no checked exceptions. I really like it, so much easier to use. Makes me wonder if there is a wider move away from checked exceptions in the Java APIs because they are painful to use w/ lambdas. I think I will revert the experimental commmit I did and then do another commit to add the map return type. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
