mlbiscoc commented on code in PR #2916:
URL: https://github.com/apache/solr/pull/2916#discussion_r1894366333


##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -295,22 +288,19 @@ private Map<String, Object> getCollectionStatus(
   @SuppressWarnings("unchecked")
   protected void crossCheckReplicaStateWithLiveNodes(
       List<String> liveNodes, Map<String, Object> collectionProps) {
-    for (Map.Entry<String, Object> next : collectionProps.entrySet()) {
-      Map<String, Object> collMap = (Map<String, Object>) next.getValue();
-      Map<String, Object> shards = (Map<String, Object>) collMap.get("shards");
-      for (Object nextShard : shards.values()) {
-        Map<String, Object> shardMap = (Map<String, Object>) nextShard;
-        Map<String, Object> replicas = (Map<String, Object>) 
shardMap.get("replicas");
-        for (Object nextReplica : replicas.values()) {
-          Map<String, Object> replicaMap = (Map<String, Object>) nextReplica;
-          if (Replica.State.getState((String) 
replicaMap.get(ZkStateReader.STATE_PROP))
-              != Replica.State.DOWN) {
-            // not down, so verify the node is live
-            String node_name = (String) 
replicaMap.get(ZkStateReader.NODE_NAME_PROP);
-            if (!liveNodes.contains(node_name)) {
-              // node is not live, so this replica is actually down
-              replicaMap.put(ZkStateReader.STATE_PROP, 
Replica.State.DOWN.toString());
-            }
+    Map<String, Object> shards = (Map<String, Object>) 
collectionProps.get("shards");
+    for (Object nextShard : shards.values()) {
+      Map<String, Object> shardMap = (Map<String, Object>) nextShard;
+      Map<String, Object> replicas = (Map<String, Object>) 
shardMap.get("replicas");
+      for (Object nextReplica : replicas.values()) {
+        Map<String, Object> replicaMap = (Map<String, Object>) nextReplica;

Review Comment:
   I'll change those `Maps` to `var` but its another reason I think I'll come 
back to this. I could try to improve on this and I think its possible to remove 
a bunch of code here like `buildResponseForCollection` and 
`postProcessCollectionJSON`



##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -216,28 +216,21 @@ private void fetchClusterStatusForCollOrAlias(
 
     MapWriter collectionPropsWriter =
         ew -> {
-          Iterator<Map<String, Object>> it =
-              collectionStream
-                  .map(
-                      (collectionState) ->
-                          collectionPropsResponse(
-                              collectionState,
-                              collectionVsAliases,
-                              routeKey,
-                              liveNodes,
-                              requestedShards))
-                  .iterator();
-          while (it.hasNext()) {
-            Map<String, Object> props = it.next();
-            props.forEach(
-                (key, value) -> {
-                  try {
-                    ew.put(key, value);
-                  } catch (IOException e) {
-                    throw new RuntimeException(e);
-                  }
-                });
-          }
+          collectionStream.forEach(
+              (collectionState) -> {

Review Comment:
   `putNoEx` removes the try/catch we can get it cleaner!



##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -216,28 +216,21 @@ private void fetchClusterStatusForCollOrAlias(
 
     MapWriter collectionPropsWriter =
         ew -> {
-          Iterator<Map<String, Object>> it =
-              collectionStream
-                  .map(
-                      (collectionState) ->
-                          collectionPropsResponse(
-                              collectionState,
-                              collectionVsAliases,
-                              routeKey,
-                              liveNodes,
-                              requestedShards))
-                  .iterator();
-          while (it.hasNext()) {
-            Map<String, Object> props = it.next();
-            props.forEach(
-                (key, value) -> {
-                  try {
-                    ew.put(key, value);
-                  } catch (IOException e) {
-                    throw new RuntimeException(e);
-                  }
-                });
-          }
+          collectionStream.forEach(
+              (collectionState) -> {
+                try {
+                  ew.put(
+                      collectionState.getName(),
+                      buildResponseForCollection(
+                          collectionState,
+                          collectionVsAliases,
+                          routeKey,
+                          liveNodes,
+                          requestedShards));
+                } catch (IOException e) {
+                  throw new RuntimeException(e);

Review Comment:
   `buildResponseForCollection` doesn't throw the `IOException`, the 
[EntryWriter](https://github.com/apache/solr/blob/7d856f82762e239044a366f989db13776044c605/solr/solrj/src/java/org/apache/solr/common/MapWriter.java#L95)
 does but looks like there is actually a `putNoEx` method that catches for us 
and that throws a generic `RuntimeException`. I could throw a `SolrException` 
there with better logging if you think its better.



-- 
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