SOLR-17820: Fix CLUSTERSTATUS health reporting GREEN with down replicas

AGENTS.md

@@ -38,7 +38,7 @@ While README.md and CONTRIBUTING.md are mainly written for humans, this file is
 - When adding a *new* Java test suite/file:
     - Subclass SolrTestCase, or if SolrCloud is needed then SolrCloudTestCase
     - If SolrTestCase and need to embed Solr, use either EmbeddedSolrServerTestRule (doesn't use HTTP) or SolrJettyTestRule if HTTP/Jetty is relevant to what is being tested.
-    - Avoid SolrTestCaseJ4 for new tests
+    - Use SolrTestCase instead of SolrTestCaseJ4 for new tests
 - System properties set in a test are restored after each test by the base classes (`SystemPropertiesRestoreRule`); do not add `try/finally` + `System.clearProperty` cleanup
 - For BATS shell integration tests in `solr/packaging/test/`:
     - Always use `run <command>` followed by `assert_output --partial "..."` or `refute_output --partial "..."` instead of capturing output into local variables and using `[[ ]]` comparisons

changelog/unreleased/solr-17820-cluster-status-health-ordering-fix.yml

@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Fix CLUSTERSTATUS/collections-detail "health" reporting GREEN for a shard with a replica on a dead node, because health was computed before the live-node cross-check corrected that replica's stale state.json "active" entry.
+type: fixed
+authors:
+  - name: Eric Pugh
+links:
+  - name: SOLR-17820
+    url: https://issues.apache.org/jira/browse/SOLR-17820

solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java

@@ -408,6 +408,11 @@ private Map<String, Object> buildResponseForCollection(
     byte[] bytes = Utils.toJSON(clusterStateCollection);
     @SuppressWarnings("unchecked")
     Map<String, Object> docCollection = (Map<String, Object>) Utils.fromJSON(bytes);
+
+    // Replicas on dead nodes can still be marked active in state.json.
+    // Correct their states before computing health.
+    crossCheckReplicaStateWithLiveNodes(liveNodes, docCollection);
+
     collectionStatus = getCollectionStatus(docCollection, name, shards);
 
     collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion());
@@ -424,9 +429,6 @@ private Map<String, Object> buildResponseForCollection(
       collectionStatus.put("PRS", prs);
     }
 
-    // now we need to walk the collectionProps tree to cross-check replica state with live nodes
-    crossCheckReplicaStateWithLiveNodes(liveNodes, collectionStatus);
-
     return collectionStatus;
   }
 }

solr/core/src/test/org/apache/solr/handler/admin/ClusterStatusTest.java

@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.admin;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.junit.Test;
+
+/** Tests that cluster health reflects replica states corrected against live nodes. */
+public class ClusterStatusTest extends SolrTestCase {
+
+  @Test
+  @SuppressWarnings("unchecked")
+  public void testHealthReflectsReplicaOnDeadNode() {
+    // Both replicas are marked active, but only node1 is live.
+    Map<String, Object> replica1 = new LinkedHashMap<>();
+    replica1.put("state", "active");
+    replica1.put("node_name", "node1:8983_solr");
+    replica1.put("leader", "true");
+
+    Map<String, Object> replica2 = new LinkedHashMap<>();
+    replica2.put("state", "active");
+    replica2.put("node_name", "node2:8983_solr");
+
+    Map<String, Object> replicas = new LinkedHashMap<>();
+    replicas.put("core_node1", replica1);
+    replicas.put("core_node2", replica2);
+
+    Map<String, Object> shard1 = new LinkedHashMap<>();
+    shard1.put("replicas", replicas);
+
+    Map<String, Object> shards = new LinkedHashMap<>();
+    shards.put("shard1", shard1);
+
+    Map<String, Object> docCollection = new LinkedHashMap<>();
+    docCollection.put("shards", shards);
+
+    List<String> liveNodes = List.of("node1:8983_solr");
+
+    // Match buildResponseForCollection: correct replica states, then compute health.
+    ClusterStatus clusterStatus = new ClusterStatus(null, new ModifiableSolrParams());
+    clusterStatus.crossCheckReplicaStateWithLiveNodes(liveNodes, docCollection);
+    Map<String, Object> result = ClusterStatus.postProcessCollectionJSON(docCollection);
+
+    assertEquals("down", replica2.get("state"));
+    // One of two replicas is active, so shard and collection health are ORANGE.
+    Map<String, Object> resultShard1 =
+        (Map<String, Object>) ((Map<String, Object>) result.get("shards")).get("shard1");
+    assertEquals("ORANGE", resultShard1.get("health"));
+    assertEquals("ORANGE", result.get("health"));
+  }
+}