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


##########
solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java:
##########
@@ -604,74 +572,61 @@ private void forceUpdateCollectionStatus() {
 
   // Since we have to restart jettys, we don't want to try re-balancing etc. 
until we're sure all
   // jettys that should be up are and all replicas are active.
-  private void checkReplicasInactive(List<JettySolrRunner> downJettys) throws 
InterruptedException {
-    TimeOut timeout = new TimeOut(timeoutMs, TimeUnit.MILLISECONDS, 
TimeSource.NANO_TIME);
-    DocCollection docCollection = null;
-    Set<String> liveNodes = null;
+  private void checkReplicasInactive(List<JettySolrRunner> downJettys) {
 
     Set<String> downJettyNodes = new TreeSet<>();
     for (JettySolrRunner jetty : downJettys) {
       downJettyNodes.add(
           jetty.getBaseUrl().getHost() + ":" + jetty.getBaseUrl().getPort() + 
"_solr");
     }
-    while (timeout.hasTimedOut() == false) {
-      forceUpdateCollectionStatus();
-      docCollection = 
cluster.getSolrClient().getClusterState().getCollection(COLLECTION_NAME);
-      liveNodes = cluster.getSolrClient().getClusterState().getLiveNodes();
-      boolean expectedInactive = true;
-
-      for (Slice slice : docCollection.getSlices()) {
-        for (Replica rep : slice.getReplicas()) {
-          if (downJettyNodes.contains(rep.getNodeName()) == false) {
-            continue; // We are on a live node
-          }
-          // A replica on an allegedly down node is reported as active.
-          if (rep.isActive(liveNodes)) {
-            expectedInactive = false;
+
+    waitForState(
+        "Waiting for all replicas to become inactive",
+        COLLECTION_NAME,
+        (liveNodes, docCollection) -> {
+          boolean expectedInactive = true;
+
+          for (Slice slice : docCollection.getSlices()) {
+            for (Replica rep : slice.getReplicas()) {
+              if (!downJettyNodes.contains(rep.getNodeName())) {

Review Comment:
   ;-).  I think we will have to agree to disagree ;-).   I really rely on the 
tools to encourage me in the right way to do things, or we end up with multiple 
ways.  I also have the memory of a gold fish, so if it isn't enforced by a 
tool, I may say "Hey, it should be X" for a year or two, and then swap to "Hey, 
it should be Y" just because I can't remember what the agreement was.   I can 
imagine that too strict standards becomes a strait jacket too.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to