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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4278b8a
Added logs and updated default splitMethod to be lINK
Jan 28, 2021
e91d3c6
Add test for splitMethods
Jan 28, 2021
f3723a0
Address review feedback
Jan 28, 2021
d071b40
Fix logging format for Gradle precommit
Jan 29, 2021
fcc005b
Addressing review feedback
Feb 22, 2021
0e87b02
Merge branch 'master' into jira/SOLR-15119
megancarey Feb 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 37 additions & 16 deletions solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,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 SplitOp.
*/
public class SplitShardCmd implements CollApiCmds.CollectionApiCommand {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private static final int MIN_NUM_SUB_SHARDS = 2;
Expand All @@ -81,16 +84,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 @@ -115,7 +137,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 @@ -131,6 +153,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 (ccc.getCoreContainer().getNodeConfig().getMetricsConfig().isEnabled()) {
t = timings.sub("checkDiskSpace");
Expand Down Expand Up @@ -191,8 +214,8 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
@SuppressWarnings("deprecation")
ShardHandler shardHandler = ccc.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 @@ -234,6 +257,8 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
String rangesStr = fillRanges(ccc.getSolrCloudManager(), 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 @@ -269,8 +294,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 @@ -322,7 +347,6 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
new AddReplicaCmd(ccc).addReplica(clusterState, new ZkNodeProps(propMap), results, null);
}


{
final ShardRequestTracker syncRequestTracker = CollectionHandlingUtils.syncRequestTracker(ccc);
String msgOnError = "SPLITSHARD failed to create subshard leaders";
Expand Down Expand Up @@ -363,6 +387,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 @@ -413,15 +438,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 @@ -550,6 +566,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 @@ -619,6 +636,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.toString());
}
success = true;
// don't unlock the shard yet - only do this if the final switch-over in
// ReplicaMutator succeeds (or fails)
Expand Down
55 changes: 55 additions & 0 deletions solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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,10 @@
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.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -106,6 +111,56 @@ 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());
MatcherAssert.assertThat("LINK split method is default, so split response should contain link timings.",
response.getResponse().toString().contains("hardLinkCopy"), Matchers.equalTo(true));

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());
MatcherAssert.assertThat("REWRITE split method was specified, so split response should NOT contain link timings.",
response.getResponse().toString().contains("hardLinkCopy"), Matchers.equalTo(false));

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) {
MatcherAssert.assertThat("Invalid split method HELLO was specified, so split should fail.",
ex.getMessage().contains("Unknown value 'splitMethod': HELLO"), Matchers.equalTo(true));
}
}

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