diff --git a/AGENTS.md b/AGENTS.md index e892fa39f646..dab06fac42b0 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 ` 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 000000000000..f55ddfa71d3d --- /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 1bd3e882a181..0b21e64668bc 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 @@ -408,6 +408,11 @@ private Map buildResponseForCollection( byte[] bytes = Utils.toJSON(clusterStateCollection); @SuppressWarnings("unchecked") Map docCollection = (Map) 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 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; } } 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 000000000000..54b68a2d16c3 --- /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 replica1 = new LinkedHashMap<>(); + replica1.put("state", "active"); + replica1.put("node_name", "node1:8983_solr"); + replica1.put("leader", "true"); + + Map replica2 = new LinkedHashMap<>(); + replica2.put("state", "active"); + replica2.put("node_name", "node2:8983_solr"); + + Map replicas = new LinkedHashMap<>(); + replicas.put("core_node1", replica1); + replicas.put("core_node2", replica2); + + Map shard1 = new LinkedHashMap<>(); + shard1.put("replicas", replicas); + + Map shards = new LinkedHashMap<>(); + shards.put("shard1", shard1); + + Map docCollection = new LinkedHashMap<>(); + docCollection.put("shards", shards); + + List 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 result = ClusterStatus.postProcessCollectionJSON(docCollection); + + assertEquals("down", replica2.get("state")); + // One of two replicas is active, so shard and collection health are ORANGE. + Map resultShard1 = + (Map) ((Map) result.get("shards")).get("shard1"); + assertEquals("ORANGE", resultShard1.get("health")); + assertEquals("ORANGE", result.get("health")); + } +}