tombentley commented on a change in pull request #9878:
URL: https://github.com/apache/kafka/pull/9878#discussion_r663754330
##########
File path:
clients/src/main/java/org/apache/kafka/common/internals/KafkaFutureImpl.java
##########
@@ -267,50 +180,82 @@ public T get() throws InterruptedException,
ExecutionException {
@Override
public T get(long timeout, TimeUnit unit) throws InterruptedException,
ExecutionException,
TimeoutException {
- SingleWaiter<T> waiter = new SingleWaiter<>();
- addWaiter(waiter);
- return waiter.await(timeout, unit);
+ try {
+ return completableFuture.get(timeout, unit);
+ } catch (ExecutionException e) {
+ maybeThrowCancellationException(e.getCause());
+ throw e;
+ }
}
/**
* Returns the result value (or throws any encountered exception) if
completed, else returns
* the given valueIfAbsent.
*/
@Override
- public synchronized T getNow(T valueIfAbsent) throws InterruptedException,
ExecutionException {
- if (exception != null)
- wrapAndThrow(exception);
- if (done)
- return value;
- return valueIfAbsent;
+ public synchronized T getNow(T valueIfAbsent) throws ExecutionException {
Review comment:
> Does it need to remove InterruptedException from other get methods
also?
`get()` and `get(long, TimeUnit)` can still throw `InterruptedException` due
to the blocking behaviour (the `completableFuture.get` that we delegate to
throws `InterruptedException`).
> For another, why we don't remove InterruptedException from
KafkaFuture#getNow?
Good spot! `KafkaFuture#getNow` should never have declared that and it's
annoying for callers to have to deal with it. But this wasn't described in the
KIP and wouldn't be a source compatible change (existing code with a `catch
(InterruptedException)` would get a compile error).
I'd be happy to remove it and notify the VOTE thread, WDYT @kkonstantine
@dajac ?
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
