[GitHub] [pulsar] haoyann commented on pull request #15321: [improve][client] Should use CompletableFuture#get(long, TimeUnit)

[GitBox <gi...@apache.org>]

haoyann commented on PR #15321:
URL: https://github.com/apache/pulsar/pull/15321#issuecomment-1110790396

   > I doubt if it's worth to do that. There are many places that also call `CompletableFuture#get` without the timeout. The synchronous call is used for where it is not performance sensitive.
   > 
   > There are two main concerns:
   > 
   > 1. This bug is already fixed in higher versions of JDK8.
   > 2. It's inconsistent to see somewhere `future.get(Integer.MAX_VALUE, TimeUnit.MILLISECONDS);`  is called and else `future.get()` is called.
   
   
   
   > 
   
   Yesh,I express my opinion
   1. `There are many places that also call CompletableFuture#get without the timeout` I don't think the server needs to be changed, because it is very convenient to upgrade jdk on the server. 
   2. There will be performance problems in the scenario of high concurrent calls. `The synchronous call is used for where it is not performance sensitive` If this condition is met, there is really no need to change.


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org