From 4278b8af95a1abc1d25abac42efd96cdf09f29b1 Mon Sep 17 00:00:00 2001 From: Megan Carey Date: Thu, 28 Jan 2021 12:35:42 -0800 Subject: [PATCH 1/5] Added logs and updated default splitMethod to be lINK --- .../cloud/api/collections/SplitShardCmd.java | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) 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 ff9df3b825c6..596b2fea27c3 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 @@ -60,7 +60,10 @@ import static org.apache.solr.common.params.CommonAdminParams.ASYNC; import static org.apache.solr.common.params.CommonAdminParams.NUM_SUB_SHARDS; - +/** + * Index split request processed by Overseer. Requests from here go to the host of the parent shard, + * and are processed by @link. + */ public class SplitShardCmd implements OverseerCollectionMessageHandler.Cmd { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static final int MIN_NUM_SUB_SHARDS = 2; @@ -80,12 +83,29 @@ public void call(ClusterState state, ZkNodeProps message, @SuppressWarnings({"ra split(state, message,(NamedList) results); } + /** + * Shard splits start here and make additional requests to the host of the parent shard. + * + * The sequence of requests is as follows: + * 1. Verify that there is enough disk space to create sub-shards. + * 2. If splitByPrefix is true, make request to get prefix ranges. + * 3. If this split was attempted previously and there are lingering sub-shards, delete them. + * 4. Create sub-shards in CONSTRUCTION state. + * 5. Add an initial replica to each sub-shard. + * 6. Request that parent shard wait for children to become ACTIVE. + * 7. Execute split: either LINK or REWRITE. + * 8. Apply buffered updates to the sub-shards so they are up-to-date with parent. + * 9. Determine node placement for additional replicas (but do not create yet). + * 10. If replicationFactor is more than 1, set shard state for sub-shards to RECOVERY; else mark ACTIVE. + * 11. Create additional replicas of sub-shards. + */ @SuppressWarnings({"rawtypes"}) public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList results) throws Exception { final String asyncId = message.getStr(ASYNC); + // get split method, or default to LINK if unspecified boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, false); - String methodStr = message.getStr(CommonAdminParams.SPLIT_METHOD, SolrIndexSplitter.SplitMethod.REWRITE.toLower()); + String methodStr = message.getStr(CommonAdminParams.SPLIT_METHOD, SolrIndexSplitter.SplitMethod.LINK.toLower()); SolrIndexSplitter.SplitMethod splitMethod = SolrIndexSplitter.SplitMethod.get(methodStr); if (splitMethod == null) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unknown value '" + CommonAdminParams.SPLIT_METHOD + @@ -114,7 +134,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList(); @@ -316,7 +340,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList nodes = clusterState.getLiveNodes(); List nodeList = new ArrayList<>(nodes.size()); nodeList.addAll(nodes); @@ -521,6 +546,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList 1, set shard state for sub-shards to RECOVERY; otherwise mark ACTIVE if (repFactor == 1) { // A commit is needed so that documents are visible when the sub-shard replicas come up // (Note: This commit used to be after the state switch, but was brought here before the state switch @@ -551,6 +577,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList replica : replicas) { From e91d3c68844c9af03de1ef56152ecd966c518c26 Mon Sep 17 00:00:00 2001 From: Megan Carey Date: Thu, 28 Jan 2021 14:48:55 -0800 Subject: [PATCH 2/5] Add test for splitMethods --- .../cloud/api/collections/SplitShardCmd.java | 2 +- .../org/apache/solr/cloud/SplitShardTest.java | 46 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) 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 596b2fea27c3..7083ca497cec 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 @@ -109,7 +109,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList Date: Thu, 28 Jan 2021 15:58:00 -0800 Subject: [PATCH 3/5] Address review feedback --- .../cloud/api/collections/SplitShardCmd.java | 52 ++++++++----------- .../org/apache/solr/cloud/SplitShardTest.java | 8 +-- 2 files changed, 28 insertions(+), 32 deletions(-) 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 7083ca497cec..5ed99a981f6c 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 @@ -87,17 +87,19 @@ public void call(ClusterState state, ZkNodeProps message, @SuppressWarnings({"ra * Shard splits start here and make additional requests to the host of the parent shard. * * The sequence of requests is as follows: - * 1. Verify that there is enough disk space to create sub-shards. - * 2. If splitByPrefix is true, make request to get prefix ranges. - * 3. If this split was attempted previously and there are lingering sub-shards, delete them. - * 4. Create sub-shards in CONSTRUCTION state. - * 5. Add an initial replica to each sub-shard. - * 6. Request that parent shard wait for children to become ACTIVE. - * 7. Execute split: either LINK or REWRITE. - * 8. Apply buffered updates to the sub-shards so they are up-to-date with parent. - * 9. Determine node placement for additional replicas (but do not create yet). - * 10. If replicationFactor is more than 1, set shard state for sub-shards to RECOVERY; else mark ACTIVE. - * 11. Create additional replicas of sub-shards. + *
    + *
  • Verify that there is enough disk space to create sub-shards.
    • + *
  • If splitByPrefix is true, make request to get prefix ranges.
    • + *
  • If this split was attempted previously and there are lingering sub-shards, delete them.
    • + *
  • Create sub-shards in CONSTRUCTION state.
    • + *
  • Add an initial replica to each sub-shard.
    • + *
  • Request that parent shard wait for children to become ACTIVE.
    • + *
  • Execute split: either LINK or REWRITE.
    • + *
  • Apply buffered updates to the sub-shards so they are up-to-date with parent.
    • + *
  • Determine node placement for additional replicas (but do not create yet).
    • + *
  • If replicationFactor is more than 1, set shard state for sub-shards to RECOVERY; else mark ACTIVE.
    • + *
  • Create additional replicas of sub-shards.
    • + *
*/ @SuppressWarnings({"rawtypes"}) public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList results) throws Exception { @@ -150,7 +152,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList(); @@ -340,7 +341,6 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList nodes = clusterState.getLiveNodes(); - List nodeList = new ArrayList<>(nodes.size()); - nodeList.addAll(nodes); - - // Remove the node that hosts the parent shard for replica creation. - nodeList.remove(nodeName); - // TODO: change this to handle sharding a slice into > 2 sub-shards. // we have already created one subReplica for each subShard on the parent node. @@ -546,7 +537,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList 1, set shard state for sub-shards to RECOVERY; otherwise mark ACTIVE + // if replicationFactor > 1, set shard state for sub-shards to RECOVERY; otherwise mark ACTIVE if (repFactor == 1) { // A commit is needed so that documents are visible when the sub-shard replicas come up // (Note: This commit used to be after the state switch, but was brought here before the state switch @@ -577,7 +568,6 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList replica : replicas) { @@ -607,6 +597,10 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList + *
  • No override specified. Verify that LINK method is used.
    • + *
  • REWRITE method specified. Verify that LINK steps are skipped.
    • + *
  • Invalid override specified. Verify that split fails.
    • + * */ @Test public void testSplitMethods() throws Exception { From d071b40d83773bd741af09f7ba6a2df252b65ae3 Mon Sep 17 00:00:00 2001 From: Megan Carey Date: Thu, 28 Jan 2021 16:37:50 -0800 Subject: [PATCH 4/5] Fix logging format for Gradle precommit Fix comment --- .../org/apache/solr/cloud/api/collections/SplitShardCmd.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 5ed99a981f6c..08a3f28e1842 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 @@ -62,7 +62,7 @@ /** * Index split request processed by Overseer. Requests from here go to the host of the parent shard, - * and are processed by @link. + * and are processed by SplitOp. */ public class SplitShardCmd implements OverseerCollectionMessageHandler.Cmd { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -599,7 +599,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList Date: Mon, 22 Feb 2021 12:15:25 -0800 Subject: [PATCH 5/5] Addressing review feedback --- .../solr/cloud/api/collections/SplitShardCmd.java | 2 +- .../test/org/apache/solr/cloud/SplitShardTest.java | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) 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 08a3f28e1842..1638d7f8191a 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 @@ -599,7 +599,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList