epugh commented on code in PR #1810:
URL: https://github.com/apache/solr/pull/1810#discussion_r1279855289


##########
solr/core/src/test/org/apache/solr/cloud/LeaderElectionTest.java:
##########
@@ -258,8 +258,8 @@ public void testCancelElection() throws Exception {
     Thread.sleep(1000);
 
     String urlScheme = zkStateReader.getClusterProperty(URL_SCHEME, "http");
-    String url1 = Utils.getBaseUrlForNodeName("127.0.0.1:80_solr/1", 
urlScheme) + "/";
-    String url2 = Utils.getBaseUrlForNodeName("127.0.0.1:80_solr/2", 
urlScheme) + "/";
+    String url1 = Utils.getBaseUrlForNodeName("127.0.0.1:80_solr/1", 
urlScheme) + "/" + "1" + '/';

Review Comment:
   So, I actually am worried this entire idea may fail..  At least the way I 
read this test, when we do leaderelection, we name our nodes `{some_ip}/solr/1` 
and `{same_ip}/solr/2` and `{same_ip}/solr/3` and then something happens.   
However, I don't know if that is ACTUALLY how it happens, or maybe just how 
this test was written, since I've never dug into leader election in any way.... 
   I also was wondering if maybe we need some sort of `.bats` test of leader 
election to prove that this change didn't break leader election in the real 
world..  I mean, technically this LeaderElectionTest.java passes...  but...



-- 
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: issues-unsubscr...@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to