[GitHub] merlimat commented on a change in pull request #726: Handle NPE at load-manager when leader couldn't find available broker

[gi...@git.apache.org]

merlimat commented on a change in pull request #726: Handle NPE at load-manager when leader couldn't find available broker
URL: https://github.com/apache/incubator-pulsar/pull/726#discussion_r135970555
 
 

 ##########
 File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java
 ##########
 @@ -453,12 +455,18 @@ private boolean isBrokerActive(String candidateBroker) throws KeeperException, I
      * @throws Exception
      */
     private String getLeastLoadedFromLoadManager(ServiceUnitId serviceUnit) throws Exception {
-        String lookupAddress = loadManager.get().getLeastLoaded(serviceUnit).getResourceId();
-        if (LOG.isDebugEnabled()) {
-            LOG.debug("{} : redirecting to the least loaded broker, lookup address={}", pulsar.getWebServiceAddress(),
-                    lookupAddress);
+        ResourceUnit leastLoadedBroker = loadManager.get().getLeastLoaded(serviceUnit);
+        if (leastLoadedBroker != null) {
+            String lookupAddress = leastLoadedBroker.getResourceId();
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} : redirecting to the least loaded broker, lookup address={}",
+                        pulsar.getWebServiceAddress(), lookupAddress);
+            }
+            return lookupAddress;
+        } else {
+            LOG.warn("No broker is available for {}", serviceUnit);
+            throw new BrokerServiceException.ServiceUnitNotReadyException("No broker is available for " + serviceUnit);
 
 Review comment:
   Ideally we should try to handle better these condition by using something like `Optional<>` instead of the exception.   For example, in this case we don't really need to get the stack-trace printed in logs. The warn message by itself would be enough.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services