BewareMyPower commented on code in PR #23433:
URL: https://github.com/apache/pulsar/pull/23433#discussion_r1796368853
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/channel/ServiceUnitStateChannelImpl.java:
##########
@@ -1462,6 +1478,10 @@ private CompletableFuture<Void>
healthCheckBrokerAsync(String brokerId) {
}
private void doHealthCheckBrokerAsyncWithRetries(String brokerId, int
retry, CompletableFuture<Void> future) {
+ if (channelDisabled()) {
Review Comment:
Yes, I think we should check `channelDisabled()` in `doCleanup` before
calling `healthCheckBrokerAsync` in `doCleanup(..., false)`.
The reason to make the change here is that the future of
`healthCheckBrokerAsync` could be blocked until timeout during the close phase.
```
# this log was added by me
2024-10-11T10:59:40,184 - INFO - [pulsar-385-8:ServiceUnitStateChannelImpl]
- XYZ doHealthCheckBrokerAsyncWithRetries localhost:61361 2
2024-10-11T10:59:45,130 - INFO -
[pulsar-load-manager-384-1:ServiceUnitStateChannelImpl] - Failed to check
broker:localhost:61361 health
java.util.concurrent.TimeoutException: null
at
java.base/java.util.concurrent.CompletableFuture.timedGet(CompletableFuture.java:1960)
~[?:?]
at
java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2095)
~[?:?]
at
org.apache.pulsar.broker.loadbalance.extensions.channel.ServiceUnitStateChannelImpl.doCleanup(ServiceUnitStateChannelImpl.java:1525)
~[classes/:?]
```
--
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]
