You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ds...@apache.org on 2026/10/22 14:44:20 UTC
(solr) 04/15: SOLR-18277: Restore parent shard state on failed split (#4959)
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
commit 5b32142efd5ef08cc5a31570b6360b8fd8fa3d05
Author: Nick <ni...@gmail.com>
AuthorDate: Mon Oct 5 04:46:06 2026 -0600
SOLR-18277: Restore parent shard state on failed split (#4959)
(cherry picked from commit 0cc328310f8f4162572ea5bd5256cea29543e261)
(cherry picked from commit edc5da188a9ea28745b13c3cda9106dc1b4271a0)
---
changelog/unreleased/SOLR-18277.yml | 7 +
.../solr/cloud/api/collections/SplitShardCmd.java | 91 +++++----
.../api/collections/SplitShardCmdCleanupTest.java | 207 +++++++++++++++++++++
3 files changed, 256 insertions(+), 49 deletions(-)
diff --git a/changelog/unreleased/SOLR-18277.yml b/changelog/unreleased/SOLR-18277.yml
new file mode 100644
index 00000000000..9c870578aca
--- /dev/null
+++ b/changelog/unreleased/SOLR-18277.yml
@@ -0,0 +1,7 @@
+title: Restore the parent shard to ACTIVE after a failed split even if cluster state is stale.
+type: fixed
+authors:
+ - name: Nick Shanin
+links:
+ - name: SOLR-18277
+ url: https://issues.apache.org/jira/browse/SOLR-18277
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
index 6d6ce8c428d..b45ec0f0c41 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
@@ -32,11 +32,9 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
-import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
@@ -163,7 +161,6 @@ public class SplitShardCmd implements CollApiCmds.CollectionApiCommand {
zkStateReader.forceUpdateCollection(collectionName);
AtomicReference<String> slice = new AtomicReference<>();
slice.set(message.getStr(ZkStateReader.SHARD_ID_PROP));
- Set<String> offlineSlices = new HashSet<>();
RTimerTree timings = new RTimerTree();
String splitKey = message.getStr("split.key");
@@ -846,8 +843,7 @@ public class SplitShardCmd implements CollApiCmds.CollectionApiCommand {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, null, e);
} finally {
if (!success) {
- cleanupAfterFailure(
- zkStateReader, collectionName, parentSlice.getName(), subSlices, offlineSlices);
+ cleanupAfterFailure(zkStateReader, collectionName, parentSlice.getName(), subSlices);
unlockForSplit(ccc.getSolrCloudManager(), collectionName, parentSlice.getName());
}
}
@@ -922,8 +918,7 @@ public class SplitShardCmd implements CollApiCmds.CollectionApiCommand {
ZkStateReader zkStateReader,
String collectionName,
String parentShard,
- List<String> subSlices,
- Set<String> offlineSlices) {
+ List<String> subSlices) {
log.info("Cleaning up after a failed split of {}/{}", collectionName, parentShard);
// get the latest state
try {
@@ -943,48 +938,8 @@ public class SplitShardCmd implements CollApiCmds.CollectionApiCommand {
return;
}
- // If parent is inactive and all sub shards are active, then rolling back
- // to make the parent active again will cause data loss.
- if (coll.getSlice(parentShard).getState() == Slice.State.INACTIVE) {
- boolean allSubSlicesActive = true;
- for (String sub : subSlices) {
- if (coll.getSlice(sub).getState() != Slice.State.ACTIVE) {
- allSubSlicesActive = false;
- break;
- }
- }
- if (allSubSlicesActive) {
- return;
- }
- }
-
- // set already created sub shards states to CONSTRUCTION - this prevents them
- // from entering into RECOVERY or ACTIVE (SOLR-9455)
- final Map<String, Object> propMap = new HashMap<>();
- boolean sendUpdateState = false;
- propMap.put(Overseer.QUEUE_OPERATION, OverseerAction.UPDATESHARDSTATE.toLower());
- propMap.put(ZkStateReader.COLLECTION_PROP, collectionName);
- for (Slice s : coll.getSlices()) {
- if (!subSlices.contains(s.getName())) {
- continue;
- }
- propMap.put(s.getName(), Slice.State.CONSTRUCTION.toString());
- sendUpdateState = true;
- }
-
- // if parent is inactive activate it again
- Slice parentSlice = coll.getSlice(parentShard);
- if (parentSlice.getState() == Slice.State.INACTIVE) {
- sendUpdateState = true;
- propMap.put(parentShard, Slice.State.ACTIVE.toString());
- }
- // plus any other previously deactivated slices
- for (String sliceName : offlineSlices) {
- propMap.put(sliceName, Slice.State.ACTIVE.toString());
- sendUpdateState = true;
- }
-
- if (sendUpdateState) {
+ Map<String, Object> propMap = buildCleanupShardStateUpdates(coll, parentShard, subSlices);
+ if (propMap != null) {
try {
ZkNodeProps m = new ZkNodeProps(propMap);
if (ccc.getDistributedClusterStateUpdater().isDistributedStateUpdate()) {
@@ -1032,6 +987,44 @@ public class SplitShardCmd implements CollApiCmds.CollectionApiCommand {
}
}
+ /**
+ * Returns the changes needed to undo a failed split, or {@code null} if the split has already
+ * finished. When it returns changes, they set the parent shard to {@code ACTIVE}, even if the
+ * cluster state has changed since it was read.
+ */
+ static Map<String, Object> buildCleanupShardStateUpdates(
+ DocCollection coll, String parentShard, List<String> subSlices) {
+ // If parent is inactive and all sub shards are active, then rolling back
+ // to make the parent active again will cause data loss.
+ if (coll.getSlice(parentShard).getState() == Slice.State.INACTIVE) {
+ boolean allSubSlicesActive = true;
+ for (String sub : subSlices) {
+ if (coll.getSlice(sub).getState() != Slice.State.ACTIVE) {
+ allSubSlicesActive = false;
+ break;
+ }
+ }
+ if (allSubSlicesActive) {
+ return null;
+ }
+ }
+
+ // set already created sub shards states to CONSTRUCTION - this prevents them
+ // from entering into RECOVERY or ACTIVE (SOLR-9455)
+ final Map<String, Object> propMap = new HashMap<>();
+ propMap.put(Overseer.QUEUE_OPERATION, OverseerAction.UPDATESHARDSTATE.toLower());
+ propMap.put(ZkStateReader.COLLECTION_PROP, coll.getName());
+ for (Slice s : coll.getSlices()) {
+ if (!subSlices.contains(s.getName())) {
+ continue;
+ }
+ propMap.put(s.getName(), Slice.State.CONSTRUCTION.toString());
+ }
+ // Always pin parent ACTIVE; a stale snapshot can miss an in-flight INACTIVE switch.
+ propMap.put(parentShard, Slice.State.ACTIVE.toString());
+ return propMap;
+ }
+
public static Slice getParentSlice(
ClusterState clusterState,
String collectionName,
diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/SplitShardCmdCleanupTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/SplitShardCmdCleanupTest.java
new file mode 100644
index 00000000000..f399e9501ed
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/SplitShardCmdCleanupTest.java
@@ -0,0 +1,207 @@
+/*
+ * 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.cloud.api.collections;
+
+import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.client.solrj.cloud.DistribStateManager;
+import org.apache.solr.client.solrj.cloud.SolrCloudManager;
+import org.apache.solr.cloud.Overseer;
+import org.apache.solr.cloud.overseer.OverseerAction;
+import org.apache.solr.cloud.overseer.SliceMutator;
+import org.apache.solr.cloud.overseer.ZkWriteCommand;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.DocRouter;
+import org.apache.solr.common.cloud.Slice;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.handler.admin.ConfigSetsHandler;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** In-memory coverage for {@link SplitShardCmd} failed-split state rollback. */
+public class SplitShardCmdCleanupTest extends SolrTestCase {
+
+ private static final String COLLECTION = "collection1";
+ private static final String PARENT = "shard1";
+ private static final String CHILD_0 = "shard1_0";
+ private static final String CHILD_1 = "shard1_1";
+ private static final List<String> CHILDREN = List.of(CHILD_0, CHILD_1);
+
+ @BeforeClass
+ public static void beforeClass() {
+ assumeWorkingMockito();
+ }
+
+ @Test
+ public void testCleanupIncludesParentWhenSnapshotStillShowsActive() {
+ DocCollection coll =
+ collection(
+ slice(PARENT, Slice.State.ACTIVE),
+ slice(CHILD_0, Slice.State.CONSTRUCTION),
+ slice(CHILD_1, Slice.State.RECOVERY));
+
+ Map<String, Object> updates =
+ SplitShardCmd.buildCleanupShardStateUpdates(coll, PARENT, CHILDREN);
+
+ assertNotNull(updates);
+ assertEquals(Slice.State.ACTIVE.toString(), updates.get(PARENT));
+ assertEquals(Slice.State.CONSTRUCTION.toString(), updates.get(CHILD_0));
+ assertEquals(Slice.State.CONSTRUCTION.toString(), updates.get(CHILD_1));
+ }
+
+ @Test
+ public void testCleanupRestoresInactiveParentWhenChildrenAreNotAllActive() {
+ DocCollection coll =
+ collection(
+ slice(PARENT, Slice.State.INACTIVE),
+ slice(CHILD_0, Slice.State.ACTIVE),
+ slice(CHILD_1, Slice.State.CONSTRUCTION));
+
+ Map<String, Object> updates =
+ SplitShardCmd.buildCleanupShardStateUpdates(coll, PARENT, CHILDREN);
+
+ assertNotNull(updates);
+ assertEquals(Slice.State.ACTIVE.toString(), updates.get(PARENT));
+ assertEquals(Slice.State.CONSTRUCTION.toString(), updates.get(CHILD_0));
+ assertEquals(Slice.State.CONSTRUCTION.toString(), updates.get(CHILD_1));
+ }
+
+ @Test
+ public void testCleanupSkipsCommittedSwitchOver() {
+ DocCollection coll =
+ collection(
+ slice(PARENT, Slice.State.INACTIVE),
+ slice(CHILD_0, Slice.State.ACTIVE),
+ slice(CHILD_1, Slice.State.ACTIVE));
+
+ assertNull(SplitShardCmd.buildCleanupShardStateUpdates(coll, PARENT, CHILDREN));
+ }
+
+ @Test
+ public void testStaleCleanupWithoutParentLeavesParentInactive() {
+ ClusterState afterSuccess = apply(initialState(), successSwitch());
+ assertSame(
+ Slice.State.INACTIVE, afterSuccess.getCollection(COLLECTION).getSlice(PARENT).getState());
+ assertSame(
+ Slice.State.ACTIVE, afterSuccess.getCollection(COLLECTION).getSlice(CHILD_0).getState());
+
+ ClusterState afterStaleCleanup = apply(afterSuccess, staleCleanupWithoutParent());
+ assertSame(
+ "omitting parent=active after the success switch is the race",
+ Slice.State.INACTIVE,
+ afterStaleCleanup.getCollection(COLLECTION).getSlice(PARENT).getState());
+ assertSame(
+ Slice.State.CONSTRUCTION,
+ afterStaleCleanup.getCollection(COLLECTION).getSlice(CHILD_0).getState());
+ }
+
+ @Test
+ public void testFixedCleanupRestoresParentAfterQueuedSuccessSwitch() {
+ DocCollection staleSnapshot = initialState().getCollection(COLLECTION);
+ Map<String, Object> cleanup =
+ SplitShardCmd.buildCleanupShardStateUpdates(staleSnapshot, PARENT, CHILDREN);
+ assertNotNull(cleanup);
+ assertEquals(Slice.State.ACTIVE.toString(), cleanup.get(PARENT));
+
+ ClusterState afterSuccess = apply(initialState(), successSwitch());
+ ClusterState afterCleanup = apply(afterSuccess, cleanup);
+
+ assertSame(
+ Slice.State.ACTIVE, afterCleanup.getCollection(COLLECTION).getSlice(PARENT).getState());
+ assertSame(
+ Slice.State.CONSTRUCTION,
+ afterCleanup.getCollection(COLLECTION).getSlice(CHILD_0).getState());
+ assertSame(
+ Slice.State.CONSTRUCTION,
+ afterCleanup.getCollection(COLLECTION).getSlice(CHILD_1).getState());
+ }
+
+ private static ClusterState initialState() {
+ return cluster(
+ collection(
+ slice(PARENT, Slice.State.ACTIVE),
+ slice(CHILD_0, Slice.State.CONSTRUCTION),
+ slice(CHILD_1, Slice.State.CONSTRUCTION)));
+ }
+
+ private static Map<String, Object> successSwitch() {
+ Map<String, Object> props = baseUpdate();
+ props.put(PARENT, Slice.State.INACTIVE.toString());
+ props.put(CHILD_0, Slice.State.ACTIVE.toString());
+ props.put(CHILD_1, Slice.State.ACTIVE.toString());
+ return props;
+ }
+
+ private static Map<String, Object> staleCleanupWithoutParent() {
+ Map<String, Object> props = baseUpdate();
+ props.put(CHILD_0, Slice.State.CONSTRUCTION.toString());
+ props.put(CHILD_1, Slice.State.CONSTRUCTION.toString());
+ return props;
+ }
+
+ private static Map<String, Object> baseUpdate() {
+ Map<String, Object> props = new HashMap<>();
+ props.put(Overseer.QUEUE_OPERATION, OverseerAction.UPDATESHARDSTATE.toLower());
+ props.put(ZkStateReader.COLLECTION_PROP, COLLECTION);
+ return props;
+ }
+
+ private static ClusterState apply(ClusterState state, Map<String, Object> update) {
+ SolrCloudManager cloudManager = mock(SolrCloudManager.class);
+ when(cloudManager.getDistribStateManager()).thenReturn(mock(DistribStateManager.class));
+ when(cloudManager.getTimeSource()).thenReturn(TimeSource.NANO_TIME);
+ ZkWriteCommand cmd =
+ new SliceMutator(cloudManager).updateShardState(state, new ZkNodeProps(update));
+ return new ClusterState(Set.of(), Map.of(COLLECTION, cmd.collection));
+ }
+
+ private static ClusterState cluster(DocCollection collection) {
+ return new ClusterState(Set.of(), Map.of(COLLECTION, collection));
+ }
+
+ private static Slice slice(String name, Slice.State state) {
+ return new Slice(
+ name, Map.of(), Map.of(ZkStateReader.STATE_PROP, state.toString()), COLLECTION);
+ }
+
+ private static DocCollection collection(Slice... slices) {
+ Map<String, Slice> map = new LinkedHashMap<>();
+ for (Slice slice : slices) {
+ map.put(slice.getName(), slice);
+ }
+ return DocCollection.create(
+ COLLECTION,
+ map,
+ Map.of(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME),
+ DocRouter.DEFAULT,
+ 1,
+ Instant.EPOCH,
+ null);
+ }
+}