315157973 commented on a change in pull request #10028:
URL: https://github.com/apache/pulsar/pull/10028#discussion_r602059532
##########
File path:
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java
##########
@@ -689,11 +782,16 @@ public boolean isClosed() {
@Override
public synchronized void updateServiceUrl(String serviceUrl) throws
PulsarClientException {
- log.info("Updating service URL to {}", serviceUrl);
+ try {
+ log.info("Updating service URL to {}", serviceUrl);
- conf.setServiceUrl(serviceUrl);
- lookup.updateServiceUrl(serviceUrl);
- cnxPool.closeAllConnections();
+ conf.setServiceUrl(serviceUrl);
+ lookup.updateServiceUrl(serviceUrl);
+ cnxPool.closeAllConnections();
+ } catch (Throwable t) {
+ log.warn("Failed to update service URL.");
+ close();
Review comment:
Looks good, I have some questions
We only need to ensure that when closing all connections, we can ensure that
all connections are called close, and that close failure of one connection will
not cause other connections to not call close.
Therefore, updating the serviceUrl fails, why close the entire pulsarClient?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
