This is an automated email from the ASF dual-hosted git repository.
sigram pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 0cc328310f8 SOLR-18277: Restore parent shard state on failed split
(#4959)
0cc328310f8 is described below
commit 0cc328310f8f4162572ea5bd5256cea29543e261
Author: Nick <[email protected]>
AuthorDate: Mon Oct 5 04:46:06 2026 -0600
SOLR-18277: Restore parent shard state on failed split (#4959)
---
changelog/unreleased/SOLR-18277.yml | 7 +
.../solr/cloud/api/collections/SplitShardCmd.java | 95 +++++-----
.../solr/cloud/api/collections/ShardSplitTest.java | 2 -
.../api/collections/SplitShardCmdCleanupTest.java | 207 +++++++++++++++++++++
4 files changed, 256 insertions(+), 55 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 4787e81734c..9bdf11435a6 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
@@ -33,11 +33,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.AtomicReference;
import org.apache.solr.client.solrj.cloud.DistribStateManager;
@@ -164,7 +162,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();
ClusterState clusterState = zkStateReader.getClusterState();
@@ -830,12 +827,7 @@ public class SplitShardCmd implements
CollApiCmds.CollectionApiCommand {
} finally {
if (!success) {
cleanupAfterFailure(
- adminCmdContext,
- zkStateReader,
- collectionName,
- parentSlice.getName(),
- subSlices,
- offlineSlices);
+ adminCmdContext, zkStateReader, collectionName,
parentSlice.getName(), subSlices);
unlockForSplit(ccc.getSolrCloudManager(), collectionName,
parentSlice.getName());
}
}
@@ -939,8 +931,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 {
@@ -960,48 +951,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()) {
@@ -1053,6 +1004,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/ShardSplitTest.java
b/solr/core/src/test/org/apache/solr/cloud/api/collections/ShardSplitTest.java
index a366a849a42..d421e793cff 100644
---
a/solr/core/src/test/org/apache/solr/cloud/api/collections/ShardSplitTest.java
+++
b/solr/core/src/test/org/apache/solr/cloud/api/collections/ShardSplitTest.java
@@ -328,7 +328,6 @@ public class ShardSplitTest extends BasicDistributedZkTest {
* <p>See SOLR-9439
*/
@Test
- @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-18277";)
public void testSplitAfterFailedSplit() throws Exception {
waitForThingsToLevelOut(15, TimeUnit.SECONDS);
@@ -384,7 +383,6 @@ public class ShardSplitTest extends BasicDistributedZkTest {
@Test
@Nightly
- @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-18277";)
public void testSplitAfterFailedSplit2() throws Exception {
waitForThingsToLevelOut(15, TimeUnit.SECONDS);
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);
+ }
+}
