[GitHub] [pulsar] michaeljmarshall commented on a change in pull request #12507: Broker: remove duplicate code in searchForCandidateBroker

[GitBox <gi...@apache.org>]

michaeljmarshall commented on a change in pull request #12507:
URL: https://github.com/apache/pulsar/pull/12507#discussion_r804977170



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java
##########
@@ -440,14 +440,6 @@ public boolean registerNamespace(NamespaceName nsname, boolean ensureOwned) thro
     private void searchForCandidateBroker(NamespaceBundle bundle,
                                           CompletableFuture<Optional<LookupResult>> lookupFuture,
                                           LookupOptions options) {
-        if (null == pulsar.getLeaderElectionService()) {
-            LOG.warn("The leader election has not yet been completed! NamespaceBundle[{}]", bundle);
-            lookupFuture.completeExceptionally(
-                    new IllegalStateException("The leader election has not yet been completed!"));

Review comment:
       I am concerned that we are changing from an future completed exceptionally to a future completed with `Optional.empty()`. In looking at the code that calls `searchForCandidateBroker`, I can see clear cases where we handle an exception differently than handling an empty option.
   
   Here is one such case: 
   
   https://github.com/apache/pulsar/blob/b80680e5385f3ea0ebdb93ba26d9d316cd67eaa4/pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java#L524-L535
   
   In that method, an exception will result in an exceptionally completed future for `isBundleOwnedByAnyBroker`, but an empty option will return a future completed with `false`. The calling methods for `isBundleOwnedByAnyBroker` appear to branch different based on success/failure.
   
   I think we need to investigate this change in behavior a bit more, possible with some tests.




-- 
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-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org