Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-15119 Add logs and make default splitMethod to be LINK #2265

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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.
megancarey marked this conversation as resolved.
Show resolved Hide resolved
*/
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;
Expand All @@ -80,16 +83,35 @@ public void call(ClusterState state, ZkNodeProps message, @SuppressWarnings({"ra
split(state, message,(NamedList<Object>) results);
}

/**
megancarey marked this conversation as resolved.
Show resolved Hide resolved
* Shard splits start here and make additional requests to the host of the parent shard.
*
* The sequence of requests is as follows:
* <ul>
* <li>Verify that there is enough disk space to create sub-shards.</li>
* <li>If splitByPrefix is true, make request to get prefix ranges.</li>
* <li>If this split was attempted previously and there are lingering sub-shards, delete them.</li>
* <li>Create sub-shards in CONSTRUCTION state.</li>
* <li>Add an initial replica to each sub-shard.</li>
* <li>Request that parent shard wait for children to become ACTIVE.</li>
* <li>Execute split: either LINK or REWRITE.</li>
* <li>Apply buffered updates to the sub-shards so they are up-to-date with parent.</li>
* <li>Determine node placement for additional replicas (but do not create yet).</li>
* <li>If replicationFactor is more than 1, set shard state for sub-shards to RECOVERY; else mark ACTIVE.</li>
* <li>Create additional replicas of sub-shards.</li>
* </ul>
*/
@SuppressWarnings({"rawtypes"})
public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<Object> 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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be called out in upgrade-notes, I think.

SolrIndexSplitter.SplitMethod splitMethod = SolrIndexSplitter.SplitMethod.get(methodStr);
if (splitMethod == null) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unknown value '" + CommonAdminParams.SPLIT_METHOD +
": " + methodStr);
"': " + methodStr);
}
boolean withTiming = message.getBool(CommonParams.TIMING, false);

Expand All @@ -114,7 +136,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
String splitKey = message.getStr("split.key");
DocCollection collection = clusterState.getCollection(collectionName);


// verify that parent shard is active; if not, throw exception
Slice parentSlice = getParentSlice(clusterState, collectionName, slice, splitKey);
if (parentSlice.getState() != Slice.State.ACTIVE) {
throw new SolrException(SolrException.ErrorCode.INVALID_STATE, "Parent slice is not active: " +
Expand All @@ -130,6 +152,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Interrupted.");
}

log.debug("Verifying that there is enough space on disk to create sub-shards");
RTimerTree t;
if (ocmh.overseer.getCoreContainer().getNodeConfig().getMetricsConfig().isEnabled()) {
t = timings.sub("checkDiskSpace");
Expand Down Expand Up @@ -190,8 +213,8 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
@SuppressWarnings("deprecation")
ShardHandler shardHandler = ocmh.shardHandlerFactory.getShardHandler();


if (message.getBool(CommonAdminParams.SPLIT_BY_PREFIX, false)) {
log.debug("Making request to SplitOp get doc prefix ranges for each sub-shard");
t = timings.sub("getRanges");

ModifiableSolrParams params = new ModifiableSolrParams();
Expand Down Expand Up @@ -233,6 +256,8 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
String rangesStr = fillRanges(ocmh.cloudManager, message, collection, parentSlice, subRanges, subSlices, subShardNames, firstNrtReplica);
t.stop();

// if this shard has attempted a split before and failed, there will be lingering INACTIVE sub-shards.
// clean these up before proceeding
boolean oldShardsDeleted = false;
for (String subSlice : subSlices) {
Slice oSlice = collection.getSlice(subSlice);
Expand Down Expand Up @@ -268,8 +293,8 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
collection = clusterState.getCollection(collectionName);
}

// create the child sub-shards in CONSTRUCTION state
String nodeName = parentShardLeader.getNodeName();

t = timings.sub("createSubSlicesAndLeadersInState");
for (int i = 0; i < subRanges.size(); i++) {
String subSlice = subSlices.get(i);
Expand Down Expand Up @@ -316,7 +341,6 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
ocmh.addReplica(clusterState, new ZkNodeProps(propMap), results, null);
}


{
final ShardRequestTracker syncRequestTracker = ocmh.syncRequestTracker();
String msgOnError = "SPLITSHARD failed to create subshard leaders";
Expand Down Expand Up @@ -357,6 +381,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
, parentShardLeader.getName(), slice, collectionName, parentShardLeader);
}

// execute actual split
ModifiableSolrParams params = new ModifiableSolrParams();
params.set(CoreAdminParams.ACTION, CoreAdminParams.CoreAdminAction.SPLIT.toString());
params.set(CommonAdminParams.SPLIT_METHOD, splitMethod.toLower());
Expand Down Expand Up @@ -407,15 +432,6 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O

log.debug("Successfully applied buffered updates on : {}", subShardNames);

// Replica creation for the new Slices

Set<String> nodes = clusterState.getLiveNodes();
List<String> nodeList = new ArrayList<>(nodes.size());
nodeList.addAll(nodes);

// Remove the node that hosts the parent shard for replica creation.
nodeList.remove(nodeName);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also cleaned this up because it was bugging me :) nodeList is never used.


// 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.
Expand Down Expand Up @@ -521,6 +537,7 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
// this ensures that the logic inside ReplicaMutator to update sub-shard state to 'active'
// always gets a chance to execute. See SOLR-7673

// 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
Expand Down Expand Up @@ -580,6 +597,10 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
if (withTiming) {
results.add(CommonParams.TIMING, timings.asNamedList());
}

if (log.isDebugEnabled()) {
log.debug("Timings for split sub-ops: " + timings);
}
success = true;
// don't unlock the shard yet - only do this if the final switch-over in
// ReplicaMutator succeeds (or fails)
Expand Down
48 changes: 48 additions & 0 deletions solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java
Expand Up @@ -28,6 +28,7 @@

import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.BaseHttpSolrClient;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
Expand All @@ -40,6 +41,7 @@
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.Slice;
import org.apache.solr.update.SolrIndexSplitter;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -106,6 +108,52 @@ public void doTest() throws IOException, SolrServerException {
}
}

/**
* Create a collection with 3 shards and split them each with a different splitMethod value.
* <ul>
* <li>No override specified. Verify that LINK method is used.</li>
* <li>REWRITE method specified. Verify that LINK steps are skipped.</li>
* <li>Invalid override specified. Verify that split fails.</li>
* </ul>
*/
@Test
public void testSplitMethods() throws Exception {
CollectionAdminRequest
.createCollection(COLLECTION_NAME, "conf", 3, 1)
.process(cluster.getSolrClient());

cluster.waitForActiveCollection(COLLECTION_NAME, 3, 3);

CollectionAdminRequest.SplitShard splitShard = CollectionAdminRequest.splitShard(COLLECTION_NAME)
.setShardName("shard1");
SolrResponse response = splitShard.process(cluster.getSolrClient());
assertTrue(response.getResponse().toString().contains("hardLinkCopy"));
megancarey marked this conversation as resolved.
Show resolved Hide resolved

waitForState("Timed out waiting for sub shards to be active. Number of active shards=" +
cluster.getSolrClient().getZkStateReader().getClusterState().getCollection(COLLECTION_NAME).getActiveSlices().size(),
COLLECTION_NAME, activeClusterShape(4, 5));

splitShard = CollectionAdminRequest.splitShard(COLLECTION_NAME)
.setNumSubShards(2)
.setShardName("shard2")
.setSplitMethod(SolrIndexSplitter.SplitMethod.REWRITE.name());
response = splitShard.process(cluster.getSolrClient());
assertFalse(response.getResponse().toString().contains("hardLinkCopy"));
waitForState("Timed out waiting for sub shards to be active. Number of active shards=" +
cluster.getSolrClient().getZkStateReader().getClusterState().getCollection(COLLECTION_NAME).getActiveSlices().size(),
COLLECTION_NAME, activeClusterShape(5, 7));

try {
splitShard = CollectionAdminRequest.splitShard(COLLECTION_NAME)
.setShardName("shard3")
.setSplitMethod("HELLO");
splitShard.process(cluster.getSolrClient());
fail("SplitShard should throw an exception when splitMethod is anything other than LINK or REWRITE");
} catch (BaseHttpSolrClient.RemoteSolrException ex) {
assertTrue(ex.getMessage().contains("Unknown value 'splitMethod': HELLO"));
}
}

@Test
public void multipleOptionsSplitTest() throws IOException, SolrServerException {
CollectionAdminRequest.SplitShard splitShard = CollectionAdminRequest.splitShard(COLLECTION_NAME)
Expand Down