Skip to content

Commit

Permalink
[cleanup] Catch TimeoutException when logging about time outs (apache…
Browse files Browse the repository at this point in the history
…#20349)

Relates to: apache#20299

### Motivation

We have several catch blocks that are dedicated to providing meaningful logs about timeouts. As such, we should catch the `TimeoutException`, not the `InterruptedException`, in order to ensure accurate logs.

### Modifications

* Replace `InterruptedException` with `TimeoutException`

### Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

### Documentation

- [x] `doc-not-needed`
  • Loading branch information
michaeljmarshall authored May 18, 2023
1 parent e4f1f09 commit 65f6112
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;
import javax.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils;
import org.apache.pulsar.broker.PulsarServerException;
Expand Down Expand Up @@ -221,7 +222,7 @@ public boolean canProduce(TopicName topicName, String role, AuthenticationDataSo
try {
return canProduceAsync(topicName, role, authenticationData).get(
conf.getMetadataStoreOperationTimeoutSeconds(), SECONDS);
} catch (InterruptedException e) {
} catch (TimeoutException e) {
log.warn("Time-out {} sec while checking authorization on {} ",
conf.getMetadataStoreOperationTimeoutSeconds(), topicName);
throw e;
Expand All @@ -237,7 +238,7 @@ public boolean canConsume(TopicName topicName, String role, AuthenticationDataSo
try {
return canConsumeAsync(topicName, role, authenticationData, subscription)
.get(conf.getMetadataStoreOperationTimeoutSeconds(), SECONDS);
} catch (InterruptedException e) {
} catch (TimeoutException e) {
log.warn("Time-out {} sec while checking authorization on {} ",
conf.getMetadataStoreOperationTimeoutSeconds(), topicName);
throw e;
Expand All @@ -263,7 +264,7 @@ public boolean canLookup(TopicName topicName, String role, AuthenticationDataSou
try {
return canLookupAsync(topicName, role, authenticationData)
.get(conf.getMetadataStoreOperationTimeoutSeconds(), SECONDS);
} catch (InterruptedException e) {
} catch (TimeoutException e) {
log.warn("Time-out {} sec while checking authorization on {} ",
conf.getMetadataStoreOperationTimeoutSeconds(), topicName);
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import javax.ws.rs.container.AsyncResponse;
import javax.ws.rs.core.Response;
Expand Down Expand Up @@ -771,7 +772,7 @@ && pulsar().getBrokerService().isAuthorizationEnabled()) {
isAuthorized = pulsar().getBrokerService().getAuthorizationService()
.allowTopicOperationAsync(topicName, TopicOperation.PRODUCE, authParams)
.get(config().getMetadataStoreOperationTimeoutSeconds(), SECONDS);
} catch (InterruptedException e) {
} catch (TimeoutException e) {
log.warn("Time-out {} sec while checking authorization on {} ",
config().getMetadataStoreOperationTimeoutSeconds(), topicName);
throw new RestException(Status.INTERNAL_SERVER_ERROR, "Time-out while checking authorization");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.Base64;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLongFieldUpdater;
import java.util.concurrent.atomic.LongAdder;
Expand Down Expand Up @@ -476,7 +477,7 @@ protected Boolean isAuthorized(String authRole, AuthenticationDataSource authent
return service.getAuthorizationService()
.allowTopicOperationAsync(topic, TopicOperation.CONSUME, authRole, subscription)
.get(service.getConfig().getMetadataStoreOperationTimeoutSeconds(), SECONDS);
} catch (InterruptedException e) {
} catch (TimeoutException e) {
log.warn("Time-out {} sec while checking authorization on {} ",
service.getConfig().getMetadataStoreOperationTimeoutSeconds(), topic);
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicLongFieldUpdater;
import java.util.concurrent.atomic.LongAdder;
import javax.servlet.http.HttpServletRequest;
Expand Down Expand Up @@ -248,7 +249,7 @@ protected Boolean isAuthorized(String authRole, AuthenticationDataSource authent
return service.getAuthorizationService()
.allowTopicOperationAsync(topic, TopicOperation.PRODUCE, authRole, authenticationData)
.get(service.getConfig().getMetadataStoreOperationTimeoutSeconds(), SECONDS);
} catch (InterruptedException e) {
} catch (TimeoutException e) {
log.warn("Time-out {} sec while checking authorization on {} ",
service.getConfig().getMetadataStoreOperationTimeoutSeconds(), topic);
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import java.io.IOException;
import java.util.Base64;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLongFieldUpdater;
import java.util.concurrent.atomic.LongAdder;
Expand Down Expand Up @@ -319,7 +320,7 @@ protected Boolean isAuthorized(String authRole, AuthenticationDataSource authent
return service.getAuthorizationService()
.allowTopicOperationAsync(topic, TopicOperation.CONSUME, authRole, subscription)
.get(service.getConfig().getMetadataStoreOperationTimeoutSeconds(), SECONDS);
} catch (InterruptedException e) {
} catch (TimeoutException e) {
log.warn("Time-out {} sec while checking authorization on {} ",
service.getConfig().getMetadataStoreOperationTimeoutSeconds(), topic);
throw e;
Expand Down

0 comments on commit 65f6112

Please sign in to comment.