BewareMyPower commented on code in PR #22858:
URL: https://github.com/apache/pulsar/pull/22858#discussion_r1630510932
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java:
##########
@@ -744,6 +745,21 @@ public void close() throws PulsarClientException {
}
}
+ private void closeUrlLookupMap() {
+ Map<String, LookupService> closedUrlLookupServices = new
HashMap(urlLookupMap.size());
+ urlLookupMap.entrySet().forEach(e -> {
+ try {
+ e.getValue().close();
+ } catch (Exception ex) {
+ log.error("Error closing lookup service {}", e.getKey(), ex);
+ }
+ closedUrlLookupServices.put(e.getKey(), e.getValue());
+ });
+ closedUrlLookupServices.entrySet().forEach(e -> {
Review Comment:
It makes sense. But it's better to add a comment to explain it. Otherwise
other contributors might just clear this map in future.
```java
private void closeUrlLookupMap() {
final Map<String, LookupService> failedLookupMap =
urlLookupMap.entrySet().stream().filter(__ -> {
try {
__.getValue().close();
return false;
} catch (Exception e) {
log.error("Error closing lookup service {}", __.getKey(), e);
return true;
}
}).collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
urlLookupMap.clear();
urlLookupMap.putAll(failedLookupMap); // retain the failed lookup
services in memory for deeper debugging
}
```
How about this implementation above? It's more simple and avoids `remove`
operations for each key (if no failure happens)
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
