This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 6b2edf4525a8bd34b199637caf25ebd890261bbc Author: Eric Pugh <[email protected]> AuthorDate: Thu Oct 8 12:15:06 2026 -0400 SOLR-17820: Fix CLUSTERSTATUS health reporting GREEN with down replicas (#5046) (cherry picked from commit 7e3b17502194f83bc28bbfd99c665d523aaf7d59) --- AGENTS.md | 2 +- ...lr-17820-cluster-status-health-ordering-fix.yml | 8 +++ .../apache/solr/handler/admin/ClusterStatus.java | 8 ++- .../solr/handler/admin/ClusterStatusTest.java | 69 ++++++++++++++++++++++ 4 files changed, 83 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2232abc0b4e..7c0bd0d1b74 100644 --- a/AGENTS.md +++ b/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 diff --git a/changelog/unreleased/solr-17820-cluster-status-health-ordering-fix.yml b/changelog/unreleased/solr-17820-cluster-status-health-ordering-fix.yml new file mode 100644 index 00000000000..f55ddfa71d3 --- /dev/null +++ b/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 diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java index 7d07cdd67a8..aaa1d6f5383 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java @@ -428,6 +428,11 @@ public class ClusterStatus { 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()); @@ -444,9 +449,6 @@ public class ClusterStatus { 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; } } diff --git a/solr/core/src/test/org/apache/solr/handler/admin/ClusterStatusTest.java b/solr/core/src/test/org/apache/solr/handler/admin/ClusterStatusTest.java new file mode 100644 index 00000000000..54b68a2d16c --- /dev/null +++ b/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")); + } +}
