[GitHub] [pulsar] lhotari commented on a change in pull request #9240: PIP-45: Implement load-manager leader election using new CoordinationService interface

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

lhotari commented on a change in pull request #9240:
URL: https://github.com/apache/pulsar/pull/9240#discussion_r568525843



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java
##########
@@ -447,7 +448,7 @@ public boolean registerNamespace(String namespace, boolean ensureOwned) throws P
     private void searchForCandidateBroker(NamespaceBundle bundle,
                                           CompletableFuture<Optional<LookupResult>> lookupFuture,
                                           LookupOptions options) {
-        if (null == pulsar.getLeaderElectionService() || !pulsar.getLeaderElectionService().isElected()) {
+        if (null == pulsar.getLeaderElectionService()) {

Review comment:
       There's a flaky test issue #9408 where it seems that the reason why the test fails is that the leader never gets assigned to one of the brokers in the test. The value stays as `Optional.empty()`. I wonder what to do about the test? is it a problem in the test code or production code?
   There's also other concerns that I have expressed in comments of #9408 related to the deep stack that was observed. The election seems to recurse when retrying and at least that happens in the test.




----------------------------------------------------------------
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:
users@infra.apache.org