nick-boss-tech commented on code in PR #4959:
URL: https://github.com/apache/solr/pull/4959#discussion_r4136657636
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -1053,6 +1014,46 @@ private void cleanupAfterFailure(
}
}
+ /**
+ * Slice-state rollback for a failed split, or {@code null} if the committed
switch-over must not
+ * be undone. Always includes parent=ACTIVE so a stale snapshot cannot skip
the restore.
+ */
+ static Map<String, Object> buildCleanupShardStateUpdates(
+ DocCollection coll, String parentShard, List<String> subSlices,
Set<String> offlineSlices) {
+ // 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());
+ for (String sliceName : offlineSlices) {
Review Comment:
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)*
Confirmed. `offlineSlices` had no population path, so I removed the unused
plumbing. The existing cleanup tests pass.
---
*AI was used to draft this comment, Nick Shanin approved it.*
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -1053,6 +1014,46 @@ private void cleanupAfterFailure(
}
}
+ /**
+ * Slice-state rollback for a failed split, or {@code null} if the committed
switch-over must not
+ * be undone. Always includes parent=ACTIVE so a stale snapshot cannot skip
the restore.
Review Comment:
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)*
Yes—the helper now restores the parent to ACTIVE unless the parent is
INACTIVE and all split children are ACTIVE, preserving a completed switch-over.
The focused cleanup tests cover both cases.
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -1053,6 +1014,46 @@ private void cleanupAfterFailure(
}
}
+ /**
+ * Slice-state rollback for a failed split, or {@code null} if the committed
switch-over must not
+ * be undone. Always includes parent=ACTIVE so a stale snapshot cannot skip
the restore.
Review Comment:
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)*
Thanks for clarifying. I’ve reworded that sentence to make it clearer that
cleanup puts the parent shard back in ACTIVE.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
