Remove only node preference

This commit removes the search preference _only_node as the same
functionality can be obtained by using the search preference
_only_nodes. This commit also adds a test that ensures that _only_nodes
will continue to support specifying node IDs.

Relates #18875
This commit is contained in:
Jason Tedor 2016-06-17 15:27:46 -04:00 committed by GitHub
parent 245def80f0
commit d09d89f8c5
8 changed files with 80 additions and 23 deletions

View File

@ -37,6 +37,7 @@ import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
@ -369,8 +370,13 @@ public class IndexShardRoutingTable implements Iterable<ShardRouting> {
}
}
if (ordered.isEmpty()) {
throw new IllegalArgumentException("no data nodes with critera(s) " +
Strings.arrayToCommaDelimitedString(nodeAttributes) + "] found for shard:" + shardId());
final String message = String.format(
Locale.ROOT,
"no data nodes with %s [%s] found for shard: %s",
nodeAttributes.length == 1 ? "criteria" : "criterion",
String.join(",", nodeAttributes),
shardId());
throw new IllegalArgumentException(message);
}
return new PlainShardIterator(shardId, ordered);
}

View File

@ -177,10 +177,6 @@ public class OperationRouting extends AbstractComponent {
return indexShard.replicaFirstActiveInitializingShardsIt();
case ONLY_LOCAL:
return indexShard.onlyNodeActiveInitializingShardsIt(localNodeId);
case ONLY_NODE:
String nodeId = preference.substring(Preference.ONLY_NODE.type().length() + 1);
ensureNodeIdExists(nodes, nodeId);
return indexShard.onlyNodeActiveInitializingShardsIt(nodeId);
case ONLY_NODES:
String nodeAttributes = preference.substring(Preference.ONLY_NODES.type().length() + 1);
return indexShard.onlyNodeSelectorActiveInitializingShardsIt(nodeAttributes.split(","), nodes);

View File

@ -64,11 +64,6 @@ public enum Preference {
*/
ONLY_LOCAL("_only_local"),
/**
* Route to specific node only
*/
ONLY_NODE("_only_node"),
/**
* Route to only node with attribute
*/
@ -100,8 +95,6 @@ public enum Preference {
return SHARDS;
case "_prefer_nodes":
return PREFER_NODES;
case "_only_node":
return ONLY_NODE;
case "_local":
return LOCAL;
case "_primary":

View File

@ -41,7 +41,9 @@ import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.object.HasToString.hasToString;
import static org.hamcrest.Matchers.equalTo;
public class OperationRoutingTests extends ESTestCase{
@ -211,8 +213,7 @@ public class OperationRoutingTests extends ESTestCase{
}
}
final ShardIterator it =
new OperationRouting(Settings.EMPTY, new ClusterSettings(Settings.EMPTY,
ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))
new OperationRouting(Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))
.getShards(clusterService.state(), indexName, 0, "_prefer_nodes:" + String.join(",", nodes));
final List<ShardRouting> all = new ArrayList<>();
ShardRouting shard;
@ -231,4 +232,63 @@ public class OperationRoutingTests extends ESTestCase{
}
}
public void testThatOnlyNodesSupportNodeIds() throws InterruptedException, IOException {
TestThreadPool threadPool = null;
ClusterService clusterService = null;
try {
threadPool = new TestThreadPool("testThatOnlyNodesSupportNodeIds");
clusterService = ClusterServiceUtils.createClusterService(threadPool);
final String indexName = "test";
ClusterServiceUtils.setState(clusterService, ClusterStateCreationUtils.stateWithActivePrimary(indexName, true, randomInt(8)));
final Index index = clusterService.state().metaData().index(indexName).getIndex();
final List<ShardRouting> shards = clusterService.state().getRoutingNodes().assignedShards(new ShardId(index, 0));
final int count = randomIntBetween(1, shards.size());
int position = 0;
final List<String> nodes = new ArrayList<>();
final List<ShardRouting> expected = new ArrayList<>();
for (int i = 0; i < count; i++) {
if (randomBoolean() && !shards.get(position).initializing()) {
nodes.add(shards.get(position).currentNodeId());
expected.add(shards.get(position));
position++;
} else {
nodes.add("missing_" + i);
}
}
if (expected.size() > 0) {
final ShardIterator it =
new OperationRouting(Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))
.getShards(clusterService.state(), indexName, 0, "_only_nodes:" + String.join(",", nodes));
final List<ShardRouting> only = new ArrayList<>();
ShardRouting shard;
while ((shard = it.nextOrNull()) != null) {
only.add(shard);
}
assertThat(new HashSet<>(only), equalTo(new HashSet<>(expected)));
} else {
final ClusterService cs = clusterService;
final IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> new OperationRouting(
Settings.EMPTY,
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))
.getShards(cs.state(), indexName, 0, "_only_nodes:" + String.join(",", nodes)));
if (nodes.size() == 1) {
assertThat(
e,
hasToString(containsString(
"no data nodes with criteria [" + String.join(",", nodes) + "] found for shard: [test][0]")));
} else {
assertThat(
e,
hasToString(containsString(
"no data nodes with criterion [" + String.join(",", nodes) + "] found for shard: [test][0]")));
}
}
} finally {
IOUtils.close(clusterService);
terminate(threadPool);
}
}
}

View File

@ -296,7 +296,7 @@ public class RecoveryWhileUnderLoadIT extends ESIntegTestCase {
if (docShard.id() == shard) {
for (ShardRouting shardRouting : state.routingTable().shardRoutingTable("test", shard)) {
GetResponse response = client().prepareGet("test", "type", Long.toString(id))
.setPreference("_only_node:" + shardRouting.currentNodeId()).get();
.setPreference("_only_nodes:" + shardRouting.currentNodeId()).get();
if (response.isExists()) {
logger.info("missing id [{}] on shard {}", id, shardRouting);
}

View File

@ -38,7 +38,9 @@ import java.util.Set;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasToString;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.greaterThan;
@ -157,10 +159,10 @@ public class SearchPreferenceIT extends ESIntegTestCase {
ensureGreen();
try {
client().prepareSearch().setQuery(matchAllQuery()).setPreference("_only_node:DOES-NOT-EXIST").execute().actionGet();
client().prepareSearch().setQuery(matchAllQuery()).setPreference("_only_nodes:DOES-NOT-EXIST").execute().actionGet();
fail("Expected IllegalArgumentException");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), is("No data node with id[DOES-NOT-EXIST] found"));
assertThat(e, hasToString(containsString("no data nodes with criteria [DOES-NOT-EXIST] found for shard: [test][")));
}
}

View File

@ -186,7 +186,11 @@ In the response for profiling queries, the `query_type` has been renamed to `typ
`description`. These changes have been made so the response format is more friendly to supporting other types of profiling
in the future.
==== Search prefernce
==== Search preferences
The <<search-request-preference,search preference>> `_only_node` has
been removed. The same behavior can be achieved by using `_only_nodes`
and specifying a single node ID.
The <<search-request-preference,search preference>> `_prefer_node` has
been superseded by `_prefer_nodes`. By specifying a single node,

View File

@ -27,10 +27,6 @@ The `preference` is a query string parameter which can be set to:
The operation will prefer to be executed on a local
allocated shard if possible.
`_only_node:xyz`::
Restricts the search to execute only on a node with
the provided node id (`xyz` in this case).
`_prefer_nodes:abc,xyz`::
Prefers execution on the nodes with the provided
node ids (`abc` or `xyz` in this case) if applicable.