From 33341600037bc72464ef56d6be5650ca99ac2069 Mon Sep 17 00:00:00 2001 From: Christian Haudum Date: Fri, 16 Oct 2015 12:21:46 +0200 Subject: [PATCH 01/23] improved building of disco nodes * improved retry policy of ec2 client * cache results for 10s --- docs/plugins/discovery-ec2.asciidoc | 6 ++- .../cloud/aws/AwsEc2Service.java | 1 + .../cloud/aws/AwsEc2ServiceImpl.java | 22 ++++++++++ .../ec2/AwsEc2UnicastHostsProvider.java | 33 ++++++++++++++ .../discovery/ec2/Ec2DiscoveryTests.java | 44 +++++++++++++++++++ 5 files changed, 105 insertions(+), 1 deletion(-) diff --git a/docs/plugins/discovery-ec2.asciidoc b/docs/plugins/discovery-ec2.asciidoc index 5ac208576df..a2b80495003 100644 --- a/docs/plugins/discovery-ec2.asciidoc +++ b/docs/plugins/discovery-ec2.asciidoc @@ -165,6 +165,11 @@ The following are a list of settings (prefixed with `discovery.ec2`) that can fu Defaults to `3s`. If no unit like `ms`, `s` or `m` is specified, milliseconds are used. +`node_cache_time`:: + + How long the list of hosts is cached to prevent further requests to the AWS API. + Defaults to `10s`. + [IMPORTANT] .Binding the network host @@ -195,7 +200,6 @@ as valid network host settings: |`_ec2_` |equivalent to _ec2:privateIpv4_. |================================================================== - [[discovery-ec2-permissions]] ===== Recommended EC2 Permissions diff --git a/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2Service.java b/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2Service.java index ab2b54633f4..a427b4af4ab 100644 --- a/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2Service.java +++ b/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2Service.java @@ -49,6 +49,7 @@ public interface AwsEc2Service extends LifecycleComponent { public static final String GROUPS = "discovery.ec2.groups"; public static final String TAG_PREFIX = "discovery.ec2.tag."; public static final String AVAILABILITY_ZONES = "discovery.ec2.availability_zones"; + public static final String NODE_CACHE_TIME = "discovery.ec2.node_cache_time"; } AmazonEC2 client(); diff --git a/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2ServiceImpl.java b/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2ServiceImpl.java index 26e001c2666..39ece106df8 100644 --- a/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2ServiceImpl.java +++ b/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2ServiceImpl.java @@ -19,10 +19,13 @@ package org.elasticsearch.cloud.aws; +import com.amazonaws.AmazonClientException; +import com.amazonaws.AmazonWebServiceRequest; import com.amazonaws.ClientConfiguration; import com.amazonaws.Protocol; import com.amazonaws.auth.*; import com.amazonaws.internal.StaticCredentialsProvider; +import com.amazonaws.retry.RetryPolicy; import com.amazonaws.services.ec2.AmazonEC2; import com.amazonaws.services.ec2.AmazonEC2Client; import org.elasticsearch.ElasticsearchException; @@ -36,6 +39,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; import java.util.Locale; +import java.util.Random; /** * @@ -103,6 +107,24 @@ public class AwsEc2ServiceImpl extends AbstractLifecycleComponent } } + // Increase the number of retries in case of 5xx API responses + final Random rand = new Random(); + RetryPolicy retryPolicy = new RetryPolicy( + RetryPolicy.RetryCondition.NO_RETRY_CONDITION, + new RetryPolicy.BackoffStrategy() { + @Override + public long delayBeforeNextRetry(AmazonWebServiceRequest originalRequest, + AmazonClientException exception, + int retriesAttempted) { + // with 10 retries the max delay time is 320s/320000ms (10 * 2^5 * 1 * 1000) + logger.warn("EC2 API request failed, retry again. Reason was:", exception); + return 1000L * (long) (10d * Math.pow(2, ((double) retriesAttempted) / 2.0d) * (1.0d + rand.nextDouble())); + } + }, + 10, + false); + clientConfiguration.setRetryPolicy(retryPolicy); + AWSCredentialsProvider credentials; if (account == null && key == null) { diff --git a/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2UnicastHostsProvider.java b/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2UnicastHostsProvider.java index 94c65047847..f7e70281a3d 100644 --- a/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2UnicastHostsProvider.java +++ b/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2UnicastHostsProvider.java @@ -31,6 +31,8 @@ import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.util.SingleObjectCache; import org.elasticsearch.discovery.zen.ping.unicast.UnicastHostsProvider; import org.elasticsearch.transport.TransportService; @@ -64,6 +66,8 @@ public class AwsEc2UnicastHostsProvider extends AbstractComponent implements Uni private final HostType hostType; + private final DiscoNodesCache discoNodes; + @Inject public AwsEc2UnicastHostsProvider(Settings settings, TransportService transportService, AwsEc2Service awsEc2Service, Version version) { super(settings); @@ -74,6 +78,9 @@ public class AwsEc2UnicastHostsProvider extends AbstractComponent implements Uni this.hostType = HostType.valueOf(settings.get(DISCOVERY_EC2.HOST_TYPE, "private_ip") .toUpperCase(Locale.ROOT)); + this.discoNodes = new DiscoNodesCache(this.settings.getAsTime(DISCOVERY_EC2.NODE_CACHE_TIME, + TimeValue.timeValueMillis(10_000L))); + this.bindAnyGroup = settings.getAsBoolean(DISCOVERY_EC2.ANY_GROUP, true); this.groups = new HashSet<>(); groups.addAll(Arrays.asList(settings.getAsArray(DISCOVERY_EC2.GROUPS))); @@ -94,6 +101,11 @@ public class AwsEc2UnicastHostsProvider extends AbstractComponent implements Uni @Override public List buildDynamicNodes() { + return discoNodes.getOrRefresh(); + } + + protected List fetchDynamicNodes() { + List discoNodes = new ArrayList<>(); DescribeInstancesResult descInstances; @@ -199,4 +211,25 @@ public class AwsEc2UnicastHostsProvider extends AbstractComponent implements Uni return describeInstancesRequest; } + + private final class DiscoNodesCache extends SingleObjectCache> { + + private boolean empty = true; + + protected DiscoNodesCache(TimeValue refreshInterval) { + super(refreshInterval, new ArrayList<>()); + } + + @Override + protected boolean needsRefresh() { + return (empty || super.needsRefresh()); + } + + @Override + protected List refresh() { + List nodes = fetchDynamicNodes(); + empty = nodes.isEmpty(); + return nodes; + } + } } diff --git a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java index ca9493bc4c9..4fc3faef316 100644 --- a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java +++ b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.transport.MockTransportService; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.local.LocalTransport; import org.junit.AfterClass; import org.junit.Before; @@ -231,4 +232,47 @@ public class Ec2DiscoveryTests extends ESTestCase { assertThat(discoveryNodes, hasSize(prodInstances)); } + abstract class DummyEc2HostProvider extends AwsEc2UnicastHostsProvider { + public int fetchCount = 0; + public DummyEc2HostProvider(Settings settings, TransportService transportService, AwsEc2Service service, Version version) { + super(settings, transportService, service, version); + } + } + + public void testGetNodeListEmptyCache() throws Exception { + AwsEc2Service awsEc2Service = new AwsEc2ServiceMock(Settings.EMPTY, 1, null); + DummyEc2HostProvider provider = new DummyEc2HostProvider(Settings.EMPTY, transportService, awsEc2Service, Version.CURRENT) { + @Override + protected List fetchDynamicNodes() { + fetchCount++; + return new ArrayList<>(); + } + }; + for (int i=0; i<3; i++) { + provider.buildDynamicNodes(); + } + assertEquals(provider.fetchCount, is(3)); + } + + public void testGetNodeListCached() throws Exception { + Settings.Builder builder = Settings.settingsBuilder() + .put(DISCOVERY_EC2.NODE_CACHE_TIME, "500ms"); + AwsEc2Service awsEc2Service = new AwsEc2ServiceMock(Settings.EMPTY, 1, null); + DummyEc2HostProvider provider = new DummyEc2HostProvider(builder.build(), transportService, awsEc2Service, Version.CURRENT) { + @Override + protected List fetchDynamicNodes() { + fetchCount++; + return Ec2DiscoveryTests.this.buildDynamicNodes(Settings.EMPTY, 1); + } + }; + for (int i=0; i<3; i++) { + provider.buildDynamicNodes(); + } + assertEquals(provider.fetchCount, is(1)); + Thread.sleep(1_000L); // wait for cache to expire + for (int i=0; i<3; i++) { + provider.buildDynamicNodes(); + } + assertEquals(provider.fetchCount, is(2)); + } } From a8268ba37dbe50e85bb7ef4d886a2faedc6f2f1e Mon Sep 17 00:00:00 2001 From: Jeff Destine Date: Thu, 29 Oct 2015 13:09:11 -0400 Subject: [PATCH 02/23] Adding US-Gov-West --- .../java/org/elasticsearch/cloud/aws/AwsEc2ServiceImpl.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2ServiceImpl.java b/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2ServiceImpl.java index 26e001c2666..7a26a4ff142 100644 --- a/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2ServiceImpl.java +++ b/plugins/discovery-ec2/src/main/java/org/elasticsearch/cloud/aws/AwsEc2ServiceImpl.java @@ -134,6 +134,8 @@ public class AwsEc2ServiceImpl extends AbstractLifecycleComponent endpoint = "ec2.us-west-2.amazonaws.com"; } else if (region.equals("ap-southeast") || region.equals("ap-southeast-1")) { endpoint = "ec2.ap-southeast-1.amazonaws.com"; + } else if (region.equals("us-gov-west") || region.equals("us-gov-west-1")) { + endpoint = "ec2.us-gov-west-1.amazonaws.com"; } else if (region.equals("ap-southeast-2")) { endpoint = "ec2.ap-southeast-2.amazonaws.com"; } else if (region.equals("ap-northeast") || region.equals("ap-northeast-1")) { From 72463768f506f854f0a9706de0a553d49f4b3e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 22 Oct 2015 14:20:34 +0200 Subject: [PATCH 03/23] Query DSL: Parsers should throw exception on unknown object This PR adds a randomized test to the query test base class that mutates an otherwise correct query by adding an additional object into the query hierarchy. Doing so makes the query illegal and should trigger some kind of exception. The new test revelead that some query parsers quietly return queries when called with such an illegal query. Those are also fixed here. Relates to #10974 --- .../termvectors/TermVectorsRequest.java | 5 +-- .../index/query/CommonTermsQueryParser.java | 11 ++++-- .../index/query/ExistsQueryParser.java | 6 ++- .../index/query/GeoShapeQueryParser.java | 10 +++-- .../index/query/IdsQueryParser.java | 8 ++-- .../index/query/MatchAllQueryParser.java | 4 +- .../index/query/MatchNoneQueryParser.java | 2 + .../index/query/MatchQueryParser.java | 8 ++-- .../index/query/MissingQueryParser.java | 4 +- .../index/query/MultiMatchQueryParser.java | 4 +- .../index/query/QueryStringQueryParser.java | 10 +++-- .../index/query/SimpleQueryStringParser.java | 2 + .../index/query/TermsQueryParser.java | 12 ++++-- .../cache/query/terms/TermsLookup.java | 11 ++++-- .../index/query/AbstractQueryTestCase.java | 26 +++++++++++++ .../query/HasChildQueryBuilderTests.java | 39 ++++++++++++++++++- .../query/HasParentQueryBuilderTests.java | 38 ++++++++++++++++++ .../query/SimpleQueryStringBuilderTests.java | 4 +- .../index/query/TermsQueryBuilderTests.java | 13 ++++--- 19 files changed, 178 insertions(+), 39 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/termvectors/TermVectorsRequest.java b/core/src/main/java/org/elasticsearch/action/termvectors/TermVectorsRequest.java index ef998922d51..c13e44097bc 100644 --- a/core/src/main/java/org/elasticsearch/action/termvectors/TermVectorsRequest.java +++ b/core/src/main/java/org/elasticsearch/action/termvectors/TermVectorsRequest.java @@ -19,7 +19,6 @@ package org.elasticsearch.action.termvectors; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.DocumentRequest; @@ -211,7 +210,7 @@ public class TermVectorsRequest extends SingleShardRequest i public String id() { return id; } - + /** * Sets the id of document the term vector is requested for. */ @@ -651,7 +650,7 @@ public class TermVectorsRequest extends SingleShardRequest i if (e.getValue() instanceof String) { mapStrStr.put(e.getKey(), (String) e.getValue()); } else { - throw new ElasticsearchException("expecting the analyzer at [{}] to be a String, but found [{}] instead", e.getKey(), e.getValue().getClass()); + throw new ElasticsearchParseException("expecting the analyzer at [{}] to be a String, but found [{}] instead", e.getKey(), e.getValue().getClass()); } } return mapStrStr; diff --git a/core/src/main/java/org/elasticsearch/index/query/CommonTermsQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/CommonTermsQueryParser.java index 86de4e31129..3ecbd11320e 100644 --- a/core/src/main/java/org/elasticsearch/index/query/CommonTermsQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/CommonTermsQueryParser.java @@ -39,7 +39,7 @@ public class CommonTermsQueryParser implements QueryParser { } else if ("boost".equals(currentFieldName)) { boost = parser.floatValue(); } else { - throw new ParsingException(parser.getTokenLocation(), "[exists] query does not support [" + currentFieldName + "]"); + throw new ParsingException(parser.getTokenLocation(), "[" + ExistsQueryBuilder.NAME + "] query does not support [" + currentFieldName + "]"); } + } else { + throw new ParsingException(parser.getTokenLocation(), "[" + ExistsQueryBuilder.NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } } if (fieldPattern == null) { - throw new ParsingException(parser.getTokenLocation(), "exists must be provided with a [field]"); + throw new ParsingException(parser.getTokenLocation(), "[" + ExistsQueryBuilder.NAME + "] must be provided with a [field]"); } ExistsQueryBuilder builder = new ExistsQueryBuilder(fieldPattern); diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryParser.java index e5198952c13..768600c3b38 100644 --- a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryParser.java @@ -70,8 +70,10 @@ public class GeoShapeQueryParser implements QueryParser { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); } else if (token == XContentParser.Token.START_OBJECT) { + if (fieldName != null) { + throw new ParsingException(parser.getTokenLocation(), "[" + GeoShapeQueryBuilder.NAME + "] point specified twice. [" + currentFieldName + "]"); + } fieldName = currentFieldName; - while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -104,10 +106,12 @@ public class GeoShapeQueryParser implements QueryParser { } else if (parseContext.parseFieldMatcher().match(currentFieldName, SHAPE_PATH_FIELD)) { shapePath = parser.text(); } + } else { + throw new ParsingException(parser.getTokenLocation(), "[" + GeoShapeQueryBuilder.NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } } } else { - throw new ParsingException(parser.getTokenLocation(), "[geo_shape] query does not support [" + currentFieldName + "]"); + throw new ParsingException(parser.getTokenLocation(), "[" + GeoShapeQueryBuilder.NAME + "] query does not support [" + currentFieldName + "]"); } } } @@ -117,7 +121,7 @@ public class GeoShapeQueryParser implements QueryParser { } else if (parseContext.parseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) { queryName = parser.text(); } else { - throw new ParsingException(parser.getTokenLocation(), "[geo_shape] query does not support [" + currentFieldName + "]"); + throw new ParsingException(parser.getTokenLocation(), "[" + GeoShapeQueryBuilder.NAME + "] query does not support [" + currentFieldName + "]"); } } } diff --git a/core/src/main/java/org/elasticsearch/index/query/IdsQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/IdsQueryParser.java index 0ffd31644e5..0496a690f5f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/IdsQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/IdsQueryParser.java @@ -79,7 +79,7 @@ public class IdsQueryParser implements QueryParser { types.add(value); } } else { - throw new ParsingException(parser.getTokenLocation(), "[ids] query does not support [" + currentFieldName + "]"); + throw new ParsingException(parser.getTokenLocation(), "[" + IdsQueryBuilder.NAME + "] query does not support [" + currentFieldName + "]"); } } else if (token.isValue()) { if ("type".equals(currentFieldName) || "_type".equals(currentFieldName)) { @@ -89,12 +89,14 @@ public class IdsQueryParser implements QueryParser { } else if ("_name".equals(currentFieldName)) { queryName = parser.text(); } else { - throw new ParsingException(parser.getTokenLocation(), "[ids] query does not support [" + currentFieldName + "]"); + throw new ParsingException(parser.getTokenLocation(), "[" + IdsQueryBuilder.NAME + "] query does not support [" + currentFieldName + "]"); } + } else { + throw new ParsingException(parser.getTokenLocation(), "[" + IdsQueryBuilder.NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } } if (!idsProvided) { - throw new ParsingException(parser.getTokenLocation(), "[ids] query, no ids values provided"); + throw new ParsingException(parser.getTokenLocation(), "[" + IdsQueryBuilder.NAME + "] query, no ids values provided"); } IdsQueryBuilder query = new IdsQueryBuilder(types.toArray(new String[types.size()])); diff --git a/core/src/main/java/org/elasticsearch/index/query/MatchAllQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/MatchAllQueryParser.java index 770f9c3dd67..6cab98838e0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MatchAllQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/MatchAllQueryParser.java @@ -52,8 +52,10 @@ public class MatchAllQueryParser implements QueryParser { } else if ("boost".equals(currentFieldName)) { boost = parser.floatValue(); } else { - throw new ParsingException(parser.getTokenLocation(), "[match_all] query does not support [" + currentFieldName + "]"); + throw new ParsingException(parser.getTokenLocation(), "[" + MatchAllQueryBuilder.NAME + "] query does not support [" + currentFieldName + "]"); } + } else { + throw new ParsingException(parser.getTokenLocation(), "[" + MatchAllQueryBuilder.NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } } MatchAllQueryBuilder queryBuilder = new MatchAllQueryBuilder(); diff --git a/core/src/main/java/org/elasticsearch/index/query/MatchNoneQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/MatchNoneQueryParser.java index 96b33f545f8..449824e72c9 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MatchNoneQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/MatchNoneQueryParser.java @@ -51,6 +51,8 @@ public class MatchNoneQueryParser implements QueryParser } else { throw new ParsingException(parser.getTokenLocation(), "["+MatchNoneQueryBuilder.NAME+"] query does not support [" + currentFieldName + "]"); } + } else { + throw new ParsingException(parser.getTokenLocation(), "[" + MatchNoneQueryBuilder.NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } } diff --git a/core/src/main/java/org/elasticsearch/index/query/MatchQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/MatchQueryParser.java index afcf25ca2a7..c50256480a0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MatchQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/MatchQueryParser.java @@ -55,7 +55,7 @@ public class MatchQueryParser implements QueryParser { XContentParser.Token token = parser.nextToken(); if (token != XContentParser.Token.FIELD_NAME) { - throw new ParsingException(parser.getTokenLocation(), "[match] query malformed, no field"); + throw new ParsingException(parser.getTokenLocation(), "[" + MatchQueryBuilder.NAME + "] query malformed, no field"); } String fieldName = parser.currentName(); @@ -93,7 +93,7 @@ public class MatchQueryParser implements QueryParser { } else if ("phrase_prefix".equals(tStr) || "phrasePrefix".equals(currentFieldName)) { type = MatchQuery.Type.PHRASE_PREFIX; } else { - throw new ParsingException(parser.getTokenLocation(), "[match] query does not support type " + tStr); + throw new ParsingException(parser.getTokenLocation(), "[" + MatchQueryBuilder.NAME + "] query does not support type " + tStr); } } else if ("analyzer".equals(currentFieldName)) { analyzer = parser.text(); @@ -131,8 +131,10 @@ public class MatchQueryParser implements QueryParser { } else if ("_name".equals(currentFieldName)) { queryName = parser.text(); } else { - throw new ParsingException(parser.getTokenLocation(), "[match] query does not support [" + currentFieldName + "]"); + throw new ParsingException(parser.getTokenLocation(), "[" + MatchQueryBuilder.NAME + "] query does not support [" + currentFieldName + "]"); } + } else { + throw new ParsingException(parser.getTokenLocation(), "[" + MatchQueryBuilder.NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } } parser.nextToken(); diff --git a/core/src/main/java/org/elasticsearch/index/query/MissingQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/MissingQueryParser.java index 8d8c5aec01f..3a696dad38f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MissingQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/MissingQueryParser.java @@ -61,8 +61,10 @@ public class MissingQueryParser implements QueryParser { } else if ("boost".equals(currentFieldName)) { boost = parser.floatValue(); } else { - throw new ParsingException(parser.getTokenLocation(), "[missing] query does not support [" + currentFieldName + "]"); + throw new ParsingException(parser.getTokenLocation(), "[" + MissingQueryBuilder.NAME + "] query does not support [" + currentFieldName + "]"); } + } else { + throw new ParsingException(parser.getTokenLocation(), "[" + MissingQueryBuilder.NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } } diff --git a/core/src/main/java/org/elasticsearch/index/query/MultiMatchQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/MultiMatchQueryParser.java index c86d0295577..d52f6707aa1 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MultiMatchQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/MultiMatchQueryParser.java @@ -122,8 +122,10 @@ public class MultiMatchQueryParser implements QueryParser, ToXContent { public TermsLookup(String index, String type, String id, String path) { if (id == null) { - throw new IllegalArgumentException("[terms] query lookup element requires specifying the id."); + throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the id."); } if (type == null) { - throw new IllegalArgumentException("[terms] query lookup element requires specifying the type."); + throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the type."); } if (path == null) { - throw new IllegalArgumentException("[terms] query lookup element requires specifying the path."); + throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the path."); } this.index = index; this.type = type; @@ -122,9 +123,11 @@ public class TermsLookup implements Writeable, ToXContent { path = parser.text(); break; default: - throw new ParsingException(parser.getTokenLocation(), "[terms] query does not support [" + currentFieldName + throw new ParsingException(parser.getTokenLocation(), "[" + TermsQueryBuilder.NAME + "] query does not support [" + currentFieldName + "] within lookup element"); } + } else { + throw new ParsingException(parser.getTokenLocation(), "[" + TermsQueryBuilder.NAME + "] unknown token [" + token + "] after [" + currentFieldName + "]"); } } return new TermsLookup(index, type, id, path).routing(routing); diff --git a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java index 212dd457171..c553e94e29f 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java @@ -20,10 +20,12 @@ package org.elasticsearch.index.query; import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; +import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.io.JsonStringEncoder; import org.apache.lucene.search.Query; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.get.GetRequest; @@ -76,6 +78,7 @@ import org.elasticsearch.indices.IndicesWarmer; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.script.*; +import org.elasticsearch.script.Script.ScriptParseException; import org.elasticsearch.script.mustache.MustacheScriptEngineService; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.ESTestCase; @@ -344,6 +347,29 @@ public abstract class AbstractQueryTestCase> // we'd like to see the offending field name here assertThat(e.getMessage(), containsString("bogusField")); } + } + + /** + * Test that adding additional object into otherwise correct query string + * should always trigger some kind of Parsing Exception. + */ + public void testUnknownObjectException() throws IOException { + String validQuery = createTestQueryBuilder().toString(); + assertThat(validQuery, containsString("{")); + for (int insertionPosition = 0; insertionPosition < validQuery.length(); insertionPosition++) { + if (validQuery.charAt(insertionPosition) == '{') { + String testQuery = validQuery.substring(0, insertionPosition) + "{ \"newField\" : " + validQuery.substring(insertionPosition) + "}"; + try { + parseQuery(testQuery); + fail("some parsing exception expected for query: " + testQuery); + } catch (ParsingException | ScriptParseException | ElasticsearchParseException e) { + // different kinds of exception wordings depending on location + // of mutation, so no simple asserts possible here + } catch (JsonParseException e) { + // mutation produced invalid json + } + } + } } /** diff --git a/core/src/test/java/org/elasticsearch/index/query/HasChildQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/HasChildQueryBuilderTests.java index c567ba3a0f3..a6c63f83c96 100644 --- a/core/src/test/java/org/elasticsearch/index/query/HasChildQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/HasChildQueryBuilderTests.java @@ -20,11 +20,15 @@ package org.elasticsearch.index.query; import com.carrotsearch.randomizedtesting.generators.RandomPicks; +import com.fasterxml.jackson.core.JsonParseException; + import org.apache.lucene.queries.TermsQuery; import org.apache.lucene.search.*; import org.apache.lucene.search.join.ScoreMode; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -35,6 +39,7 @@ import org.elasticsearch.index.mapper.Uid; import org.elasticsearch.index.mapper.internal.TypeFieldMapper; import org.elasticsearch.index.mapper.internal.UidFieldMapper; import org.elasticsearch.index.query.support.QueryInnerHits; +import org.elasticsearch.script.Script.ScriptParseException; import org.elasticsearch.search.fetch.innerhits.InnerHitsBuilder; import org.elasticsearch.search.fetch.innerhits.InnerHitsContext; import org.elasticsearch.search.internal.SearchContext; @@ -44,7 +49,8 @@ import org.elasticsearch.test.TestSearchContext; import java.io.IOException; import java.util.Collections; -import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath; +import static org.hamcrest.Matchers.containsString; + import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; @@ -52,6 +58,7 @@ public class HasChildQueryBuilderTests extends AbstractQueryTestCase { private List randomTerms; @@ -195,9 +198,9 @@ public class TermsQueryBuilderTests extends AbstractQueryTestCase Date: Mon, 2 Nov 2015 14:38:03 +0100 Subject: [PATCH 04/23] Adding US-Gov-West for S3 Follow up for #14358 --- .../java/org/elasticsearch/cloud/aws/InternalAwsS3Service.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/InternalAwsS3Service.java b/plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/InternalAwsS3Service.java index 81b7e315b69..4752a3f80b2 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/InternalAwsS3Service.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/cloud/aws/InternalAwsS3Service.java @@ -193,6 +193,8 @@ public class InternalAwsS3Service extends AbstractLifecycleComponent Date: Mon, 2 Nov 2015 15:33:36 +0100 Subject: [PATCH 05/23] Fix test for ec2 discovery See #14155 --- .../org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java index 4fc3faef316..6f88be2be5a 100644 --- a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java +++ b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java @@ -251,7 +251,7 @@ public class Ec2DiscoveryTests extends ESTestCase { for (int i=0; i<3; i++) { provider.buildDynamicNodes(); } - assertEquals(provider.fetchCount, is(3)); + assertThat(provider.fetchCount, is(3)); } public void testGetNodeListCached() throws Exception { @@ -268,11 +268,11 @@ public class Ec2DiscoveryTests extends ESTestCase { for (int i=0; i<3; i++) { provider.buildDynamicNodes(); } - assertEquals(provider.fetchCount, is(1)); + assertThat(provider.fetchCount, is(1)); Thread.sleep(1_000L); // wait for cache to expire for (int i=0; i<3; i++) { provider.buildDynamicNodes(); } - assertEquals(provider.fetchCount, is(2)); + assertThat(provider.fetchCount, is(2)); } } From 9056ebb20dfa8219ce8fa581a1e2eef479329433 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Mon, 2 Nov 2015 09:39:14 -0500 Subject: [PATCH 06/23] gradle cleanEclipse should completely nuke .settings. Today this will only remove `.settings/org.eclipse.jdt.core.prefs`, leaving a bunch of stale eclipse configuration everywhere. --- .settings/org.eclipse.core.resources.prefs | 6 ------ .settings/org.eclipse.jdt.core.prefs | 18 ------------------ .settings/org.eclipse.jdt.ui.prefs | 6 ------ build.gradle | 5 +++++ 4 files changed, 5 insertions(+), 30 deletions(-) delete mode 100644 .settings/org.eclipse.core.resources.prefs delete mode 100644 .settings/org.eclipse.jdt.core.prefs delete mode 100644 .settings/org.eclipse.jdt.ui.prefs diff --git a/.settings/org.eclipse.core.resources.prefs b/.settings/org.eclipse.core.resources.prefs deleted file mode 100644 index 5731b2f8244..00000000000 --- a/.settings/org.eclipse.core.resources.prefs +++ /dev/null @@ -1,6 +0,0 @@ -eclipse.preferences.version=1 -encoding//src/main/java=UTF-8 -encoding//src/main/resources=UTF-8 -encoding//src/test/resources=UTF-8 -encoding/=UTF-8 -encoding/rest-api-spec=UTF-8 diff --git a/.settings/org.eclipse.jdt.core.prefs b/.settings/org.eclipse.jdt.core.prefs deleted file mode 100644 index f1163fdd583..00000000000 --- a/.settings/org.eclipse.jdt.core.prefs +++ /dev/null @@ -1,18 +0,0 @@ -eclipse.preferences.version=1 -org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations=enabled -org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation=ignore -org.eclipse.jdt.core.compiler.annotation.nullable=org.elasticsearch.common.Nullable -org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled -org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.7 -org.eclipse.jdt.core.compiler.compliance=1.7 -org.eclipse.jdt.core.compiler.problem.forbiddenReference=warning -org.eclipse.jdt.core.compiler.problem.nonnullParameterAnnotationDropped=warning -org.eclipse.jdt.core.compiler.problem.nullAnnotationInferenceConflict=warning -org.eclipse.jdt.core.compiler.problem.nullReference=warning -org.eclipse.jdt.core.compiler.problem.nullSpecViolation=warning -org.eclipse.jdt.core.compiler.problem.nullUncheckedConversion=warning -org.eclipse.jdt.core.compiler.problem.potentialNullReference=warning -org.eclipse.jdt.core.compiler.source=1.7 -org.eclipse.jdt.core.formatter.lineSplit=140 -org.eclipse.jdt.core.formatter.tabulation.char=space -org.eclipse.jdt.core.formatter.tabulation.size=4 diff --git a/.settings/org.eclipse.jdt.ui.prefs b/.settings/org.eclipse.jdt.ui.prefs deleted file mode 100644 index 4a9959fc9fc..00000000000 --- a/.settings/org.eclipse.jdt.ui.prefs +++ /dev/null @@ -1,6 +0,0 @@ -eclipse.preferences.version=1 -formatter_settings_version=12 -# Intellij IDEA import order -org.eclipse.jdt.ui.importorder=;com;org;java;javax;\#; -# License header -org.eclipse.jdt.ui.text.custom_code_templates= diff --git a/build.gradle b/build.gradle index 621f137eb94..93406d4965e 100644 --- a/build.gradle +++ b/build.gradle @@ -178,6 +178,11 @@ allprojects { } } } + task cleanEclipseSettings(type: Delete) { + delete '.settings' + } + // otherwise .settings is not nuked entirely + tasks.cleanEclipse.dependsOn(cleanEclipseSettings) // otherwise the eclipse merging is *super confusing* tasks.eclipse.dependsOn(cleanEclipse) } From 4587a94fcf821f9d772eab44e69e8c25f2ae100f Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Mon, 2 Nov 2015 10:44:51 -0500 Subject: [PATCH 07/23] generate complete eclipse configuration from 'gradle eclipse' --- build.gradle | 6 ++++++ .../org.eclipse.core.resources.prefs | 6 ++++++ .../org.eclipse.jdt.core.prefs | 19 +++++++++++++++++++ .../eclipse.settings/org.eclipse.jdt.ui.prefs | 6 ++++++ 4 files changed, 37 insertions(+) create mode 100644 buildSrc/src/main/resources/eclipse.settings/org.eclipse.core.resources.prefs create mode 100644 buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.core.prefs create mode 100644 buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs diff --git a/build.gradle b/build.gradle index 93406d4965e..9adb3aeb0eb 100644 --- a/build.gradle +++ b/build.gradle @@ -181,10 +181,16 @@ allprojects { task cleanEclipseSettings(type: Delete) { delete '.settings' } + task copyEclipseSettings(type: Copy) { + // TODO: "package this up" for external builds + from new File(project.rootDir, 'buildSrc/src/main/resources/eclipse.settings') + into '.settings' + } // otherwise .settings is not nuked entirely tasks.cleanEclipse.dependsOn(cleanEclipseSettings) // otherwise the eclipse merging is *super confusing* tasks.eclipse.dependsOn(cleanEclipse) + tasks.eclipse.dependsOn(copyEclipseSettings) } diff --git a/buildSrc/src/main/resources/eclipse.settings/org.eclipse.core.resources.prefs b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.core.resources.prefs new file mode 100644 index 00000000000..6fd0a9aab13 --- /dev/null +++ b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.core.resources.prefs @@ -0,0 +1,6 @@ +eclipse.preferences.version=1 +encoding//src/main/java=UTF-8 +encoding//src/main/resources=UTF-8 +encoding//src/test/java=UTF-8 +encoding//src/test/resources=UTF-8 +encoding/=UTF-8 \ No newline at end of file diff --git a/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.core.prefs b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.core.prefs new file mode 100644 index 00000000000..f9b349c46d1 --- /dev/null +++ b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.core.prefs @@ -0,0 +1,19 @@ +eclipse.preferences.version=1 + +# previous configuration from maven build +# this is merged with gradle's generated properties during 'gradle eclipse' + +org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations=enabled +org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation=ignore +org.eclipse.jdt.core.compiler.annotation.nullable=org.elasticsearch.common.Nullable +org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled +org.eclipse.jdt.core.compiler.problem.forbiddenReference=warning +org.eclipse.jdt.core.compiler.problem.nonnullParameterAnnotationDropped=warning +org.eclipse.jdt.core.compiler.problem.nullAnnotationInferenceConflict=warning +org.eclipse.jdt.core.compiler.problem.nullReference=warning +org.eclipse.jdt.core.compiler.problem.nullSpecViolation=warning +org.eclipse.jdt.core.compiler.problem.nullUncheckedConversion=warning +org.eclipse.jdt.core.compiler.problem.potentialNullReference=warning +org.eclipse.jdt.core.formatter.lineSplit=140 +org.eclipse.jdt.core.formatter.tabulation.char=space +org.eclipse.jdt.core.formatter.tabulation.size=4 diff --git a/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs new file mode 100644 index 00000000000..a42667f8698 --- /dev/null +++ b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs @@ -0,0 +1,6 @@ +eclipse.preferences.version=1 +formatter_settings_version=12 +# Intellij IDEA import order +org.eclipse.jdt.ui.importorder=;com;org;java;javax;\#; +# License header +org.eclipse.jdt.ui.text.custom_code_templates= \ No newline at end of file From 951bc5a4a7c8e56b274f9b0e0e1959c7217ffb45 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Mon, 2 Nov 2015 10:55:31 -0500 Subject: [PATCH 08/23] disable null pointer analysis so the IDE is not crashing. --- .../org.eclipse.jdt.core.prefs | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.core.prefs b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.core.prefs index f9b349c46d1..9bee5e587b0 100644 --- a/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.core.prefs +++ b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.core.prefs @@ -3,17 +3,20 @@ eclipse.preferences.version=1 # previous configuration from maven build # this is merged with gradle's generated properties during 'gradle eclipse' -org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations=enabled -org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation=ignore -org.eclipse.jdt.core.compiler.annotation.nullable=org.elasticsearch.common.Nullable -org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled +# NOTE: null pointer analysis etc is not enabled currently, it seems very unstable +# (e.g. crashing eclipse etc) +# org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations=enabled +# org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation=ignore +# org.eclipse.jdt.core.compiler.annotation.nullable=org.elasticsearch.common.Nullable +# org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled +# org.eclipse.jdt.core.compiler.problem.nonnullParameterAnnotationDropped=warning +# org.eclipse.jdt.core.compiler.problem.nullAnnotationInferenceConflict=warning +# org.eclipse.jdt.core.compiler.problem.nullReference=warning +# org.eclipse.jdt.core.compiler.problem.nullSpecViolation=warning +# org.eclipse.jdt.core.compiler.problem.nullUncheckedConversion=warning +# org.eclipse.jdt.core.compiler.problem.potentialNullReference=warning + org.eclipse.jdt.core.compiler.problem.forbiddenReference=warning -org.eclipse.jdt.core.compiler.problem.nonnullParameterAnnotationDropped=warning -org.eclipse.jdt.core.compiler.problem.nullAnnotationInferenceConflict=warning -org.eclipse.jdt.core.compiler.problem.nullReference=warning -org.eclipse.jdt.core.compiler.problem.nullSpecViolation=warning -org.eclipse.jdt.core.compiler.problem.nullUncheckedConversion=warning -org.eclipse.jdt.core.compiler.problem.potentialNullReference=warning org.eclipse.jdt.core.formatter.lineSplit=140 org.eclipse.jdt.core.formatter.tabulation.char=space org.eclipse.jdt.core.formatter.tabulation.size=4 From 1e00b854110c444a143543d8be1e599398748a94 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Mon, 2 Nov 2015 11:45:55 -0500 Subject: [PATCH 09/23] Fix auto-generated eclipse try/catch to be less trappy When generating a try-catch block, the eclipse default is something like this: ``` try { something(); } catch (Exception e) { // TODO: auto-generated stub e.printStackTrace(); } which is terrible, so the ES eclipse changes this to rethrow a RuntimeException instead. ``` try { something(); } catch (Exception e) { throw new RuntimeException(); } ``` Unfortunately, this loses the original exception entirely, instead it should be: ``` try { something(); } catch (Exception e) { throw new RuntimeException(e); } ``` --- .../main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs index a42667f8698..c7c10803c30 100644 --- a/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs +++ b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs @@ -3,4 +3,4 @@ formatter_settings_version=12 # Intellij IDEA import order org.eclipse.jdt.ui.importorder=;com;org;java;javax;\#; # License header -org.eclipse.jdt.ui.text.custom_code_templates= \ No newline at end of file +org.eclipse.jdt.ui.text.custom_code_templates= From 73a136e9fc38c7df64799cc0698ae20ea838b042 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Mon, 2 Nov 2015 11:58:34 -0500 Subject: [PATCH 10/23] Use ${exception_var} for the case of nested generated try/catch --- .../main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs index c7c10803c30..391a8715868 100644 --- a/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs +++ b/buildSrc/src/main/resources/eclipse.settings/org.eclipse.jdt.ui.prefs @@ -3,4 +3,4 @@ formatter_settings_version=12 # Intellij IDEA import order org.eclipse.jdt.ui.importorder=;com;org;java;javax;\#; # License header -org.eclipse.jdt.ui.text.custom_code_templates= +org.eclipse.jdt.ui.text.custom_code_templates= From f83800b1d283fc61b65772b4dfb90832450b5ec9 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Mon, 2 Nov 2015 12:05:44 -0500 Subject: [PATCH 11/23] don't discard original exception (suspiciously/possibly eclipse-generated) --- .../org/elasticsearch/index/mapper/object/ObjectMapper.java | 2 +- .../scripts/score/script/NativeNaiveTFIDFScoreScript.java | 2 +- .../elasticsearch/common/util/concurrent/CountDownTests.java | 2 +- .../http/netty/NettyHttpServerPipeliningTests.java | 2 +- .../java/org/elasticsearch/transport/netty/KeyedLockTests.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java index fc47fc561cb..9dc1a1d3dc7 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java @@ -355,7 +355,7 @@ public class ObjectMapper extends Mapper implements AllFieldMapper.IncludeInAll, try { clone = (ObjectMapper) super.clone(); } catch (CloneNotSupportedException e) { - throw new RuntimeException(); + throw new RuntimeException(e); } return clone; } diff --git a/core/src/test/java/org/elasticsearch/benchmark/scripts/score/script/NativeNaiveTFIDFScoreScript.java b/core/src/test/java/org/elasticsearch/benchmark/scripts/score/script/NativeNaiveTFIDFScoreScript.java index 9d6a1cd2f07..e96b35df96a 100644 --- a/core/src/test/java/org/elasticsearch/benchmark/scripts/score/script/NativeNaiveTFIDFScoreScript.java +++ b/core/src/test/java/org/elasticsearch/benchmark/scripts/score/script/NativeNaiveTFIDFScoreScript.java @@ -70,7 +70,7 @@ public class NativeNaiveTFIDFScoreScript extends AbstractSearchScript { score += indexFieldTerm.tf() * indexField.docCount() / indexFieldTerm.df(); } } catch (IOException e) { - throw new RuntimeException(); + throw new RuntimeException(e); } } return score; diff --git a/core/src/test/java/org/elasticsearch/common/util/concurrent/CountDownTests.java b/core/src/test/java/org/elasticsearch/common/util/concurrent/CountDownTests.java index db10addfe2e..1a32064fe7d 100644 --- a/core/src/test/java/org/elasticsearch/common/util/concurrent/CountDownTests.java +++ b/core/src/test/java/org/elasticsearch/common/util/concurrent/CountDownTests.java @@ -43,7 +43,7 @@ public class CountDownTests extends ESTestCase { try { latch.await(); } catch (InterruptedException e) { - throw new RuntimeException(); + throw new RuntimeException(e); } while (true) { if(frequently()) { diff --git a/core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerPipeliningTests.java b/core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerPipeliningTests.java index 53c743d34d5..b675d29c9da 100644 --- a/core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerPipeliningTests.java +++ b/core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerPipeliningTests.java @@ -214,7 +214,7 @@ public class NettyHttpServerPipeliningTests extends ESTestCase { Thread.sleep(timeout); } catch (InterruptedException e1) { Thread.currentThread().interrupt(); - throw new RuntimeException(); + throw new RuntimeException(e1); } } diff --git a/core/src/test/java/org/elasticsearch/transport/netty/KeyedLockTests.java b/core/src/test/java/org/elasticsearch/transport/netty/KeyedLockTests.java index cbac0aded0e..9581dfff42f 100644 --- a/core/src/test/java/org/elasticsearch/transport/netty/KeyedLockTests.java +++ b/core/src/test/java/org/elasticsearch/transport/netty/KeyedLockTests.java @@ -112,7 +112,7 @@ public class KeyedLockTests extends ESTestCase { try { startLatch.await(); } catch (InterruptedException e) { - throw new RuntimeException(); + throw new RuntimeException(e); } int numRuns = scaledRandomIntBetween(5000, 50000); for (int i = 0; i < numRuns; i++) { From b7f8e5c1df5e3369c0cf2f2e47109a8fb2ab6f6b Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 2 Nov 2015 10:14:04 -0800 Subject: [PATCH 12/23] Build: Make idea/eclipse project generation build generated resources for plugins This adds a generated-resources dir that the plugin properties are generated into. This must be outside of the build dir, since intellij has build as "excluded". closes #14392 --- .gitignore | 1 + build.gradle | 2 +- .../elasticsearch/gradle/plugin/PluginBuildPlugin.groovy | 8 +++++--- .../gradle/plugin/PluginPropertiesTask.groovy | 6 +++++- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index cd5cdbd1839..31f2aa5fc66 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ work/ logs/ .DS_Store build/ +generated-resources/ **/.local* docs/html/ docs/build.log diff --git a/build.gradle b/build.gradle index 9adb3aeb0eb..b3e5e09e07a 100644 --- a/build.gradle +++ b/build.gradle @@ -153,7 +153,7 @@ allprojects { apply plugin: 'idea' } -if (hasProperty('projectsPrefix') == false) { +if (projectsPrefix.isEmpty()) { idea { project { languageLevel = sourceCompatibility diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy index cf9a1ff7302..63dd6f2beae 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy @@ -84,9 +84,11 @@ class PluginBuildPlugin extends BuildPlugin { static Task configureBundleTask(Project project) { PluginPropertiesTask buildProperties = project.tasks.create(name: 'pluginProperties', type: PluginPropertiesTask) File pluginMetadata = project.file("src/main/plugin-metadata") - project.processTestResources { - from buildProperties - from pluginMetadata + project.sourceSets.test { + output.dir(buildProperties.propertiesFile.parentFile, builtBy: 'pluginProperties') + resources { + srcDir pluginMetadata + } } Task bundle = project.tasks.create(name: 'bundlePlugin', type: Zip, dependsOn: [project.jar, buildProperties]) bundle.configure { diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy index 202de784f15..6dc5e2178c3 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy @@ -33,9 +33,13 @@ class PluginPropertiesTask extends DefaultTask { PluginPropertiesExtension extension Map properties = new HashMap<>() + File generatedResourcesDir = new File(project.projectDir, "generated-resources") PluginPropertiesTask() { extension = project.extensions.create('esplugin', PluginPropertiesExtension, project) + project.clean { + delete generatedResourcesDir + } project.afterEvaluate { if (extension.description == null) { throw new InvalidUserDataException('description is a required setting for esplugin') @@ -54,7 +58,7 @@ class PluginPropertiesTask extends DefaultTask { } @OutputFile - File propertiesFile = new File(project.buildDir, "plugin" + File.separator + "plugin-descriptor.properties") + File propertiesFile = new File(generatedResourcesDir, "plugin-descriptor.properties") void fillProperties() { // TODO: need to copy the templated plugin-descriptor with a dependent task, since copy requires a file (not uri) From 5bfecd4f76a7f05aab17eb70545ac11361e08dc0 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 2 Nov 2015 12:30:16 -0800 Subject: [PATCH 13/23] Build: Simplify plugin properties generation The gradle task to generate plugin properties files now is a simple copy with expansion (ie maven filtering). It also no longer depends on compiling. closes #14450 --- .../gradle/plugin/PluginBuildPlugin.groovy | 2 +- .../gradle/plugin/PluginPropertiesTask.groovy | 77 ++++++++----------- .../resources/plugin-descriptor.properties | 18 ++--- 3 files changed, 41 insertions(+), 56 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy index 63dd6f2beae..6c9c3f16d7c 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy @@ -85,7 +85,7 @@ class PluginBuildPlugin extends BuildPlugin { PluginPropertiesTask buildProperties = project.tasks.create(name: 'pluginProperties', type: PluginPropertiesTask) File pluginMetadata = project.file("src/main/plugin-metadata") project.sourceSets.test { - output.dir(buildProperties.propertiesFile.parentFile, builtBy: 'pluginProperties') + output.dir(buildProperties.generatedResourcesDir, builtBy: 'pluginProperties') resources { srcDir pluginMetadata } diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy index 6dc5e2178c3..be81d8826d8 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy @@ -19,81 +19,66 @@ package org.elasticsearch.gradle.plugin import org.elasticsearch.gradle.ElasticsearchProperties -import org.gradle.api.DefaultTask import org.gradle.api.InvalidUserDataException -import org.gradle.api.tasks.OutputFile -import org.gradle.api.tasks.TaskAction +import org.gradle.api.Task +import org.gradle.api.tasks.Copy /** * Creates a plugin descriptor. - * - * TODO: copy the example properties file to plugin documentation */ -class PluginPropertiesTask extends DefaultTask { +class PluginPropertiesTask extends Copy { PluginPropertiesExtension extension - Map properties = new HashMap<>() - File generatedResourcesDir = new File(project.projectDir, "generated-resources") + File generatedResourcesDir = new File(project.projectDir, 'generated-resources') PluginPropertiesTask() { + File templateFile = new File(project.buildDir, 'templates/plugin-descriptor.properties') + Task copyPluginPropertiesTemplate = project.tasks.create('copyPluginPropertiesTemplate') { + doLast { + InputStream resourceTemplate = PluginPropertiesTask.getResourceAsStream('/plugin-descriptor.properties') + templateFile.parentFile.mkdirs() + templateFile.setText(resourceTemplate.getText('UTF-8'), 'UTF-8') + } + } + dependsOn(copyPluginPropertiesTemplate) extension = project.extensions.create('esplugin', PluginPropertiesExtension, project) project.clean { delete generatedResourcesDir } project.afterEvaluate { + // check require properties are set if (extension.description == null) { throw new InvalidUserDataException('description is a required setting for esplugin') } if (extension.jvm && extension.classname == null) { throw new InvalidUserDataException('classname is a required setting for esplugin with jvm=true') } - if (extension.jvm) { - dependsOn(project.classes) // so we can check for the classname - } - fillProperties() configure { - inputs.properties(properties) + doFirst { + if (extension.jvm && extension.isolated == false) { + String warning = "WARNING: Disabling plugin isolation in ${project.name} is deprecated and will be removed in the future" + logger.warn("${'=' * warning.length()}\n${warning}\n${'=' * warning.length()}") + } + } + // configure property substitution + from templateFile + into generatedResourcesDir + expand(generateSubstitutions()) } } } - @OutputFile - File propertiesFile = new File(generatedResourcesDir, "plugin-descriptor.properties") - - void fillProperties() { - // TODO: need to copy the templated plugin-descriptor with a dependent task, since copy requires a file (not uri) - properties = [ + Map generateSubstitutions() { + return [ 'name': extension.name, 'description': extension.description, 'version': extension.version, - 'elasticsearch.version': ElasticsearchProperties.version, + 'elasticsearchVersion': ElasticsearchProperties.version, + 'javaVersion': project.targetCompatibility as String, 'jvm': extension.jvm as String, - 'site': extension.site as String + 'site': extension.site as String, + 'isolated': extension.isolated as String, + 'classname': extension.jvm ? extension.classname : 'NA' ] - if (extension.jvm) { - properties['classname'] = extension.classname - properties['isolated'] = extension.isolated as String - properties['java.version'] = project.targetCompatibility as String - } - } - - @TaskAction - void buildProperties() { - if (extension.jvm) { - File classesDir = project.sourceSets.main.output.classesDir - File classFile = new File(classesDir, extension.classname.replace('.', File.separator) + '.class') - if (classFile.exists() == false) { - throw new InvalidUserDataException('classname ' + extension.classname + ' does not exist') - } - if (extension.isolated == false) { - logger.warn('Disabling isolation is deprecated and will be removed in the future') - } - } - - Properties props = new Properties() - for (Map.Entry prop : properties) { - props.put(prop.getKey(), prop.getValue()) - } - props.store(propertiesFile.newWriter(), null) } } diff --git a/buildSrc/src/main/resources/plugin-descriptor.properties b/buildSrc/src/main/resources/plugin-descriptor.properties index 4e5486f482d..4c676c26cad 100644 --- a/buildSrc/src/main/resources/plugin-descriptor.properties +++ b/buildSrc/src/main/resources/plugin-descriptor.properties @@ -31,19 +31,19 @@ ### mandatory elements for all plugins: # # 'description': simple summary of the plugin -description=${project.description} +description=${description} # # 'version': plugin's version -version=${project.version} +version=${version} # # 'name': the plugin name -name=${elasticsearch.plugin.name} +name=${name} ### mandatory elements for site plugins: # # 'site': set to true to indicate contents of the _site/ # directory in the root of the plugin should be served. -site=${elasticsearch.plugin.site} +site=${site} # ### mandatory elements for jvm plugins : # @@ -52,19 +52,19 @@ site=${elasticsearch.plugin.site} # Note that only jar files in the root directory are # added to the classpath for the plugin! If you need # other resources, package them into a resources jar. -jvm=${elasticsearch.plugin.jvm} +jvm=${jvm} # # 'classname': the name of the class to load, fully-qualified. -classname=${elasticsearch.plugin.classname} +classname=${classname} # # 'java.version' version of java the code is built against # use the system property java.specification.version # version string must be a sequence of nonnegative decimal integers # separated by "."'s and may have leading zeros -java.version=${java.target.version} +java.version=${javaVersion} # # 'elasticsearch.version' version of elasticsearch compiled against -elasticsearch.version=${elasticsearch.version} +elasticsearch.version=${elasticsearchVersion} # ### deprecated elements for jvm plugins : # @@ -72,5 +72,5 @@ elasticsearch.version=${elasticsearch.version} # passing false is deprecated, and only intended to support plugins # that have hard dependencies against each other. If this is # not specified, then the plugin is isolated by default. -isolated=${elasticsearch.plugin.isolated} +isolated=${isolated} # \ No newline at end of file From e3b8dc71211cf4a52754213faf2e1f4cdbe9939c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 28 Oct 2015 10:16:54 -0400 Subject: [PATCH 14/23] Forbid changing thread pool types This commit forbids the changing of thread pool types for any thread pool. The motivation here is that these are expert settings with little practical advantage. Closes #14294, relates #2509, relates #2858, relates #5152 --- .../elasticsearch/cluster/ClusterModule.java | 2 +- .../rest/action/cat/RestThreadPoolAction.java | 2 +- .../elasticsearch/threadpool/ThreadPool.java | 275 ++++++--- .../search/SearchWithRejectionsIT.java | 1 - .../threadpool/SimpleThreadPoolIT.java | 69 +-- .../ThreadPoolSerializationTests.java | 30 +- .../ThreadPoolTypeSettingsValidatorTests.java | 54 ++ .../UpdateThreadPoolSettingsTests.java | 569 ++++++++++-------- docs/reference/migration/migrate_3_0.asciidoc | 10 +- docs/reference/modules/threadpool.asciidoc | 63 +- .../test/InternalTestCluster.java | 42 +- 11 files changed, 687 insertions(+), 430 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/threadpool/ThreadPoolTypeSettingsValidatorTests.java diff --git a/core/src/main/java/org/elasticsearch/cluster/ClusterModule.java b/core/src/main/java/org/elasticsearch/cluster/ClusterModule.java index 60a4913bb5e..31f5eb4a921 100644 --- a/core/src/main/java/org/elasticsearch/cluster/ClusterModule.java +++ b/core/src/main/java/org/elasticsearch/cluster/ClusterModule.java @@ -177,7 +177,7 @@ public class ClusterModule extends AbstractModule { registerClusterDynamicSetting(RecoverySettings.INDICES_RECOVERY_INTERNAL_ACTION_TIMEOUT, Validator.TIME_NON_NEGATIVE); registerClusterDynamicSetting(RecoverySettings.INDICES_RECOVERY_INTERNAL_LONG_ACTION_TIMEOUT, Validator.TIME_NON_NEGATIVE); registerClusterDynamicSetting(RecoverySettings.INDICES_RECOVERY_MAX_SIZE_PER_SEC, Validator.BYTES_SIZE); - registerClusterDynamicSetting(ThreadPool.THREADPOOL_GROUP + "*", Validator.EMPTY); + registerClusterDynamicSetting(ThreadPool.THREADPOOL_GROUP + "*", ThreadPool.THREAD_POOL_TYPE_SETTINGS_VALIDATOR); registerClusterDynamicSetting(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_INITIAL_PRIMARIES_RECOVERIES, Validator.INTEGER); registerClusterDynamicSetting(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_RECOVERIES, Validator.INTEGER); registerClusterDynamicSetting(DiskThresholdDecider.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK, Validator.EMPTY); diff --git a/core/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java b/core/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java index 311c4eca2cc..2ad9defe2a7 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java @@ -288,7 +288,7 @@ public class RestThreadPoolAction extends AbstractCatAction { } } - table.addCell(poolInfo == null ? null : poolInfo.getType()); + table.addCell(poolInfo == null ? null : poolInfo.getThreadPoolType()); table.addCell(poolStats == null ? null : poolStats.getActive()); table.addCell(poolStats == null ? null : poolStats.getThreads()); table.addCell(poolStats == null ? null : poolStats.getQueue()); diff --git a/core/src/main/java/org/elasticsearch/threadpool/ThreadPool.java b/core/src/main/java/org/elasticsearch/threadpool/ThreadPool.java index 039b46b5c7a..b0d81279b03 100644 --- a/core/src/main/java/org/elasticsearch/threadpool/ThreadPool.java +++ b/core/src/main/java/org/elasticsearch/threadpool/ThreadPool.java @@ -20,6 +20,8 @@ package org.elasticsearch.threadpool; import org.apache.lucene.util.Counter; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.settings.Validator; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.io.stream.StreamInput; @@ -39,22 +41,11 @@ import org.elasticsearch.common.xcontent.XContentBuilderString; import org.elasticsearch.node.settings.NodeSettingsService; import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Queue; -import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.Executor; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.RejectedExecutionHandler; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; +import java.util.*; +import java.util.concurrent.*; +import java.util.function.Function; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.common.settings.Settings.settingsBuilder; @@ -86,6 +77,101 @@ public class ThreadPool extends AbstractComponent { public static final String FETCH_SHARD_STORE = "fetch_shard_store"; } + public enum ThreadPoolType { + CACHED("cached"), + DIRECT("direct"), + FIXED("fixed"), + SCALING("scaling"); + + private final String type; + + public String getType() { + return type; + } + + ThreadPoolType(String type) { + this.type = type; + } + + private final static Map TYPE_MAP; + + static { + Map typeMap = new HashMap<>(); + for (ThreadPoolType threadPoolType : ThreadPoolType.values()) { + typeMap.put(threadPoolType.getType(), threadPoolType); + } + TYPE_MAP = Collections.unmodifiableMap(typeMap); + } + + public static ThreadPoolType fromType(String type) { + ThreadPoolType threadPoolType = TYPE_MAP.get(type); + if (threadPoolType == null) { + throw new IllegalArgumentException("no ThreadPoolType for " + type); + } + return threadPoolType; + } + } + + public static Map THREAD_POOL_TYPES; + + static { + HashMap map = new HashMap<>(); + map.put(Names.SAME, ThreadPoolType.DIRECT); + map.put(Names.GENERIC, ThreadPoolType.CACHED); + map.put(Names.LISTENER, ThreadPoolType.FIXED); + map.put(Names.GET, ThreadPoolType.FIXED); + map.put(Names.INDEX, ThreadPoolType.FIXED); + map.put(Names.BULK, ThreadPoolType.FIXED); + map.put(Names.SEARCH, ThreadPoolType.FIXED); + map.put(Names.SUGGEST, ThreadPoolType.FIXED); + map.put(Names.PERCOLATE, ThreadPoolType.FIXED); + map.put(Names.MANAGEMENT, ThreadPoolType.SCALING); + map.put(Names.FLUSH, ThreadPoolType.SCALING); + map.put(Names.REFRESH, ThreadPoolType.SCALING); + map.put(Names.WARMER, ThreadPoolType.SCALING); + map.put(Names.SNAPSHOT, ThreadPoolType.SCALING); + map.put(Names.FORCE_MERGE, ThreadPoolType.FIXED); + map.put(Names.FETCH_SHARD_STARTED, ThreadPoolType.SCALING); + map.put(Names.FETCH_SHARD_STORE, ThreadPoolType.SCALING); + THREAD_POOL_TYPES = Collections.unmodifiableMap(map); + } + + private static void add(Map executorSettings, ExecutorSettingsBuilder builder) { + Settings settings = builder.build(); + String name = settings.get("name"); + executorSettings.put(name, settings); + } + + private static class ExecutorSettingsBuilder { + Map settings = new HashMap<>(); + + public ExecutorSettingsBuilder(String name) { + settings.put("name", name); + settings.put("type", THREAD_POOL_TYPES.get(name).getType()); + } + + public ExecutorSettingsBuilder size(int availableProcessors) { + return add("size", Integer.toString(availableProcessors)); + } + + public ExecutorSettingsBuilder queueSize(int queueSize) { + return add("queue_size", Integer.toString(queueSize)); + } + + public ExecutorSettingsBuilder keepAlive(String keepAlive) { + return add("keep_alive", keepAlive); + } + + private ExecutorSettingsBuilder add(String key, String value) { + settings.put(key, value); + return this; + } + + public Settings build() { + return settingsBuilder().put(settings).build(); + } + } + public static final String THREADPOOL_GROUP = "threadpool."; private volatile Map executors; @@ -102,7 +188,6 @@ public class ThreadPool extends AbstractComponent { static final Executor DIRECT_EXECUTOR = command -> command.run(); - public ThreadPool(String name) { this(Settings.builder().put("name", name).build()); } @@ -112,42 +197,31 @@ public class ThreadPool extends AbstractComponent { assert settings.get("name") != null : "ThreadPool's settings should contain a name"; - Map groupSettings = settings.getGroups(THREADPOOL_GROUP); + Map groupSettings = getThreadPoolSettingsGroup(settings); int availableProcessors = EsExecutors.boundedNumberOfProcessors(settings); int halfProcMaxAt5 = Math.min(((availableProcessors + 1) / 2), 5); int halfProcMaxAt10 = Math.min(((availableProcessors + 1) / 2), 10); Map defaultExecutorTypeSettings = new HashMap<>(); - defaultExecutorTypeSettings.put(Names.GENERIC, settingsBuilder().put("type", "cached").put("keep_alive", "30s").build()); - defaultExecutorTypeSettings.put(Names.INDEX, - settingsBuilder().put("type", "fixed").put("size", availableProcessors).put("queue_size", 200).build()); - defaultExecutorTypeSettings.put(Names.BULK, - settingsBuilder().put("type", "fixed").put("size", availableProcessors).put("queue_size", 50).build()); - defaultExecutorTypeSettings.put(Names.GET, - settingsBuilder().put("type", "fixed").put("size", availableProcessors).put("queue_size", 1000).build()); - defaultExecutorTypeSettings.put(Names.SEARCH, - settingsBuilder().put("type", "fixed").put("size", ((availableProcessors * 3) / 2) + 1).put("queue_size", 1000).build()); - defaultExecutorTypeSettings.put(Names.SUGGEST, - settingsBuilder().put("type", "fixed").put("size", availableProcessors).put("queue_size", 1000).build()); - defaultExecutorTypeSettings.put(Names.PERCOLATE, - settingsBuilder().put("type", "fixed").put("size", availableProcessors).put("queue_size", 1000).build()); - defaultExecutorTypeSettings .put(Names.MANAGEMENT, settingsBuilder().put("type", "scaling").put("keep_alive", "5m").put("size", 5).build()); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.GENERIC).keepAlive("30s")); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.INDEX).size(availableProcessors).queueSize(200)); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.BULK).size(availableProcessors).queueSize(50)); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.GET).size(availableProcessors).queueSize(1000)); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.SEARCH).size(((availableProcessors * 3) / 2) + 1).queueSize(1000)); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.SUGGEST).size(availableProcessors).queueSize(1000)); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.PERCOLATE).size(availableProcessors).queueSize(1000)); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.MANAGEMENT).size(5).keepAlive("5m")); // no queue as this means clients will need to handle rejections on listener queue even if the operation succeeded // the assumption here is that the listeners should be very lightweight on the listeners side - defaultExecutorTypeSettings.put(Names.LISTENER, settingsBuilder().put("type", "fixed").put("size", halfProcMaxAt10).build()); - defaultExecutorTypeSettings.put(Names.FLUSH, - settingsBuilder().put("type", "scaling").put("keep_alive", "5m").put("size", halfProcMaxAt5).build()); - defaultExecutorTypeSettings.put(Names.REFRESH, - settingsBuilder().put("type", "scaling").put("keep_alive", "5m").put("size", halfProcMaxAt10).build()); - defaultExecutorTypeSettings.put(Names.WARMER, - settingsBuilder().put("type", "scaling").put("keep_alive", "5m").put("size", halfProcMaxAt5).build()); - defaultExecutorTypeSettings.put(Names.SNAPSHOT, - settingsBuilder().put("type", "scaling").put("keep_alive", "5m").put("size", halfProcMaxAt5).build()); - defaultExecutorTypeSettings.put(Names.FORCE_MERGE, settingsBuilder().put("type", "fixed").put("size", 1).build()); - defaultExecutorTypeSettings.put(Names.FETCH_SHARD_STARTED, - settingsBuilder().put("type", "scaling").put("keep_alive", "5m").put("size", availableProcessors * 2).build()); - defaultExecutorTypeSettings.put(Names.FETCH_SHARD_STORE, - settingsBuilder().put("type", "scaling").put("keep_alive", "5m").put("size", availableProcessors * 2).build()); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.LISTENER).size(halfProcMaxAt10)); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.FLUSH).size(halfProcMaxAt5).keepAlive("5m")); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.REFRESH).size(halfProcMaxAt10).keepAlive("5m")); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.WARMER).size(halfProcMaxAt5).keepAlive("5m")); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.SNAPSHOT).size(halfProcMaxAt5).keepAlive("5m")); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.FORCE_MERGE).size(1)); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.FETCH_SHARD_STARTED).size(availableProcessors * 2).keepAlive("5m")); + add(defaultExecutorTypeSettings, new ExecutorSettingsBuilder(Names.FETCH_SHARD_STORE).size(availableProcessors * 2).keepAlive("5m")); + this.defaultExecutorTypeSettings = unmodifiableMap(defaultExecutorTypeSettings); Map executors = new HashMap<>(); @@ -163,8 +237,8 @@ public class ThreadPool extends AbstractComponent { executors.put(entry.getKey(), build(entry.getKey(), entry.getValue(), Settings.EMPTY)); } - executors.put(Names.SAME, new ExecutorHolder(DIRECT_EXECUTOR, new Info(Names.SAME, "same"))); - if (!executors.get(Names.GENERIC).info.getType().equals("cached")) { + executors.put(Names.SAME, new ExecutorHolder(DIRECT_EXECUTOR, new Info(Names.SAME, ThreadPoolType.DIRECT))); + if (!executors.get(Names.GENERIC).info.getThreadPoolType().equals(ThreadPoolType.CACHED)) { throw new IllegalArgumentException("generic thread pool must be of type cached"); } this.executors = unmodifiableMap(executors); @@ -178,6 +252,12 @@ public class ThreadPool extends AbstractComponent { this.estimatedTimeThread.start(); } + private Map getThreadPoolSettingsGroup(Settings settings) { + Map groupSettings = settings.getGroups(THREADPOOL_GROUP); + validate(groupSettings); + return groupSettings; + } + public void setNodeSettingsService(NodeSettingsService nodeSettingsService) { if(settingsListenerIsSet) { throw new IllegalStateException("the node settings listener was set more then once"); @@ -326,24 +406,28 @@ public class ThreadPool extends AbstractComponent { settings = Settings.Builder.EMPTY_SETTINGS; } Info previousInfo = previousExecutorHolder != null ? previousExecutorHolder.info : null; - String type = settings.get("type", previousInfo != null ? previousInfo.getType() : defaultSettings.get("type")); + String type = settings.get("type", previousInfo != null ? previousInfo.getThreadPoolType().getType() : defaultSettings.get("type")); + ThreadPoolType threadPoolType = ThreadPoolType.fromType(type); ThreadFactory threadFactory = EsExecutors.daemonThreadFactory(this.settings, name); - if ("same".equals(type)) { + if (ThreadPoolType.DIRECT == threadPoolType) { if (previousExecutorHolder != null) { logger.debug("updating thread_pool [{}], type [{}]", name, type); } else { logger.debug("creating thread_pool [{}], type [{}]", name, type); } - return new ExecutorHolder(DIRECT_EXECUTOR, new Info(name, type)); - } else if ("cached".equals(type)) { + return new ExecutorHolder(DIRECT_EXECUTOR, new Info(name, threadPoolType)); + } else if (ThreadPoolType.CACHED == threadPoolType) { + if (!Names.GENERIC.equals(name)) { + throw new IllegalArgumentException("thread pool type cached is reserved only for the generic thread pool and can not be applied to [" + name + "]"); + } TimeValue defaultKeepAlive = defaultSettings.getAsTime("keep_alive", timeValueMinutes(5)); if (previousExecutorHolder != null) { - if ("cached".equals(previousInfo.getType())) { + if (ThreadPoolType.CACHED == previousInfo.getThreadPoolType()) { TimeValue updatedKeepAlive = settings.getAsTime("keep_alive", previousInfo.getKeepAlive()); if (!previousInfo.getKeepAlive().equals(updatedKeepAlive)) { logger.debug("updating thread_pool [{}], type [{}], keep_alive [{}]", name, type, updatedKeepAlive); ((EsThreadPoolExecutor) previousExecutorHolder.executor()).setKeepAliveTime(updatedKeepAlive.millis(), TimeUnit.MILLISECONDS); - return new ExecutorHolder(previousExecutorHolder.executor(), new Info(name, type, -1, -1, updatedKeepAlive, null)); + return new ExecutorHolder(previousExecutorHolder.executor(), new Info(name, threadPoolType, -1, -1, updatedKeepAlive, null)); } return previousExecutorHolder; } @@ -358,13 +442,13 @@ public class ThreadPool extends AbstractComponent { logger.debug("creating thread_pool [{}], type [{}], keep_alive [{}]", name, type, keepAlive); } Executor executor = EsExecutors.newCached(name, keepAlive.millis(), TimeUnit.MILLISECONDS, threadFactory); - return new ExecutorHolder(executor, new Info(name, type, -1, -1, keepAlive, null)); - } else if ("fixed".equals(type)) { + return new ExecutorHolder(executor, new Info(name, threadPoolType, -1, -1, keepAlive, null)); + } else if (ThreadPoolType.FIXED == threadPoolType) { int defaultSize = defaultSettings.getAsInt("size", EsExecutors.boundedNumberOfProcessors(settings)); SizeValue defaultQueueSize = getAsSizeOrUnbounded(defaultSettings, "queue", getAsSizeOrUnbounded(defaultSettings, "queue_size", null)); if (previousExecutorHolder != null) { - if ("fixed".equals(previousInfo.getType())) { + if (ThreadPoolType.FIXED == previousInfo.getThreadPoolType()) { SizeValue updatedQueueSize = getAsSizeOrUnbounded(settings, "capacity", getAsSizeOrUnbounded(settings, "queue", getAsSizeOrUnbounded(settings, "queue_size", previousInfo.getQueueSize()))); if (Objects.equals(previousInfo.getQueueSize(), updatedQueueSize)) { int updatedSize = settings.getAsInt("size", previousInfo.getMax()); @@ -378,7 +462,7 @@ public class ThreadPool extends AbstractComponent { ((EsThreadPoolExecutor) previousExecutorHolder.executor()).setCorePoolSize(updatedSize); ((EsThreadPoolExecutor) previousExecutorHolder.executor()).setMaximumPoolSize(updatedSize); } - return new ExecutorHolder(previousExecutorHolder.executor(), new Info(name, type, updatedSize, updatedSize, null, updatedQueueSize)); + return new ExecutorHolder(previousExecutorHolder.executor(), new Info(name, threadPoolType, updatedSize, updatedSize, null, updatedQueueSize)); } return previousExecutorHolder; } @@ -393,13 +477,13 @@ public class ThreadPool extends AbstractComponent { SizeValue queueSize = getAsSizeOrUnbounded(settings, "capacity", getAsSizeOrUnbounded(settings, "queue", getAsSizeOrUnbounded(settings, "queue_size", defaultQueueSize))); logger.debug("creating thread_pool [{}], type [{}], size [{}], queue_size [{}]", name, type, size, queueSize); Executor executor = EsExecutors.newFixed(name, size, queueSize == null ? -1 : (int) queueSize.singles(), threadFactory); - return new ExecutorHolder(executor, new Info(name, type, size, size, null, queueSize)); - } else if ("scaling".equals(type)) { + return new ExecutorHolder(executor, new Info(name, threadPoolType, size, size, null, queueSize)); + } else if (ThreadPoolType.SCALING == threadPoolType) { TimeValue defaultKeepAlive = defaultSettings.getAsTime("keep_alive", timeValueMinutes(5)); int defaultMin = defaultSettings.getAsInt("min", 1); int defaultSize = defaultSettings.getAsInt("size", EsExecutors.boundedNumberOfProcessors(settings)); if (previousExecutorHolder != null) { - if ("scaling".equals(previousInfo.getType())) { + if (ThreadPoolType.SCALING == previousInfo.getThreadPoolType()) { TimeValue updatedKeepAlive = settings.getAsTime("keep_alive", previousInfo.getKeepAlive()); int updatedMin = settings.getAsInt("min", previousInfo.getMin()); int updatedSize = settings.getAsInt("max", settings.getAsInt("size", previousInfo.getMax())); @@ -414,7 +498,7 @@ public class ThreadPool extends AbstractComponent { if (previousInfo.getMax() != updatedSize) { ((EsThreadPoolExecutor) previousExecutorHolder.executor()).setMaximumPoolSize(updatedSize); } - return new ExecutorHolder(previousExecutorHolder.executor(), new Info(name, type, updatedMin, updatedSize, updatedKeepAlive, null)); + return new ExecutorHolder(previousExecutorHolder.executor(), new Info(name, threadPoolType, updatedMin, updatedSize, updatedKeepAlive, null)); } return previousExecutorHolder; } @@ -437,13 +521,13 @@ public class ThreadPool extends AbstractComponent { logger.debug("creating thread_pool [{}], type [{}], min [{}], size [{}], keep_alive [{}]", name, type, min, size, keepAlive); } Executor executor = EsExecutors.newScaling(name, min, size, keepAlive.millis(), TimeUnit.MILLISECONDS, threadFactory); - return new ExecutorHolder(executor, new Info(name, type, min, size, keepAlive, null)); + return new ExecutorHolder(executor, new Info(name, threadPoolType, min, size, keepAlive, null)); } throw new IllegalArgumentException("No type found [" + type + "], for [" + name + "]"); } public void updateSettings(Settings settings) { - Map groupSettings = settings.getGroups("threadpool"); + Map groupSettings = getThreadPoolSettingsGroup(settings); if (groupSettings.isEmpty()) { return; } @@ -490,6 +574,20 @@ public class ThreadPool extends AbstractComponent { } } + private void validate(Map groupSettings) { + for (String key : groupSettings.keySet()) { + if (!THREAD_POOL_TYPES.containsKey(key)) { + continue; + } + String type = groupSettings.get(key).get("type"); + ThreadPoolType correctThreadPoolType = THREAD_POOL_TYPES.get(key); + // TODO: the type equality check can be removed after #3760/#6732 are addressed + if (type != null && !correctThreadPoolType.getType().equals(type)) { + throw new IllegalArgumentException("setting " + THREADPOOL_GROUP + key + ".type to " + type + " is not permitted; must be " + correctThreadPoolType.getType()); + } + } + } + /** * A thread pool size can also be unbounded and is represented by -1, which is not supported by SizeValue (which only supports positive numbers) */ @@ -643,7 +741,7 @@ public class ThreadPool extends AbstractComponent { public static class Info implements Streamable, ToXContent { private String name; - private String type; + private ThreadPoolType type; private int min; private int max; private TimeValue keepAlive; @@ -653,15 +751,15 @@ public class ThreadPool extends AbstractComponent { } - public Info(String name, String type) { + public Info(String name, ThreadPoolType type) { this(name, type, -1); } - public Info(String name, String type, int size) { + public Info(String name, ThreadPoolType type, int size) { this(name, type, size, size, null, null); } - public Info(String name, String type, int min, int max, @Nullable TimeValue keepAlive, @Nullable SizeValue queueSize) { + public Info(String name, ThreadPoolType type, int min, int max, @Nullable TimeValue keepAlive, @Nullable SizeValue queueSize) { this.name = name; this.type = type; this.min = min; @@ -674,7 +772,7 @@ public class ThreadPool extends AbstractComponent { return this.name; } - public String getType() { + public ThreadPoolType getThreadPoolType() { return this.type; } @@ -699,7 +797,7 @@ public class ThreadPool extends AbstractComponent { @Override public void readFrom(StreamInput in) throws IOException { name = in.readString(); - type = in.readString(); + type = ThreadPoolType.fromType(in.readString()); min = in.readInt(); max = in.readInt(); if (in.readBoolean()) { @@ -716,7 +814,7 @@ public class ThreadPool extends AbstractComponent { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(name); - out.writeString(type); + out.writeString(type.getType()); out.writeInt(min); out.writeInt(max); if (keepAlive == null) { @@ -739,7 +837,7 @@ public class ThreadPool extends AbstractComponent { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(name, XContentBuilder.FieldCaseConversion.NONE); - builder.field(Fields.TYPE, type); + builder.field(Fields.TYPE, type.getType()); if (min != -1) { builder.field(Fields.MIN, min); } @@ -814,4 +912,37 @@ public class ThreadPool extends AbstractComponent { return false; } + public static ThreadPoolTypeSettingsValidator THREAD_POOL_TYPE_SETTINGS_VALIDATOR = new ThreadPoolTypeSettingsValidator(); + private static class ThreadPoolTypeSettingsValidator implements Validator { + @Override + public String validate(String setting, String value, ClusterState clusterState) { + // TODO: the type equality validation can be removed after #3760/#6732 are addressed + Matcher matcher = Pattern.compile("threadpool\\.(.*)\\.type").matcher(setting); + if (!matcher.matches()) { + return null; + } else { + String threadPool = matcher.group(1); + ThreadPool.ThreadPoolType defaultThreadPoolType = ThreadPool.THREAD_POOL_TYPES.get(threadPool); + ThreadPool.ThreadPoolType threadPoolType; + try { + threadPoolType = ThreadPool.ThreadPoolType.fromType(value); + } catch (IllegalArgumentException e) { + return e.getMessage(); + } + if (defaultThreadPoolType.equals(threadPoolType)) { + return null; + } else { + return String.format( + Locale.ROOT, + "thread pool type for [%s] can only be updated to [%s] but was [%s]", + threadPool, + defaultThreadPoolType.getType(), + threadPoolType.getType() + ); + } + } + + } + } + } diff --git a/core/src/test/java/org/elasticsearch/search/SearchWithRejectionsIT.java b/core/src/test/java/org/elasticsearch/search/SearchWithRejectionsIT.java index a8c6a194c56..d3e3de5fda1 100644 --- a/core/src/test/java/org/elasticsearch/search/SearchWithRejectionsIT.java +++ b/core/src/test/java/org/elasticsearch/search/SearchWithRejectionsIT.java @@ -37,7 +37,6 @@ public class SearchWithRejectionsIT extends ESIntegTestCase { @Override public Settings nodeSettings(int nodeOrdinal) { return settingsBuilder().put(super.nodeSettings(nodeOrdinal)) - .put("threadpool.search.type", "fixed") .put("threadpool.search.size", 1) .put("threadpool.search.queue_size", 1) .build(); diff --git a/core/src/test/java/org/elasticsearch/threadpool/SimpleThreadPoolIT.java b/core/src/test/java/org/elasticsearch/threadpool/SimpleThreadPoolIT.java index d52f67dc82c..838c2a6d401 100644 --- a/core/src/test/java/org/elasticsearch/threadpool/SimpleThreadPoolIT.java +++ b/core/src/test/java/org/elasticsearch/threadpool/SimpleThreadPoolIT.java @@ -46,20 +46,13 @@ import java.lang.management.ThreadMXBean; import java.util.HashSet; import java.util.Map; import java.util.Set; -import java.util.concurrent.BrokenBarrierException; -import java.util.concurrent.CyclicBarrier; -import java.util.concurrent.Executor; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.*; import java.util.regex.Pattern; import static org.elasticsearch.common.settings.Settings.settingsBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.sameInstance; +import static org.hamcrest.Matchers.*; /** */ @@ -67,7 +60,7 @@ import static org.hamcrest.Matchers.sameInstance; public class SimpleThreadPoolIT extends ESIntegTestCase { @Override protected Settings nodeSettings(int nodeOrdinal) { - return Settings.settingsBuilder().put(super.nodeSettings(nodeOrdinal)).put("threadpool.search.type", "cached").build(); + return Settings.settingsBuilder().build(); } public void testThreadNames() throws Exception { @@ -130,26 +123,23 @@ public class SimpleThreadPoolIT extends ESIntegTestCase { internalCluster().startNodesAsync(2).get(); ThreadPool threadPool = internalCluster().getDataNodeInstance(ThreadPool.class); // Check that settings are changed - assertThat(((ThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(5L)); - client().admin().cluster().prepareUpdateSettings().setTransientSettings(settingsBuilder().put("threadpool.search.keep_alive", "10m").build()).execute().actionGet(); - assertThat(((ThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(10L)); + assertThat(((ThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getQueue().remainingCapacity(), equalTo(1000)); + client().admin().cluster().prepareUpdateSettings().setTransientSettings(settingsBuilder().put("threadpool.search.queue_size", 2000).build()).execute().actionGet(); + assertThat(((ThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getQueue().remainingCapacity(), equalTo(2000)); // Make sure that threads continue executing when executor is replaced final CyclicBarrier barrier = new CyclicBarrier(2); Executor oldExecutor = threadPool.executor(Names.SEARCH); - threadPool.executor(Names.SEARCH).execute(new Runnable() { - @Override - public void run() { - try { - barrier.await(); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } catch (BrokenBarrierException ex) { - // - } - } - }); - client().admin().cluster().prepareUpdateSettings().setTransientSettings(settingsBuilder().put("threadpool.search.type", "fixed").build()).execute().actionGet(); + threadPool.executor(Names.SEARCH).execute(() -> { + try { + barrier.await(); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } catch (BrokenBarrierException ex) { + // + } + }); + client().admin().cluster().prepareUpdateSettings().setTransientSettings(settingsBuilder().put("threadpool.search.queue_size", 1000).build()).execute().actionGet(); assertThat(threadPool.executor(Names.SEARCH), not(sameInstance(oldExecutor))); assertThat(((ThreadPoolExecutor) oldExecutor).isShutdown(), equalTo(true)); assertThat(((ThreadPoolExecutor) oldExecutor).isTerminating(), equalTo(true)); @@ -157,24 +147,19 @@ public class SimpleThreadPoolIT extends ESIntegTestCase { barrier.await(10, TimeUnit.SECONDS); // Make sure that new thread executor is functional - threadPool.executor(Names.SEARCH).execute(new Runnable() { - @Override - public void run() { - try { - barrier.await(); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } catch (BrokenBarrierException ex) { - // + threadPool.executor(Names.SEARCH).execute(() -> { + try { + barrier.await(); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } catch (BrokenBarrierException ex) { + // + } } - } - }); - client().admin().cluster().prepareUpdateSettings().setTransientSettings(settingsBuilder().put("threadpool.search.type", "fixed").build()).execute().actionGet(); + ); + client().admin().cluster().prepareUpdateSettings().setTransientSettings(settingsBuilder().put("threadpool.search.queue_size", 500)).execute().actionGet(); barrier.await(10, TimeUnit.SECONDS); - // This was here: Thread.sleep(200); - // Why? What was it for? - // Check that node info is correct NodesInfoResponse nodesInfoResponse = client().admin().cluster().prepareNodesInfo().all().execute().actionGet(); for (int i = 0; i < 2; i++) { @@ -182,7 +167,7 @@ public class SimpleThreadPoolIT extends ESIntegTestCase { boolean found = false; for (ThreadPool.Info info : nodeInfo.getThreadPool()) { if (info.getName().equals(Names.SEARCH)) { - assertThat(info.getType(), equalTo("fixed")); + assertEquals(info.getThreadPoolType(), ThreadPool.ThreadPoolType.FIXED); found = true; break; } diff --git a/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolSerializationTests.java b/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolSerializationTests.java index cb27fd71f9d..3d57c1d5206 100644 --- a/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolSerializationTests.java +++ b/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolSerializationTests.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.threadpool; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.Version; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; @@ -30,7 +31,9 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.ESTestCase; +import org.junit.Before; +import java.io.IOException; import java.util.Map; import static org.elasticsearch.common.settings.Settings.settingsBuilder; @@ -44,9 +47,16 @@ import static org.hamcrest.Matchers.nullValue; */ public class ThreadPoolSerializationTests extends ESTestCase { BytesStreamOutput output = new BytesStreamOutput(); + private ThreadPool.ThreadPoolType threadPoolType; + + @Before + public void setUp() throws Exception { + super.setUp(); + threadPoolType = randomFrom(ThreadPool.ThreadPoolType.values()); + } public void testThatQueueSizeSerializationWorks() throws Exception { - ThreadPool.Info info = new ThreadPool.Info("foo", "search", 1, 10, TimeValue.timeValueMillis(3000), SizeValue.parseSizeValue("10k")); + ThreadPool.Info info = new ThreadPool.Info("foo", threadPoolType, 1, 10, TimeValue.timeValueMillis(3000), SizeValue.parseSizeValue("10k")); output.setVersion(Version.CURRENT); info.writeTo(output); @@ -58,7 +68,7 @@ public class ThreadPoolSerializationTests extends ESTestCase { } public void testThatNegativeQueueSizesCanBeSerialized() throws Exception { - ThreadPool.Info info = new ThreadPool.Info("foo", "search", 1, 10, TimeValue.timeValueMillis(3000), null); + ThreadPool.Info info = new ThreadPool.Info("foo", threadPoolType, 1, 10, TimeValue.timeValueMillis(3000), null); output.setVersion(Version.CURRENT); info.writeTo(output); @@ -70,7 +80,7 @@ public class ThreadPoolSerializationTests extends ESTestCase { } public void testThatToXContentWritesOutUnboundedCorrectly() throws Exception { - ThreadPool.Info info = new ThreadPool.Info("foo", "search", 1, 10, TimeValue.timeValueMillis(3000), null); + ThreadPool.Info info = new ThreadPool.Info("foo", threadPoolType, 1, 10, TimeValue.timeValueMillis(3000), null); XContentBuilder builder = jsonBuilder(); builder.startObject(); info.toXContent(builder, ToXContent.EMPTY_PARAMS); @@ -95,7 +105,7 @@ public class ThreadPoolSerializationTests extends ESTestCase { } public void testThatToXContentWritesInteger() throws Exception { - ThreadPool.Info info = new ThreadPool.Info("foo", "search", 1, 10, TimeValue.timeValueMillis(3000), SizeValue.parseSizeValue("1k")); + ThreadPool.Info info = new ThreadPool.Info("foo", threadPoolType, 1, 10, TimeValue.timeValueMillis(3000), SizeValue.parseSizeValue("1k")); XContentBuilder builder = jsonBuilder(); builder.startObject(); info.toXContent(builder, ToXContent.EMPTY_PARAMS); @@ -111,4 +121,16 @@ public class ThreadPoolSerializationTests extends ESTestCase { assertThat(map, hasKey("queue_size")); assertThat(map.get("queue_size").toString(), is("1000")); } + + public void testThatThreadPoolTypeIsSerializedCorrectly() throws IOException { + ThreadPool.Info info = new ThreadPool.Info("foo", threadPoolType); + output.setVersion(Version.CURRENT); + info.writeTo(output); + + StreamInput input = StreamInput.wrap(output.bytes()); + ThreadPool.Info newInfo = new ThreadPool.Info(); + newInfo.readFrom(input); + + assertThat(newInfo.getThreadPoolType(), is(threadPoolType)); + } } diff --git a/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolTypeSettingsValidatorTests.java b/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolTypeSettingsValidatorTests.java new file mode 100644 index 00000000000..aa3b2a8dec2 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolTypeSettingsValidatorTests.java @@ -0,0 +1,54 @@ +package org.elasticsearch.threadpool; + +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.settings.Validator; +import org.elasticsearch.test.ESTestCase; +import org.junit.Before; + +import java.util.*; + +import static org.junit.Assert.*; + +public class ThreadPoolTypeSettingsValidatorTests extends ESTestCase { + private Validator validator; + + @Before + public void setUp() throws Exception { + super.setUp(); + validator = ThreadPool.THREAD_POOL_TYPE_SETTINGS_VALIDATOR; + } + + public void testValidThreadPoolTypeSettings() { + for (Map.Entry entry : ThreadPool.THREAD_POOL_TYPES.entrySet()) { + assertNull(validateSetting(validator, entry.getKey(), entry.getValue().getType())); + } + } + + public void testInvalidThreadPoolTypeSettings() { + for (Map.Entry entry : ThreadPool.THREAD_POOL_TYPES.entrySet()) { + Set set = new HashSet<>(); + set.addAll(Arrays.asList(ThreadPool.ThreadPoolType.values())); + set.remove(entry.getValue()); + ThreadPool.ThreadPoolType invalidThreadPoolType = randomFrom(set.toArray(new ThreadPool.ThreadPoolType[set.size()])); + String expectedMessage = String.format( + Locale.ROOT, + "thread pool type for [%s] can only be updated to [%s] but was [%s]", + entry.getKey(), + entry.getValue().getType(), + invalidThreadPoolType.getType()); + String message = validateSetting(validator, entry.getKey(), invalidThreadPoolType.getType()); + assertNotNull(message); + assertEquals(expectedMessage, message); + } + } + + public void testNonThreadPoolTypeSetting() { + String setting = ThreadPool.THREADPOOL_GROUP + randomAsciiOfLength(10) + "foo"; + String value = randomAsciiOfLength(10); + assertNull(validator.validate(setting, value, ClusterState.PROTO)); + } + + private String validateSetting(Validator validator, String threadPoolName, String value) { + return validator.validate(ThreadPool.THREADPOOL_GROUP + threadPoolName + ".type", value, ClusterState.PROTO); + } +} diff --git a/core/src/test/java/org/elasticsearch/threadpool/UpdateThreadPoolSettingsTests.java b/core/src/test/java/org/elasticsearch/threadpool/UpdateThreadPoolSettingsTests.java index cd252b60d71..faa08243af0 100644 --- a/core/src/test/java/org/elasticsearch/threadpool/UpdateThreadPoolSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/threadpool/UpdateThreadPoolSettingsTests.java @@ -25,22 +25,330 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool.Names; import java.lang.reflect.Field; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import static org.elasticsearch.common.settings.Settings.settingsBuilder; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; -import static org.hamcrest.Matchers.sameInstance; +import static org.hamcrest.Matchers.*; /** */ public class UpdateThreadPoolSettingsTests extends ESTestCase { + public void testCorrectThreadPoolTypePermittedInSettings() throws InterruptedException { + String threadPoolName = randomThreadPoolName(); + ThreadPool.ThreadPoolType correctThreadPoolType = ThreadPool.THREAD_POOL_TYPES.get(threadPoolName); + ThreadPool threadPool = null; + try { + threadPool = new ThreadPool(settingsBuilder() + .put("name", "testCorrectThreadPoolTypePermittedInSettings") + .put("threadpool." + threadPoolName + ".type", correctThreadPoolType.getType()) + .build()); + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), correctThreadPoolType); + } finally { + terminateThreadPoolIfNeeded(threadPool); + } + } + + public void testThreadPoolCanNotOverrideThreadPoolType() throws InterruptedException { + String threadPoolName = randomThreadPoolName(); + ThreadPool.ThreadPoolType incorrectThreadPoolType = randomIncorrectThreadPoolType(threadPoolName); + ThreadPool.ThreadPoolType correctThreadPoolType = ThreadPool.THREAD_POOL_TYPES.get(threadPoolName); + ThreadPool threadPool = null; + try { + threadPool = new ThreadPool( + settingsBuilder() + .put("name", "testThreadPoolCanNotOverrideThreadPoolType") + .put("threadpool." + threadPoolName + ".type", incorrectThreadPoolType.getType()) + .build()); + terminate(threadPool); + fail("expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertThat( + e.getMessage(), + is("setting threadpool." + threadPoolName + ".type to " + incorrectThreadPoolType.getType() + " is not permitted; must be " + correctThreadPoolType.getType())); + } finally { + terminateThreadPoolIfNeeded(threadPool); + } + } + + public void testUpdateSettingsCanNotChangeThreadPoolType() throws InterruptedException { + String threadPoolName = randomThreadPoolName(); + ThreadPool.ThreadPoolType invalidThreadPoolType = randomIncorrectThreadPoolType(threadPoolName); + ThreadPool.ThreadPoolType validThreadPoolType = ThreadPool.THREAD_POOL_TYPES.get(threadPoolName); + ThreadPool threadPool = null; + try { + threadPool = new ThreadPool(settingsBuilder().put("name", "testUpdateSettingsCanNotChangeThreadPoolType").build()); + + + threadPool.updateSettings( + settingsBuilder() + .put("threadpool." + threadPoolName + ".type", invalidThreadPoolType.getType()) + .build() + ); + fail("expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertThat( + e.getMessage(), + is("setting threadpool." + threadPoolName + ".type to " + invalidThreadPoolType.getType() + " is not permitted; must be " + validThreadPoolType.getType())); + } finally { + terminateThreadPoolIfNeeded(threadPool); + } + } + + public void testCachedExecutorType() throws InterruptedException { + String threadPoolName = randomThreadPool(ThreadPool.ThreadPoolType.CACHED); + ThreadPool threadPool = null; + try { + threadPool = new ThreadPool( + Settings.settingsBuilder() + .put("name", "testCachedExecutorType").build()); + + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), ThreadPool.ThreadPoolType.CACHED); + assertThat(threadPool.executor(threadPoolName), instanceOf(EsThreadPoolExecutor.class)); + + threadPool.updateSettings(settingsBuilder() + .put("threadpool." + threadPoolName + ".keep_alive", "10m") + .build()); + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), ThreadPool.ThreadPoolType.CACHED); + assertThat(threadPool.executor(threadPoolName), instanceOf(EsThreadPoolExecutor.class)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getCorePoolSize(), equalTo(0)); + // Make sure keep alive value changed + assertThat(info(threadPool, threadPoolName).getKeepAlive().minutes(), equalTo(10L)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(10L)); + + // Make sure keep alive value reused + assertThat(info(threadPool, threadPoolName).getKeepAlive().minutes(), equalTo(10L)); + assertThat(threadPool.executor(threadPoolName), instanceOf(EsThreadPoolExecutor.class)); + + // Change keep alive + Executor oldExecutor = threadPool.executor(threadPoolName); + threadPool.updateSettings(settingsBuilder().put("threadpool." + threadPoolName + ".keep_alive", "1m").build()); + // Make sure keep alive value changed + assertThat(info(threadPool, threadPoolName).getKeepAlive().minutes(), equalTo(1L)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(1L)); + // Make sure executor didn't change + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), ThreadPool.ThreadPoolType.CACHED); + assertThat(threadPool.executor(threadPoolName), sameInstance(oldExecutor)); + + // Set the same keep alive + threadPool.updateSettings(settingsBuilder().put("threadpool." + threadPoolName + ".keep_alive", "1m").build()); + // Make sure keep alive value didn't change + assertThat(info(threadPool, threadPoolName).getKeepAlive().minutes(), equalTo(1L)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(1L)); + // Make sure executor didn't change + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), ThreadPool.ThreadPoolType.CACHED); + assertThat(threadPool.executor(threadPoolName), sameInstance(oldExecutor)); + } finally { + terminateThreadPoolIfNeeded(threadPool); + } + } + + public void testFixedExecutorType() throws InterruptedException { + String threadPoolName = randomThreadPool(ThreadPool.ThreadPoolType.FIXED); + ThreadPool threadPool = null; + + try { + threadPool = new ThreadPool(settingsBuilder() + .put("name", "testCachedExecutorType").build()); + assertThat(threadPool.executor(threadPoolName), instanceOf(EsThreadPoolExecutor.class)); + + threadPool.updateSettings(settingsBuilder() + .put("threadpool." + threadPoolName + ".size", "15") + .build()); + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), ThreadPool.ThreadPoolType.FIXED); + assertThat(threadPool.executor(threadPoolName), instanceOf(EsThreadPoolExecutor.class)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getCorePoolSize(), equalTo(15)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getMaximumPoolSize(), equalTo(15)); + assertThat(info(threadPool, threadPoolName).getMin(), equalTo(15)); + assertThat(info(threadPool, threadPoolName).getMax(), equalTo(15)); + // keep alive does not apply to fixed thread pools + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(0L)); + + // Put old type back + threadPool.updateSettings(Settings.EMPTY); + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), ThreadPool.ThreadPoolType.FIXED); + // Make sure keep alive value is not used + assertThat(info(threadPool, threadPoolName).getKeepAlive(), nullValue()); + // Make sure keep pool size value were reused + assertThat(info(threadPool, threadPoolName).getMin(), equalTo(15)); + assertThat(info(threadPool, threadPoolName).getMax(), equalTo(15)); + assertThat(threadPool.executor(threadPoolName), instanceOf(EsThreadPoolExecutor.class)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getCorePoolSize(), equalTo(15)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getMaximumPoolSize(), equalTo(15)); + + // Change size + Executor oldExecutor = threadPool.executor(threadPoolName); + threadPool.updateSettings(settingsBuilder().put("threadpool." + threadPoolName + ".size", "10").build()); + // Make sure size values changed + assertThat(info(threadPool, threadPoolName).getMax(), equalTo(10)); + assertThat(info(threadPool, threadPoolName).getMin(), equalTo(10)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getMaximumPoolSize(), equalTo(10)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getCorePoolSize(), equalTo(10)); + // Make sure executor didn't change + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), ThreadPool.ThreadPoolType.FIXED); + assertThat(threadPool.executor(threadPoolName), sameInstance(oldExecutor)); + + // Change queue capacity + threadPool.updateSettings(settingsBuilder() + .put("threadpool." + threadPoolName + ".queue", "500") + .build()); + } finally { + terminateThreadPoolIfNeeded(threadPool); + } + } + + public void testScalingExecutorType() throws InterruptedException { + String threadPoolName = randomThreadPool(ThreadPool.ThreadPoolType.SCALING); + ThreadPool threadPool = null; + try { + threadPool = new ThreadPool(settingsBuilder() + .put("threadpool." + threadPoolName + ".size", 10) + .put("name", "testCachedExecutorType").build()); + assertThat(info(threadPool, threadPoolName).getMin(), equalTo(1)); + assertThat(info(threadPool, threadPoolName).getMax(), equalTo(10)); + assertThat(info(threadPool, threadPoolName).getKeepAlive().minutes(), equalTo(5L)); + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), ThreadPool.ThreadPoolType.SCALING); + assertThat(threadPool.executor(threadPoolName), instanceOf(EsThreadPoolExecutor.class)); + + // Change settings that doesn't require pool replacement + Executor oldExecutor = threadPool.executor(threadPoolName); + threadPool.updateSettings(settingsBuilder() + .put("threadpool." + threadPoolName + ".keep_alive", "10m") + .put("threadpool." + threadPoolName + ".min", "2") + .put("threadpool." + threadPoolName + ".size", "15") + .build()); + assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), ThreadPool.ThreadPoolType.SCALING); + assertThat(threadPool.executor(threadPoolName), instanceOf(EsThreadPoolExecutor.class)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getCorePoolSize(), equalTo(2)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getMaximumPoolSize(), equalTo(15)); + assertThat(info(threadPool, threadPoolName).getMin(), equalTo(2)); + assertThat(info(threadPool, threadPoolName).getMax(), equalTo(15)); + // Make sure keep alive value changed + assertThat(info(threadPool, threadPoolName).getKeepAlive().minutes(), equalTo(10L)); + assertThat(((EsThreadPoolExecutor) threadPool.executor(threadPoolName)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(10L)); + assertThat(threadPool.executor(threadPoolName), sameInstance(oldExecutor)); + } finally { + terminateThreadPoolIfNeeded(threadPool); + } + } + + public void testShutdownNowInterrupts() throws Exception { + String threadPoolName = randomThreadPool(ThreadPool.ThreadPoolType.FIXED); + ThreadPool threadPool = null; + try { + threadPool = new ThreadPool(Settings.settingsBuilder() + .put("threadpool." + threadPoolName + ".queue_size", 1000) + .put("name", "testCachedExecutorType").build()); + assertEquals(info(threadPool, threadPoolName).getQueueSize().getSingles(), 1000L); + + final CountDownLatch latch = new CountDownLatch(1); + ThreadPoolExecutor oldExecutor = (ThreadPoolExecutor) threadPool.executor(threadPoolName); + threadPool.executor(threadPoolName).execute(() -> { + try { + new CountDownLatch(1).await(); + } catch (InterruptedException ex) { + latch.countDown(); + Thread.currentThread().interrupt(); + } + } + ); + threadPool.updateSettings(settingsBuilder().put("threadpool." + threadPoolName + ".queue_size", 2000).build()); + assertThat(threadPool.executor(threadPoolName), not(sameInstance(oldExecutor))); + assertThat(oldExecutor.isShutdown(), equalTo(true)); + assertThat(oldExecutor.isTerminating(), equalTo(true)); + assertThat(oldExecutor.isTerminated(), equalTo(false)); + threadPool.shutdownNow(); // should interrupt the thread + latch.await(3, TimeUnit.SECONDS); // If this throws then ThreadPool#shutdownNow didn't interrupt + } finally { + terminateThreadPoolIfNeeded(threadPool); + } + } + + public void testCustomThreadPool() throws Exception { + ThreadPool threadPool = null; + try { + threadPool = new ThreadPool(Settings.settingsBuilder() + .put("threadpool.my_pool1.type", "scaling") + .put("threadpool.my_pool2.type", "fixed") + .put("threadpool.my_pool2.size", "1") + .put("threadpool.my_pool2.queue_size", "1") + .put("name", "testCustomThreadPool").build()); + ThreadPoolInfo groups = threadPool.info(); + boolean foundPool1 = false; + boolean foundPool2 = false; + outer: + for (ThreadPool.Info info : groups) { + if ("my_pool1".equals(info.getName())) { + foundPool1 = true; + assertEquals(info.getThreadPoolType(), ThreadPool.ThreadPoolType.SCALING); + } else if ("my_pool2".equals(info.getName())) { + foundPool2 = true; + assertEquals(info.getThreadPoolType(), ThreadPool.ThreadPoolType.FIXED); + assertThat(info.getMin(), equalTo(1)); + assertThat(info.getMax(), equalTo(1)); + assertThat(info.getQueueSize().singles(), equalTo(1l)); + } else { + for (Field field : Names.class.getFields()) { + if (info.getName().equalsIgnoreCase(field.getName())) { + // This is ok it is a default thread pool + continue outer; + } + } + fail("Unexpected pool name: " + info.getName()); + } + } + assertThat(foundPool1, is(true)); + assertThat(foundPool2, is(true)); + + // Updating my_pool2 + Settings settings = Settings.builder() + .put("threadpool.my_pool2.size", "10") + .build(); + threadPool.updateSettings(settings); + + groups = threadPool.info(); + foundPool1 = false; + foundPool2 = false; + outer: + for (ThreadPool.Info info : groups) { + if ("my_pool1".equals(info.getName())) { + foundPool1 = true; + assertEquals(info.getThreadPoolType(), ThreadPool.ThreadPoolType.SCALING); + } else if ("my_pool2".equals(info.getName())) { + foundPool2 = true; + assertThat(info.getMax(), equalTo(10)); + assertThat(info.getMin(), equalTo(10)); + assertThat(info.getQueueSize().singles(), equalTo(1l)); + assertEquals(info.getThreadPoolType(), ThreadPool.ThreadPoolType.FIXED); + } else { + for (Field field : Names.class.getFields()) { + if (info.getName().equalsIgnoreCase(field.getName())) { + // This is ok it is a default thread pool + continue outer; + } + } + fail("Unexpected pool name: " + info.getName()); + } + } + assertThat(foundPool1, is(true)); + assertThat(foundPool2, is(true)); + } finally { + terminateThreadPoolIfNeeded(threadPool); + } + } + + private void terminateThreadPoolIfNeeded(ThreadPool threadPool) throws InterruptedException { + if (threadPool != null) { + terminate(threadPool); + } + } + private ThreadPool.Info info(ThreadPool threadPool, String name) { for (ThreadPool.Info info : threadPool.info()) { if (info.getName().equals(name)) { @@ -50,247 +358,20 @@ public class UpdateThreadPoolSettingsTests extends ESTestCase { return null; } - public void testCachedExecutorType() throws InterruptedException { - ThreadPool threadPool = new ThreadPool( - Settings.settingsBuilder() - .put("threadpool.search.type", "cached") - .put("name","testCachedExecutorType").build()); - - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("cached")); - assertThat(info(threadPool, Names.SEARCH).getKeepAlive().minutes(), equalTo(5L)); - assertThat(threadPool.executor(Names.SEARCH), instanceOf(EsThreadPoolExecutor.class)); - - // Replace with different type - threadPool.updateSettings(settingsBuilder().put("threadpool.search.type", "same").build()); - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("same")); - assertThat(threadPool.executor(Names.SEARCH), is(ThreadPool.DIRECT_EXECUTOR)); - - // Replace with different type again - threadPool.updateSettings(settingsBuilder() - .put("threadpool.search.type", "scaling") - .put("threadpool.search.keep_alive", "10m") - .build()); - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("scaling")); - assertThat(threadPool.executor(Names.SEARCH), instanceOf(EsThreadPoolExecutor.class)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getCorePoolSize(), equalTo(1)); - // Make sure keep alive value changed - assertThat(info(threadPool, Names.SEARCH).getKeepAlive().minutes(), equalTo(10L)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(10L)); - - // Put old type back - threadPool.updateSettings(settingsBuilder().put("threadpool.search.type", "cached").build()); - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("cached")); - // Make sure keep alive value reused - assertThat(info(threadPool, Names.SEARCH).getKeepAlive().minutes(), equalTo(10L)); - assertThat(threadPool.executor(Names.SEARCH), instanceOf(EsThreadPoolExecutor.class)); - - // Change keep alive - Executor oldExecutor = threadPool.executor(Names.SEARCH); - threadPool.updateSettings(settingsBuilder().put("threadpool.search.keep_alive", "1m").build()); - // Make sure keep alive value changed - assertThat(info(threadPool, Names.SEARCH).getKeepAlive().minutes(), equalTo(1L)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(1L)); - // Make sure executor didn't change - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("cached")); - assertThat(threadPool.executor(Names.SEARCH), sameInstance(oldExecutor)); - - // Set the same keep alive - threadPool.updateSettings(settingsBuilder().put("threadpool.search.keep_alive", "1m").build()); - // Make sure keep alive value didn't change - assertThat(info(threadPool, Names.SEARCH).getKeepAlive().minutes(), equalTo(1L)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(1L)); - // Make sure executor didn't change - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("cached")); - assertThat(threadPool.executor(Names.SEARCH), sameInstance(oldExecutor)); - terminate(threadPool); + private String randomThreadPoolName() { + Set threadPoolNames = ThreadPool.THREAD_POOL_TYPES.keySet(); + return randomFrom(threadPoolNames.toArray(new String[threadPoolNames.size()])); } - public void testFixedExecutorType() throws InterruptedException { - ThreadPool threadPool = new ThreadPool(settingsBuilder() - .put("threadpool.search.type", "fixed") - .put("name","testCachedExecutorType").build()); - - assertThat(threadPool.executor(Names.SEARCH), instanceOf(EsThreadPoolExecutor.class)); - - // Replace with different type - threadPool.updateSettings(settingsBuilder() - .put("threadpool.search.type", "scaling") - .put("threadpool.search.keep_alive", "10m") - .put("threadpool.search.min", "2") - .put("threadpool.search.size", "15") - .build()); - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("scaling")); - assertThat(threadPool.executor(Names.SEARCH), instanceOf(EsThreadPoolExecutor.class)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getCorePoolSize(), equalTo(2)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getMaximumPoolSize(), equalTo(15)); - assertThat(info(threadPool, Names.SEARCH).getMin(), equalTo(2)); - assertThat(info(threadPool, Names.SEARCH).getMax(), equalTo(15)); - // Make sure keep alive value changed - assertThat(info(threadPool, Names.SEARCH).getKeepAlive().minutes(), equalTo(10L)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(10L)); - - // Put old type back - threadPool.updateSettings(settingsBuilder() - .put("threadpool.search.type", "fixed") - .build()); - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("fixed")); - // Make sure keep alive value is not used - assertThat(info(threadPool, Names.SEARCH).getKeepAlive(), nullValue()); - // Make sure keep pool size value were reused - assertThat(info(threadPool, Names.SEARCH).getMin(), equalTo(15)); - assertThat(info(threadPool, Names.SEARCH).getMax(), equalTo(15)); - assertThat(threadPool.executor(Names.SEARCH), instanceOf(EsThreadPoolExecutor.class)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getCorePoolSize(), equalTo(15)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getMaximumPoolSize(), equalTo(15)); - - // Change size - Executor oldExecutor = threadPool.executor(Names.SEARCH); - threadPool.updateSettings(settingsBuilder().put("threadpool.search.size", "10").build()); - // Make sure size values changed - assertThat(info(threadPool, Names.SEARCH).getMax(), equalTo(10)); - assertThat(info(threadPool, Names.SEARCH).getMin(), equalTo(10)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getMaximumPoolSize(), equalTo(10)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getCorePoolSize(), equalTo(10)); - // Make sure executor didn't change - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("fixed")); - assertThat(threadPool.executor(Names.SEARCH), sameInstance(oldExecutor)); - - // Change queue capacity - threadPool.updateSettings(settingsBuilder() - .put("threadpool.search.queue", "500") - .build()); - - terminate(threadPool); + private ThreadPool.ThreadPoolType randomIncorrectThreadPoolType(String threadPoolName) { + Set set = new HashSet<>(); + set.addAll(Arrays.asList(ThreadPool.ThreadPoolType.values())); + set.remove(ThreadPool.THREAD_POOL_TYPES.get(threadPoolName)); + ThreadPool.ThreadPoolType invalidThreadPoolType = randomFrom(set.toArray(new ThreadPool.ThreadPoolType[set.size()])); + return invalidThreadPoolType; } - public void testScalingExecutorType() throws InterruptedException { - ThreadPool threadPool = new ThreadPool(settingsBuilder() - .put("threadpool.search.type", "scaling") - .put("threadpool.search.size", 10) - .put("name","testCachedExecutorType").build()); - - assertThat(info(threadPool, Names.SEARCH).getMin(), equalTo(1)); - assertThat(info(threadPool, Names.SEARCH).getMax(), equalTo(10)); - assertThat(info(threadPool, Names.SEARCH).getKeepAlive().minutes(), equalTo(5L)); - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("scaling")); - assertThat(threadPool.executor(Names.SEARCH), instanceOf(EsThreadPoolExecutor.class)); - - // Change settings that doesn't require pool replacement - Executor oldExecutor = threadPool.executor(Names.SEARCH); - threadPool.updateSettings(settingsBuilder() - .put("threadpool.search.type", "scaling") - .put("threadpool.search.keep_alive", "10m") - .put("threadpool.search.min", "2") - .put("threadpool.search.size", "15") - .build()); - assertThat(info(threadPool, Names.SEARCH).getType(), equalTo("scaling")); - assertThat(threadPool.executor(Names.SEARCH), instanceOf(EsThreadPoolExecutor.class)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getCorePoolSize(), equalTo(2)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getMaximumPoolSize(), equalTo(15)); - assertThat(info(threadPool, Names.SEARCH).getMin(), equalTo(2)); - assertThat(info(threadPool, Names.SEARCH).getMax(), equalTo(15)); - // Make sure keep alive value changed - assertThat(info(threadPool, Names.SEARCH).getKeepAlive().minutes(), equalTo(10L)); - assertThat(((EsThreadPoolExecutor) threadPool.executor(Names.SEARCH)).getKeepAliveTime(TimeUnit.MINUTES), equalTo(10L)); - assertThat(threadPool.executor(Names.SEARCH), sameInstance(oldExecutor)); - - terminate(threadPool); + private String randomThreadPool(ThreadPool.ThreadPoolType type) { + return randomFrom(ThreadPool.THREAD_POOL_TYPES.entrySet().stream().filter(t -> t.getValue().equals(type)).map(t -> t.getKey()).collect(Collectors.toList())); } - - public void testShutdownNowInterrupts() throws Exception { - ThreadPool threadPool = new ThreadPool(Settings.settingsBuilder() - .put("threadpool.search.type", "cached") - .put("name","testCachedExecutorType").build()); - - final CountDownLatch latch = new CountDownLatch(1); - ThreadPoolExecutor oldExecutor = (ThreadPoolExecutor) threadPool.executor(Names.SEARCH); - threadPool.executor(Names.SEARCH).execute(new Runnable() { - @Override - public void run() { - try { - new CountDownLatch(1).await(); - } catch (InterruptedException ex) { - latch.countDown(); - Thread.currentThread().interrupt(); - } - } - }); - threadPool.updateSettings(settingsBuilder().put("threadpool.search.type", "fixed").build()); - assertThat(threadPool.executor(Names.SEARCH), not(sameInstance(oldExecutor))); - assertThat(oldExecutor.isShutdown(), equalTo(true)); - assertThat(oldExecutor.isTerminating(), equalTo(true)); - assertThat(oldExecutor.isTerminated(), equalTo(false)); - threadPool.shutdownNow(); // should interrupt the thread - latch.await(3, TimeUnit.SECONDS); // If this throws then shotdownNow didn't interrupt - terminate(threadPool); - } - - public void testCustomThreadPool() throws Exception { - ThreadPool threadPool = new ThreadPool(Settings.settingsBuilder() - .put("threadpool.my_pool1.type", "cached") - .put("threadpool.my_pool2.type", "fixed") - .put("threadpool.my_pool2.size", "1") - .put("threadpool.my_pool2.queue_size", "1") - .put("name", "testCustomThreadPool").build()); - - ThreadPoolInfo groups = threadPool.info(); - boolean foundPool1 = false; - boolean foundPool2 = false; - outer: for (ThreadPool.Info info : groups) { - if ("my_pool1".equals(info.getName())) { - foundPool1 = true; - assertThat(info.getType(), equalTo("cached")); - } else if ("my_pool2".equals(info.getName())) { - foundPool2 = true; - assertThat(info.getType(), equalTo("fixed")); - assertThat(info.getMin(), equalTo(1)); - assertThat(info.getMax(), equalTo(1)); - assertThat(info.getQueueSize().singles(), equalTo(1l)); - } else { - for (Field field : Names.class.getFields()) { - if (info.getName().equalsIgnoreCase(field.getName())) { - // This is ok it is a default thread pool - continue outer; - } - } - fail("Unexpected pool name: " + info.getName()); - } - } - assertThat(foundPool1, is(true)); - assertThat(foundPool2, is(true)); - - // Updating my_pool2 - Settings settings = Settings.builder() - .put("threadpool.my_pool2.size", "10") - .build(); - threadPool.updateSettings(settings); - - groups = threadPool.info(); - foundPool1 = false; - foundPool2 = false; - outer: for (ThreadPool.Info info : groups) { - if ("my_pool1".equals(info.getName())) { - foundPool1 = true; - assertThat(info.getType(), equalTo("cached")); - } else if ("my_pool2".equals(info.getName())) { - foundPool2 = true; - assertThat(info.getMax(), equalTo(10)); - assertThat(info.getMin(), equalTo(10)); - assertThat(info.getQueueSize().singles(), equalTo(1l)); - assertThat(info.getType(), equalTo("fixed")); - } else { - for (Field field : Names.class.getFields()) { - if (info.getName().equalsIgnoreCase(field.getName())) { - // This is ok it is a default thread pool - continue outer; - } - } - fail("Unexpected pool name: " + info.getName()); - } - } - assertThat(foundPool1, is(true)); - assertThat(foundPool2, is(true)); - terminate(threadPool); - } - } diff --git a/docs/reference/migration/migrate_3_0.asciidoc b/docs/reference/migration/migrate_3_0.asciidoc index e0bdff8ddee..aa602f9d64d 100644 --- a/docs/reference/migration/migrate_3_0.asciidoc +++ b/docs/reference/migration/migrate_3_0.asciidoc @@ -389,4 +389,12 @@ request cache and the field data cache. This setting would arbitrarily pick the first interface not marked as loopback. Instead, specify by address scope (e.g. `_local_,_site_` for all loopback and private network addresses) or by explicit interface names, -hostnames, or addresses. +hostnames, or addresses. + +=== Forbid changing of thread pool types + +Previously, <> could be dynamically adjusted. The thread pool type effectively +controls the backing queue for the thread pool and modifying this is an expert setting with minimal practical benefits +and high risk of being misused. The ability to change the thread pool type for any thread pool has been removed; do note +that it is still possible to adjust relevant thread pool parameters for each of the thread pools (e.g., depending on +the thread pool type, `keep_alive`, `queue_size`, etc.). diff --git a/docs/reference/modules/threadpool.asciidoc b/docs/reference/modules/threadpool.asciidoc index 591277889bc..bfd5474183c 100644 --- a/docs/reference/modules/threadpool.asciidoc +++ b/docs/reference/modules/threadpool.asciidoc @@ -9,87 +9,92 @@ of discarded. There are several thread pools, but the important ones include: +`generic`:: + For generic operations (e.g., background node discovery). + Thread pool type is `cached`. + `index`:: - For index/delete operations. Defaults to `fixed` + For index/delete operations. Thread pool type is `fixed` with a size of `# of available processors`, queue_size of `200`. `search`:: - For count/search operations. Defaults to `fixed` + For count/search operations. Thread pool type is `fixed` with a size of `int((# of available_processors * 3) / 2) + 1`, queue_size of `1000`. `suggest`:: - For suggest operations. Defaults to `fixed` + For suggest operations. Thread pool type is `fixed` with a size of `# of available processors`, queue_size of `1000`. `get`:: - For get operations. Defaults to `fixed` + For get operations. Thread pool type is `fixed` with a size of `# of available processors`, queue_size of `1000`. `bulk`:: - For bulk operations. Defaults to `fixed` + For bulk operations. Thread pool type is `fixed` with a size of `# of available processors`, queue_size of `50`. `percolate`:: - For percolate operations. Defaults to `fixed` + For percolate operations. Thread pool type is `fixed` with a size of `# of available processors`, queue_size of `1000`. `snapshot`:: - For snapshot/restore operations. Defaults to `scaling` with a - keep-alive of `5m` and a size of `min(5, (# of available processors)/2)`, max at 5. + For snapshot/restore operations. Thread pool type is `scaling` with a + keep-alive of `5m` and a size of `min(5, (# of available processors)/2)`. `warmer`:: - For segment warm-up operations. Defaults to `scaling` with a - keep-alive of `5m` and a size of `min(5, (# of available processors)/2)`, max at 5. + For segment warm-up operations. Thread pool type is `scaling` with a + keep-alive of `5m` and a size of `min(5, (# of available processors)/2)`. `refresh`:: - For refresh operations. Defaults to `scaling` with a - keep-alive of `5m` and a size of `min(10, (# of available processors)/2)`, max at 10. + For refresh operations. Thread pool type is `scaling` with a + keep-alive of `5m` and a size of `min(10, (# of available processors)/2)`. `listener`:: Mainly for java client executing of action when listener threaded is set to true. - Default size of `(# of available processors)/2`, max at 10. + Thread pool type is `scaling` with a default size of `min(10, (# of available processors)/2)`. -Changing a specific thread pool can be done by setting its type and -specific type parameters, for example, changing the `index` thread pool -to have more threads: +Changing a specific thread pool can be done by setting its type-specific parameters; for example, changing the `index` +thread pool to have more threads: [source,js] -------------------------------------------------- threadpool: index: - type: fixed size: 30 -------------------------------------------------- -NOTE: you can update threadpool settings live using - <>. - +NOTE: you can update thread pool settings dynamically using <>. [float] [[types]] === Thread pool types -The following are the types of thread pools that can be used and their -respective parameters: +The following are the types of thread pools and their respective parameters: [float] -==== `cache` +==== `cached` -The `cache` thread pool is an unbounded thread pool that will spawn a -thread if there are pending requests. Here is an example of how to set -it: +The `cached` thread pool is an unbounded thread pool that will spawn a +thread if there are pending requests. This thread pool is used to +prevent requests submitted to this pool from blocking or being +rejected. Unused threads in this thread pool will be terminated after +a keep alive expires (defaults to five minutes). The `cached` thread +pool is reserved for the <> thread pool. + +The `keep_alive` parameter determines how long a thread should be kept +around in the thread pool without doing any work. [source,js] -------------------------------------------------- threadpool: - index: - type: cached + generic: + keep_alive: 2m -------------------------------------------------- [float] @@ -111,7 +116,6 @@ full, it will abort the request. -------------------------------------------------- threadpool: index: - type: fixed size: 30 queue_size: 1000 -------------------------------------------------- @@ -130,7 +134,6 @@ around in the thread pool without it doing any work. -------------------------------------------------- threadpool: warmer: - type: scaling size: 8 keep_alive: 2m -------------------------------------------------- diff --git a/test-framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test-framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 6cafe4c3b79..4f8e64cc1a0 100644 --- a/test-framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test-framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -64,10 +64,10 @@ import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.index.IndexModule; import org.elasticsearch.index.IndexService; +import org.elasticsearch.index.MockEngineFactoryPlugin; import org.elasticsearch.index.engine.CommitStats; import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.shard.IndexShard; -import org.elasticsearch.index.MockEngineFactoryPlugin; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.store.IndexStoreConfig; import org.elasticsearch.indices.IndicesService; @@ -88,7 +88,6 @@ import org.elasticsearch.test.disruption.ServiceDisruptionScheme; import org.elasticsearch.test.store.MockFSIndexStore; import org.elasticsearch.test.transport.AssertingLocalTransport; import org.elasticsearch.test.transport.MockTransportService; -import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.Transport; import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.netty.NettyTransport; @@ -98,20 +97,11 @@ import java.io.Closeable; import java.io.IOException; import java.net.InetSocketAddress; import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.NavigableMap; -import java.util.Random; -import java.util.Set; -import java.util.TreeMap; -import java.util.concurrent.*; +import java.util.*; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; @@ -119,15 +109,11 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import static junit.framework.Assert.fail; -import static org.apache.lucene.util.LuceneTestCase.TEST_NIGHTLY; -import static org.apache.lucene.util.LuceneTestCase.rarely; -import static org.apache.lucene.util.LuceneTestCase.usually; +import static org.apache.lucene.util.LuceneTestCase.*; import static org.elasticsearch.common.settings.Settings.settingsBuilder; import static org.elasticsearch.test.ESTestCase.assertBusy; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.assertThat; /** @@ -404,18 +390,6 @@ public final class InternalTestCluster extends TestCluster { if (random.nextBoolean()) { // sometimes set a builder.put(SearchService.DEFAULT_KEEPALIVE_KEY, TimeValue.timeValueSeconds(100 + random.nextInt(5 * 60))); } - if (random.nextBoolean()) { - // change threadpool types to make sure we don't have components that rely on the type of thread pools - for (String name : Arrays.asList(ThreadPool.Names.BULK, ThreadPool.Names.FLUSH, ThreadPool.Names.GET, - ThreadPool.Names.INDEX, ThreadPool.Names.MANAGEMENT, ThreadPool.Names.FORCE_MERGE, - ThreadPool.Names.PERCOLATE, ThreadPool.Names.REFRESH, ThreadPool.Names.SEARCH, ThreadPool.Names.SNAPSHOT, - ThreadPool.Names.SUGGEST, ThreadPool.Names.WARMER)) { - if (random.nextBoolean()) { - final String type = RandomPicks.randomFrom(random, Arrays.asList("fixed", "cached", "scaling")); - builder.put(ThreadPool.THREADPOOL_GROUP + name + ".type", type); - } - } - } if (random.nextInt(10) == 0) { // node gets an extra cpu this time From 48ee09fbd457eddb27e63165f3df657f7d0a8da2 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 2 Nov 2015 22:14:28 -0500 Subject: [PATCH 15/23] Add missing license file to ThreadPoolTypeSettingsValidatorTests.java --- .../ThreadPoolTypeSettingsValidatorTests.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolTypeSettingsValidatorTests.java b/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolTypeSettingsValidatorTests.java index aa3b2a8dec2..3dfca5cb283 100644 --- a/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolTypeSettingsValidatorTests.java +++ b/core/src/test/java/org/elasticsearch/threadpool/ThreadPoolTypeSettingsValidatorTests.java @@ -1,3 +1,22 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed wit[√h + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + package org.elasticsearch.threadpool; import org.elasticsearch.cluster.ClusterState; From d81993026112b4557c5d387b4348a1d0f18865b8 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 15 Oct 2015 17:25:39 +0200 Subject: [PATCH 16/23] [Doc] Fix correct number of slashes when installing a plugin with zip file --- docs/plugins/authors.asciidoc | 2 +- docs/plugins/plugin-script.asciidoc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plugins/authors.asciidoc b/docs/plugins/authors.asciidoc index c0f310683f3..75b7776ec09 100644 --- a/docs/plugins/authors.asciidoc +++ b/docs/plugins/authors.asciidoc @@ -112,7 +112,7 @@ directory in the root of the plugin should be served. === Testing your plugin When testing a Java plugin, it will only be auto-loaded if it is in the -`plugins/` directory. Use `bin/plugin install file://path/to/your/plugin` +`plugins/` directory. Use `bin/plugin install file:///path/to/your/plugin` to install your plugin for testing. You may also load your plugin within the test framework for integration tests. diff --git a/docs/plugins/plugin-script.asciidoc b/docs/plugins/plugin-script.asciidoc index 52ff574cfc7..e334fcc718f 100644 --- a/docs/plugins/plugin-script.asciidoc +++ b/docs/plugins/plugin-script.asciidoc @@ -101,7 +101,7 @@ For instance, to install a plugin from your local file system, you could run: [source,shell] ----------------------------------- -sudo bin/plugin install file:/path/to/plugin.zip +sudo bin/plugin install file:///path/to/plugin.zip ----------------------------------- [[listing-removing]] From 3f94a566ff5005e4c4819706f3032a995dfec97d Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 2 Nov 2015 10:15:49 +0100 Subject: [PATCH 17/23] Deduplicate cause if already contained in shard failures If we have a shard failure on SearchPhaseExecutionException we can deduplicate the original cause and use the more informative ShardSearchFailure containing the shard ID etc. but we should deduplicate the actual cause to prevent stack trace duplication. --- .../search/SearchPhaseExecutionException.java | 26 ++++++++++++++----- ...nsportSearchScrollQueryAndFetchAction.java | 4 +-- ...sportSearchScrollQueryThenFetchAction.java | 8 +++--- .../type/TransportSearchTypeAction.java | 6 +++-- .../org/elasticsearch/ESExceptionTests.java | 17 +++++++++++- .../index/query/TemplateQueryIT.java | 2 +- 6 files changed, 46 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/search/SearchPhaseExecutionException.java b/core/src/main/java/org/elasticsearch/action/search/SearchPhaseExecutionException.java index 68bcf6d269c..40e8b0730ff 100644 --- a/core/src/main/java/org/elasticsearch/action/search/SearchPhaseExecutionException.java +++ b/core/src/main/java/org/elasticsearch/action/search/SearchPhaseExecutionException.java @@ -38,13 +38,11 @@ public class SearchPhaseExecutionException extends ElasticsearchException { private final ShardSearchFailure[] shardFailures; public SearchPhaseExecutionException(String phaseName, String msg, ShardSearchFailure[] shardFailures) { - super(msg); - this.phaseName = phaseName; - this.shardFailures = shardFailures; + this(phaseName, msg, null, shardFailures); } public SearchPhaseExecutionException(String phaseName, String msg, Throwable cause, ShardSearchFailure[] shardFailures) { - super(msg, cause); + super(msg, deduplicateCause(cause, shardFailures)); this.phaseName = phaseName; this.shardFailures = shardFailures; } @@ -63,12 +61,26 @@ public class SearchPhaseExecutionException extends ElasticsearchException { public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeOptionalString(phaseName); - out.writeVInt(shardFailures == null ? 0 : shardFailures.length); - if (shardFailures != null) { + out.writeVInt(shardFailures.length); + for (ShardSearchFailure failure : shardFailures) { + failure.writeTo(out); + } + } + + private static final Throwable deduplicateCause(Throwable cause, ShardSearchFailure[] shardFailures) { + if (shardFailures == null) { + throw new IllegalArgumentException("shardSearchFailures must not be null"); + } + // if the cause of this exception is also the cause of one of the shard failures we don't add it + // to prevent duplication in stack traces rendered to the REST layer + if (cause != null) { for (ShardSearchFailure failure : shardFailures) { - failure.writeTo(out); + if (failure.getCause() == cause) { + return null; + } } } + return cause; } @Override diff --git a/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchScrollQueryAndFetchAction.java b/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchScrollQueryAndFetchAction.java index cd4238ccdea..2a953f9b732 100644 --- a/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchScrollQueryAndFetchAction.java +++ b/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchScrollQueryAndFetchAction.java @@ -114,7 +114,7 @@ public class TransportSearchScrollQueryAndFetchAction extends AbstractComponent public void start() { if (scrollId.getContext().length == 0) { - listener.onFailure(new SearchPhaseExecutionException("query", "no nodes to search on", null)); + listener.onFailure(new SearchPhaseExecutionException("query", "no nodes to search on", ShardSearchFailure.EMPTY_ARRAY)); return; } @@ -175,7 +175,7 @@ public class TransportSearchScrollQueryAndFetchAction extends AbstractComponent successfulOps.decrementAndGet(); if (counter.decrementAndGet() == 0) { if (successfulOps.get() == 0) { - listener.onFailure(new SearchPhaseExecutionException("query_fetch", "all shards failed", buildShardFailures())); + listener.onFailure(new SearchPhaseExecutionException("query_fetch", "all shards failed", t, buildShardFailures())); } else { finishHim(); } diff --git a/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchScrollQueryThenFetchAction.java b/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchScrollQueryThenFetchAction.java index 85b06ea7860..8f2df714319 100644 --- a/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchScrollQueryThenFetchAction.java +++ b/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchScrollQueryThenFetchAction.java @@ -123,7 +123,7 @@ public class TransportSearchScrollQueryThenFetchAction extends AbstractComponent public void start() { if (scrollId.getContext().length == 0) { - listener.onFailure(new SearchPhaseExecutionException("query", "no nodes to search on", null)); + listener.onFailure(new SearchPhaseExecutionException("query", "no nodes to search on", ShardSearchFailure.EMPTY_ARRAY)); return; } final AtomicInteger counter = new AtomicInteger(scrollId.getContext().length); @@ -143,7 +143,7 @@ public class TransportSearchScrollQueryThenFetchAction extends AbstractComponent try { executeFetchPhase(); } catch (Throwable e) { - listener.onFailure(new SearchPhaseExecutionException("query", "Fetch failed", e, null)); + listener.onFailure(new SearchPhaseExecutionException("query", "Fetch failed", e, ShardSearchFailure.EMPTY_ARRAY)); return; } } @@ -181,12 +181,12 @@ public class TransportSearchScrollQueryThenFetchAction extends AbstractComponent successfulOps.decrementAndGet(); if (counter.decrementAndGet() == 0) { if (successfulOps.get() == 0) { - listener.onFailure(new SearchPhaseExecutionException("query", "all shards failed", buildShardFailures())); + listener.onFailure(new SearchPhaseExecutionException("query", "all shards failed", t, buildShardFailures())); } else { try { executeFetchPhase(); } catch (Throwable e) { - listener.onFailure(new SearchPhaseExecutionException("query", "Fetch failed", e, null)); + listener.onFailure(new SearchPhaseExecutionException("query", "Fetch failed", e, ShardSearchFailure.EMPTY_ARRAY)); } } } diff --git a/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchTypeAction.java b/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchTypeAction.java index fa5776387dd..31cd3986d2f 100644 --- a/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchTypeAction.java +++ b/core/src/main/java/org/elasticsearch/action/search/type/TransportSearchTypeAction.java @@ -220,17 +220,19 @@ public abstract class TransportSearchTypeAction extends TransportAction Date: Tue, 3 Nov 2015 11:35:47 +0100 Subject: [PATCH 18/23] Fix test bug in UpdateThreadPoolSettingsTests. --- .../threadpool/UpdateThreadPoolSettingsTests.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/org/elasticsearch/threadpool/UpdateThreadPoolSettingsTests.java b/core/src/test/java/org/elasticsearch/threadpool/UpdateThreadPoolSettingsTests.java index faa08243af0..95ceea1e490 100644 --- a/core/src/test/java/org/elasticsearch/threadpool/UpdateThreadPoolSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/threadpool/UpdateThreadPoolSettingsTests.java @@ -49,7 +49,13 @@ public class UpdateThreadPoolSettingsTests extends ESTestCase { .put("name", "testCorrectThreadPoolTypePermittedInSettings") .put("threadpool." + threadPoolName + ".type", correctThreadPoolType.getType()) .build()); - assertEquals(info(threadPool, threadPoolName).getThreadPoolType(), correctThreadPoolType); + ThreadPool.Info info = info(threadPool, threadPoolName); + if (ThreadPool.Names.SAME.equals(threadPoolName)) { + assertNull(info); // we don't report on the "same" threadpool + } else { + // otherwise check we have the expected type + assertEquals(info.getThreadPoolType(), correctThreadPoolType); + } } finally { terminateThreadPoolIfNeeded(threadPool); } From 2fe519c3e71c78536b21fa16fe0de27e53544334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 3 Nov 2015 11:48:11 +0100 Subject: [PATCH 19/23] Revert fd3a46a because fix is to fragile Reverting fix for #13884 because it was discussed to be too fragile with respect to future changes in lucene simple query string parsing. Undoes fix and removes test. --- .../index/query/SimpleQueryParser.java | 22 +-- .../index/query/SimpleQueryStringBuilder.java | 18 +-- .../index/query/AbstractQueryTestCase.java | 2 +- .../query/SimpleQueryStringBuilderTests.java | 131 +++++------------- .../search/query/SimpleQueryStringIT.java | 15 +- 5 files changed, 48 insertions(+), 140 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java index b38552663d1..f8b0deaf9be 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java @@ -70,7 +70,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp rethrowUnlessLenient(e); } } - return simplify(bq.build()); + return super.simplify(bq.build()); } /** @@ -93,7 +93,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp rethrowUnlessLenient(e); } } - return simplify(bq.build()); + return super.simplify(bq.build()); } @Override @@ -111,7 +111,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp rethrowUnlessLenient(e); } } - return simplify(bq.build()); + return super.simplify(bq.build()); } /** @@ -140,19 +140,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp return rethrowUnlessLenient(e); } } - return simplify(bq.build()); - } - - /** - * Override of lucenes SimpleQueryParser that doesn't simplify for the 1-clause case. - */ - @Override - protected Query simplify(BooleanQuery bq) { - if (bq.clauses().isEmpty()) { - return null; - } else { - return bq; - } + return super.simplify(bq.build()); } /** @@ -307,7 +295,7 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp // For further reasoning see // https://issues.apache.org/jira/browse/LUCENE-4021 return (Objects.equals(locale.toLanguageTag(), other.locale.toLanguageTag()) - && Objects.equals(lowercaseExpandedTerms, other.lowercaseExpandedTerms) + && Objects.equals(lowercaseExpandedTerms, other.lowercaseExpandedTerms) && Objects.equals(lenient, other.lenient) && Objects.equals(analyzeWildcard, other.analyzeWildcard)); } diff --git a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java index 2b221ed9ab0..a93c60ec147 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java @@ -20,8 +20,6 @@ package org.elasticsearch.index.query; import org.apache.lucene.analysis.Analyzer; -import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BooleanClause.Occur; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.Strings; @@ -287,20 +285,8 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder 1 - && ((booleanQuery.clauses().iterator().next().getQuery() instanceof BooleanQuery) == false)) { - // special case for one term query and more than one field: (f1:t1 f2:t1 f3:t1) - // we need to wrap this in additional BooleanQuery so minimum_should_match is applied correctly - BooleanQuery.Builder builder = new BooleanQuery.Builder(); - builder.add(new BooleanClause(booleanQuery, Occur.SHOULD)); - booleanQuery = builder.build(); - } - if (minimumShouldMatch != null) { - booleanQuery = Queries.applyMinimumShouldMatch(booleanQuery, minimumShouldMatch); - } - query = booleanQuery; + if (minimumShouldMatch != null && query instanceof BooleanQuery) { + query = Queries.applyMinimumShouldMatch((BooleanQuery) query, minimumShouldMatch); } return query; } diff --git a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java index c553e94e29f..58cf1f3e522 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java @@ -523,7 +523,7 @@ public abstract class AbstractQueryTestCase> testQuery.writeTo(output); try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) { QueryBuilder prototype = queryParser(testQuery.getName()).getBuilderPrototype(); - QueryBuilder deserializedQuery = prototype.readFrom(in); + QueryBuilder deserializedQuery = prototype.readFrom(in); assertEquals(deserializedQuery, testQuery); assertEquals(deserializedQuery.hashCode(), testQuery.hashCode()); assertNotSame(deserializedQuery, testQuery); diff --git a/core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java index ac11c67e516..deb0f5a99fa 100644 --- a/core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java @@ -38,29 +38,19 @@ import java.util.HashSet; import java.util.Iterator; import java.util.Locale; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; -import java.util.TreeMap; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase { - private String[] queryTerms; - @Override protected SimpleQueryStringBuilder doCreateTestQueryBuilder() { - int numberOfTerms = randomIntBetween(1, 5); - queryTerms = new String[numberOfTerms]; - StringBuilder queryString = new StringBuilder(); - for (int i = 0; i < numberOfTerms; i++) { - queryTerms[i] = randomAsciiOfLengthBetween(1, 10); - queryString.append(queryTerms[i] + " "); - } - SimpleQueryStringBuilder result = new SimpleQueryStringBuilder(queryString.toString().trim()); + SimpleQueryStringBuilder result = new SimpleQueryStringBuilder(randomAsciiOfLengthBetween(1, 10)); if (randomBoolean()) { result.analyzeWildcard(randomBoolean()); } @@ -84,13 +74,9 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase flagSet = new HashSet<>(); - if (numberOfTerms > 1) { - flagSet.add(SimpleQueryStringFlag.WHITESPACE); - } int size = randomIntBetween(0, SimpleQueryStringFlag.values().length); for (int i = 0; i < size; i++) { - SimpleQueryStringFlag randomFlag = randomFrom(SimpleQueryStringFlag.values()); - flagSet.add(randomFlag); + flagSet.add(randomFrom(SimpleQueryStringFlag.values())); } if (flagSet.size() > 0) { result.flags(flagSet.toArray(new SimpleQueryStringFlag[flagSet.size()])); @@ -101,12 +87,13 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase fields = new HashMap<>(); for (int i = 0; i < fieldCount; i++) { if (randomBoolean()) { - fields.put("f" + i + "_" + randomAsciiOfLengthBetween(1, 10), AbstractQueryBuilder.DEFAULT_BOOST); + fields.put(randomAsciiOfLengthBetween(1, 10), AbstractQueryBuilder.DEFAULT_BOOST); } else { - fields.put(randomBoolean() ? STRING_FIELD_NAME : "f" + i + "_" + randomAsciiOfLengthBetween(1, 10), 2.0f / randomIntBetween(1, 20)); + fields.put(randomBoolean() ? STRING_FIELD_NAME : randomAsciiOfLengthBetween(1, 10), 2.0f / randomIntBetween(1, 20)); } } result.fields(fields); + return result; } @@ -271,8 +258,8 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase 0 || shardContext.indexQueryParserService().getIndexCreatedVersion().before(Version.V_1_4_0_Beta1)) { Query luceneQuery = queryBuilder.toQuery(shardContext); - assertThat(luceneQuery, instanceOf(BooleanQuery.class)); - TermQuery termQuery = (TermQuery) ((BooleanQuery) luceneQuery).clauses().get(0).getQuery(); + assertThat(luceneQuery, instanceOf(TermQuery.class)); + TermQuery termQuery = (TermQuery) luceneQuery; assertThat(termQuery.getTerm(), equalTo(new Term(MetaData.ALL, query))); } } @@ -290,7 +277,7 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase 1) { assertTrue("Query should have been BooleanQuery but was " + query.getClass().getName(), query instanceof BooleanQuery); BooleanQuery boolQuery = (BooleanQuery) query; @@ -303,42 +290,32 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase expectedFields = new TreeMap(queryBuilder.fields()); - if (expectedFields.size() == 0) { - expectedFields.put(MetaData.ALL, AbstractQueryBuilder.DEFAULT_BOOST); - } - for (int i = 0; i < queryTerms.length; i++) { - BooleanClause booleanClause = boolQuery.clauses().get(i); - Iterator> fieldsIter = expectedFields.entrySet().iterator(); - - if (queryTerms.length == 1 && expectedFields.size() == 1) { - assertThat(booleanClause.getQuery(), instanceOf(TermQuery.class)); - TermQuery termQuery = (TermQuery) booleanClause.getQuery(); - Entry entry = fieldsIter.next(); - assertThat(termQuery.getTerm().field(), equalTo(entry.getKey())); - assertThat(termQuery.getBoost(), equalTo(entry.getValue())); - assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryTerms[i].toLowerCase(Locale.ROOT))); - } else { - assertThat(booleanClause.getQuery(), instanceOf(BooleanQuery.class)); - for (BooleanClause clause : ((BooleanQuery) booleanClause.getQuery()).clauses()) { - TermQuery termQuery = (TermQuery) clause.getQuery(); - Entry entry = fieldsIter.next(); - assertThat(termQuery.getTerm().field(), equalTo(entry.getKey())); - assertThat(termQuery.getBoost(), equalTo(entry.getValue())); - assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryTerms[i].toLowerCase(Locale.ROOT))); - } - } + assertThat(boolQuery.clauses().size(), equalTo(queryBuilder.fields().size())); + Iterator fields = queryBuilder.fields().keySet().iterator(); + for (BooleanClause booleanClause : boolQuery) { + assertThat(booleanClause.getQuery(), instanceOf(TermQuery.class)); + TermQuery termQuery = (TermQuery) booleanClause.getQuery(); + assertThat(termQuery.getTerm().field(), equalTo(fields.next())); + assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryBuilder.value().toLowerCase(Locale.ROOT))); } if (queryBuilder.minimumShouldMatch() != null) { - int optionalClauses = queryTerms.length; - if (queryBuilder.defaultOperator().equals(Operator.AND) && queryTerms.length > 1) { - optionalClauses = 0; - } - int expectedMinimumShouldMatch = Queries.calculateMinShouldMatch(optionalClauses, queryBuilder.minimumShouldMatch()); - assertEquals(expectedMinimumShouldMatch, boolQuery.getMinimumNumberShouldMatch()); + assertThat(boolQuery.getMinimumNumberShouldMatch(), greaterThan(0)); } + } else if (queryBuilder.fields().size() <= 1) { + assertTrue("Query should have been TermQuery but was " + query.getClass().getName(), query instanceof TermQuery); + + TermQuery termQuery = (TermQuery) query; + String field; + if (queryBuilder.fields().size() == 0) { + field = MetaData.ALL; + } else { + field = queryBuilder.fields().keySet().iterator().next(); + } + assertThat(termQuery.getTerm().field(), equalTo(field)); + assertThat(termQuery.getTerm().text().toLowerCase(Locale.ROOT), equalTo(queryBuilder.value().toLowerCase(Locale.ROOT))); + } else { + fail("Encountered lucene query type we do not have a validation implementation for in our " + SimpleQueryStringBuilderTests.class.getSimpleName()); } } @@ -364,18 +341,15 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase 1) { - expectedMinimumShouldMatch = 0; - } - - assertEquals(expectedMinimumShouldMatch, query.getMinimumNumberShouldMatch()); - for (BooleanClause clause : query.clauses()) { - if (numberOfFields == 1 && numberOfTerms == 1) { - assertTrue(clause.getQuery() instanceof TermQuery); - } else { - assertEquals(numberOfFields, ((BooleanQuery) clause.getQuery()).clauses().size()); - } - } - } } diff --git a/core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java b/core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java index 404b221e389..358122f54ec 100644 --- a/core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java +++ b/core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java @@ -109,6 +109,7 @@ public class SimpleQueryStringIT extends ESIntegTestCase { client().prepareIndex("test", "type1", "3").setSource("body", "foo bar"), client().prepareIndex("test", "type1", "4").setSource("body", "foo baz bar")); + logger.info("--> query 1"); SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")).get(); assertHitCount(searchResponse, 2l); @@ -119,13 +120,7 @@ public class SimpleQueryStringIT extends ESIntegTestCase { assertHitCount(searchResponse, 2l); assertSearchHits(searchResponse, "3", "4"); - logger.info("--> query 3"); // test case from #13884 - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo") - .field("body").field("body2").field("body3").minimumShouldMatch("-50%")).get(); - assertHitCount(searchResponse, 3l); - assertSearchHits(searchResponse, "1", "3", "4"); - - logger.info("--> query 4"); + logger.info("--> query 3"); searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body").field("body2").minimumShouldMatch("70%")).get(); assertHitCount(searchResponse, 2l); assertSearchHits(searchResponse, "3", "4"); @@ -136,17 +131,17 @@ public class SimpleQueryStringIT extends ESIntegTestCase { client().prepareIndex("test", "type1", "7").setSource("body2", "foo bar", "other", "foo"), client().prepareIndex("test", "type1", "8").setSource("body2", "foo baz bar", "other", "foo")); - logger.info("--> query 5"); + logger.info("--> query 4"); searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").field("body").field("body2").minimumShouldMatch("2")).get(); assertHitCount(searchResponse, 4l); assertSearchHits(searchResponse, "3", "4", "7", "8"); - logger.info("--> query 6"); + logger.info("--> query 5"); searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")).get(); assertHitCount(searchResponse, 5l); assertSearchHits(searchResponse, "3", "4", "6", "7", "8"); - logger.info("--> query 7"); + logger.info("--> query 6"); searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body2").field("other").minimumShouldMatch("70%")).get(); assertHitCount(searchResponse, 3l); assertSearchHits(searchResponse, "6", "7", "8"); From 43323c354115617b6f04eeb32b568a8092b9cab3 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 3 Nov 2015 07:40:49 -0500 Subject: [PATCH 20/23] Fix bug in cat thread pool This commit fixes a bug in cat thread pool. This bug resulted from a refactoring of the handling of thread pool types. To get the previously displayed thread pool type from the ThreadPoolType object, ThreadPoolType#getType needs to be called. --- .../org/elasticsearch/rest/action/cat/RestThreadPoolAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java b/core/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java index 2ad9defe2a7..fa2e662c738 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/cat/RestThreadPoolAction.java @@ -288,7 +288,7 @@ public class RestThreadPoolAction extends AbstractCatAction { } } - table.addCell(poolInfo == null ? null : poolInfo.getThreadPoolType()); + table.addCell(poolInfo == null ? null : poolInfo.getThreadPoolType().getType()); table.addCell(poolStats == null ? null : poolStats.getActive()); table.addCell(poolStats == null ? null : poolStats.getThreads()); table.addCell(poolStats == null ? null : poolStats.getQueue()); From ce6aa258a919c8490fb1b881d552b84f8b8b7a16 Mon Sep 17 00:00:00 2001 From: javanna Date: Mon, 2 Nov 2015 11:57:15 +0100 Subject: [PATCH 21/23] Remove support for query_binary and filter_binary query_binary and filter_binary are unused at this point, as we only parse on the coordinating node and the java api only holds structured java objects for queries and filters, meaning they all implement Writeable and get natively serialized. Relates to #14308 Closes #14433 --- .../index/query/QueryParseContext.java | 12 ------ .../query/FilterBinaryParseElement.java | 42 ------------------- .../search/query/QueryBinaryParseElement.java | 39 ----------------- .../search/query/QueryPhase.java | 4 -- docs/reference/migration/migrate_3_0.asciidoc | 4 ++ 5 files changed, 4 insertions(+), 97 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/search/query/FilterBinaryParseElement.java delete mode 100644 core/src/main/java/org/elasticsearch/search/query/QueryBinaryParseElement.java diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java index 70a6a18aab2..78d76d8292e 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java @@ -22,7 +22,6 @@ package org.elasticsearch.index.query; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; -import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.indices.query.IndicesQueriesRegistry; @@ -76,17 +75,6 @@ public class QueryParseContext { String fieldName = parser.currentName(); if ("query".equals(fieldName)) { queryBuilder = parseInnerQueryBuilder(); - } else if ("query_binary".equals(fieldName) || "queryBinary".equals(fieldName)) { - byte[] querySource = parser.binaryValue(); - XContentParser qSourceParser = XContentFactory.xContent(querySource).createParser(querySource); - QueryParseContext queryParseContext = new QueryParseContext(indicesQueriesRegistry); - queryParseContext.reset(qSourceParser); - try { - queryParseContext.parseFieldMatcher(parseFieldMatcher); - queryBuilder = queryParseContext.parseInnerQueryBuilder(); - } finally { - queryParseContext.reset(null); - } } else { throw new ParsingException(parser.getTokenLocation(), "request does not support [" + parser.currentName() + "]"); } diff --git a/core/src/main/java/org/elasticsearch/search/query/FilterBinaryParseElement.java b/core/src/main/java/org/elasticsearch/search/query/FilterBinaryParseElement.java deleted file mode 100644 index facb17e61f0..00000000000 --- a/core/src/main/java/org/elasticsearch/search/query/FilterBinaryParseElement.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.search.query; - -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.index.query.ParsedQuery; -import org.elasticsearch.search.SearchParseElement; -import org.elasticsearch.search.internal.SearchContext; - -/** - * - */ -public class FilterBinaryParseElement implements SearchParseElement { - - @Override - public void parse(XContentParser parser, SearchContext context) throws Exception { - byte[] filterSource = parser.binaryValue(); - try (XContentParser fSourceParser = XContentFactory.xContent(filterSource).createParser(filterSource)) { - ParsedQuery filter = context.queryParserService().parseInnerFilter(fSourceParser); - if (filter != null) { - context.parsedPostFilter(filter); - } - } - } -} \ No newline at end of file diff --git a/core/src/main/java/org/elasticsearch/search/query/QueryBinaryParseElement.java b/core/src/main/java/org/elasticsearch/search/query/QueryBinaryParseElement.java deleted file mode 100644 index 1d6119bddac..00000000000 --- a/core/src/main/java/org/elasticsearch/search/query/QueryBinaryParseElement.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.search.query; - -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.search.SearchParseElement; -import org.elasticsearch.search.internal.SearchContext; - -/** - * - */ -public class QueryBinaryParseElement implements SearchParseElement { - - @Override - public void parse(XContentParser parser, SearchContext context) throws Exception { - byte[] querySource = parser.binaryValue(); - try (XContentParser qSourceParser = XContentFactory.xContent(querySource).createParser(querySource)) { - context.parsedQuery(context.queryParserService().parse(qSourceParser)); - } - } -} \ No newline at end of file diff --git a/core/src/main/java/org/elasticsearch/search/query/QueryPhase.java b/core/src/main/java/org/elasticsearch/search/query/QueryPhase.java index 44fae80a020..ce8836cd336 100644 --- a/core/src/main/java/org/elasticsearch/search/query/QueryPhase.java +++ b/core/src/main/java/org/elasticsearch/search/query/QueryPhase.java @@ -90,12 +90,8 @@ public class QueryPhase implements SearchPhase { parseElements.put("indices_boost", new IndicesBoostParseElement()); parseElements.put("indicesBoost", new IndicesBoostParseElement()); parseElements.put("query", new QueryParseElement()); - parseElements.put("queryBinary", new QueryBinaryParseElement()); - parseElements.put("query_binary", new QueryBinaryParseElement()); parseElements.put("post_filter", new PostFilterParseElement()); parseElements.put("postFilter", new PostFilterParseElement()); - parseElements.put("filterBinary", new FilterBinaryParseElement()); - parseElements.put("filter_binary", new FilterBinaryParseElement()); parseElements.put("sort", new SortParseElement()); parseElements.put("trackScores", new TrackScoresParseElement()); parseElements.put("track_scores", new TrackScoresParseElement()); diff --git a/docs/reference/migration/migrate_3_0.asciidoc b/docs/reference/migration/migrate_3_0.asciidoc index aa602f9d64d..f190b7fdce6 100644 --- a/docs/reference/migration/migrate_3_0.asciidoc +++ b/docs/reference/migration/migrate_3_0.asciidoc @@ -117,6 +117,10 @@ Removed support for multiple highlighter names, the only supported ones are: `pl Removed support for the deprecated top level `filter` in the search api, replaced by `post_filter`. +==== `query_binary` and `filter_binary` removed + +Removed support for the undocumented `query_binary` and `filter_binary` sections of a search request. + === Parent/Child changes The `children` aggregation, parent child inner hits and `has_child` and `has_parent` queries will not work on indices From ea4e514385da30062fbfb3db942f284156dba71e Mon Sep 17 00:00:00 2001 From: David Tvaltchrelidze Date: Wed, 27 May 2015 14:30:17 +0100 Subject: [PATCH 22/23] Fix HTML response during redirection It misses a quote... Closes #11374 --- core/src/main/java/org/elasticsearch/http/HttpServer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/elasticsearch/http/HttpServer.java b/core/src/main/java/org/elasticsearch/http/HttpServer.java index f3b8c3f0a4b..9971ce7722d 100644 --- a/core/src/main/java/org/elasticsearch/http/HttpServer.java +++ b/core/src/main/java/org/elasticsearch/http/HttpServer.java @@ -189,7 +189,7 @@ public class HttpServer extends AbstractLifecycleComponent { sitePath = null; // If a trailing / is missing, we redirect to the right page #2654 String redirectUrl = request.rawPath() + "/"; - BytesRestResponse restResponse = new BytesRestResponse(RestStatus.MOVED_PERMANENTLY, "text/html", ""); + BytesRestResponse restResponse = new BytesRestResponse(RestStatus.MOVED_PERMANENTLY, "text/html", ""); restResponse.addHeader("Location", redirectUrl); channel.sendResponse(restResponse); return; From b6a23185638197cfa5f2cab612c558b9e73734fe Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Tue, 3 Nov 2015 09:52:25 -0500 Subject: [PATCH 23/23] upgrade rhino for plugins/lang-javascript the current jar is over 3 years old, we should upgrade it for bugfixes. the current integration could be more secure: set a global policy and enforce additional (compile-time) checks closes #14466 --- .../elasticsearch/bootstrap/untrusted.policy | 5 ++- plugins/lang-javascript/build.gradle | 2 +- .../licenses/rhino-1.7.7.jar.sha1 | 1 + .../licenses/rhino-1.7R4.jar.sha1 | 1 - .../plugin/javascript/JavaScriptPlugin.java | 5 +++ .../JavaScriptScriptEngineService.java | 40 ++++++++++++++----- .../javascript/JavaScriptSecurityTests.java | 23 ++++++++--- .../script/python/PythonSecurityTests.java | 14 ++++--- 8 files changed, 65 insertions(+), 26 deletions(-) create mode 100644 plugins/lang-javascript/licenses/rhino-1.7.7.jar.sha1 delete mode 100644 plugins/lang-javascript/licenses/rhino-1.7R4.jar.sha1 diff --git a/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy b/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy index 2475c56e814..dbbc4f14d7e 100644 --- a/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy +++ b/core/src/main/resources/org/elasticsearch/bootstrap/untrusted.policy @@ -19,13 +19,16 @@ /* * Limited security policy for scripts. - * This is what is needed for invokeDynamic functionality to work. + * This is what is needed for basic functionality to work. */ grant { // groovy IndyInterface bootstrap requires this property for indy logging permission java.util.PropertyPermission "groovy.indy.logging", "read"; + // needed by Rhino engine exception handling + permission java.util.PropertyPermission "rhino.stack.style", "read"; + // needed IndyInterface selectMethod (setCallSiteTarget) permission java.lang.RuntimePermission "getClassLoader"; }; diff --git a/plugins/lang-javascript/build.gradle b/plugins/lang-javascript/build.gradle index 938d7cc5e73..ead459f29d1 100644 --- a/plugins/lang-javascript/build.gradle +++ b/plugins/lang-javascript/build.gradle @@ -23,7 +23,7 @@ esplugin { } dependencies { - compile 'org.mozilla:rhino:1.7R4' + compile 'org.mozilla:rhino:1.7.7' } compileJava.options.compilerArgs << "-Xlint:-rawtypes,-unchecked" diff --git a/plugins/lang-javascript/licenses/rhino-1.7.7.jar.sha1 b/plugins/lang-javascript/licenses/rhino-1.7.7.jar.sha1 new file mode 100644 index 00000000000..8c997d41c2b --- /dev/null +++ b/plugins/lang-javascript/licenses/rhino-1.7.7.jar.sha1 @@ -0,0 +1 @@ +3a9ea863b86126b0ed8f2fe2230412747cd3c254 \ No newline at end of file diff --git a/plugins/lang-javascript/licenses/rhino-1.7R4.jar.sha1 b/plugins/lang-javascript/licenses/rhino-1.7R4.jar.sha1 deleted file mode 100644 index 3432f18a5de..00000000000 --- a/plugins/lang-javascript/licenses/rhino-1.7R4.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -e982f2136574b9a423186fbaeaaa98dc3e5a5288 diff --git a/plugins/lang-javascript/src/main/java/org/elasticsearch/plugin/javascript/JavaScriptPlugin.java b/plugins/lang-javascript/src/main/java/org/elasticsearch/plugin/javascript/JavaScriptPlugin.java index a6832fe7afe..9ca36bd9f86 100644 --- a/plugins/lang-javascript/src/main/java/org/elasticsearch/plugin/javascript/JavaScriptPlugin.java +++ b/plugins/lang-javascript/src/main/java/org/elasticsearch/plugin/javascript/JavaScriptPlugin.java @@ -28,6 +28,11 @@ import org.elasticsearch.script.javascript.JavaScriptScriptEngineService; */ public class JavaScriptPlugin extends Plugin { + static { + // install rhino policy on plugin init + JavaScriptScriptEngineService.init(); + } + @Override public String name() { return "lang-javascript"; diff --git a/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java b/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java index 70fc5f46d63..621d338128f 100644 --- a/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java +++ b/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java @@ -58,6 +58,34 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements private Scriptable globalScope; + // one time initialization of rhino security manager integration + private static final CodeSource DOMAIN; + static { + try { + DOMAIN = new CodeSource(new URL("file:" + BootstrapInfo.UNTRUSTED_CODEBASE), (Certificate[]) null); + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + SecurityController.initGlobal(new PolicySecurityController() { + @Override + public GeneratedClassLoader createClassLoader(ClassLoader parent, Object securityDomain) { + // don't let scripts compile other scripts + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + sm.checkPermission(new SpecialPermission()); + } + // check the domain, this is all we allow + if (securityDomain != DOMAIN) { + throw new SecurityException("illegal securityDomain: " + securityDomain); + } + return super.createClassLoader(parent, securityDomain); + } + }); + } + + /** ensures this engine is initialized */ + public static void init() {} + @Inject public JavaScriptScriptEngineService(Settings settings) { super(settings); @@ -100,21 +128,11 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements @Override public Object compile(String script) { - // we don't know why kind of safeguards rhino has, - // but just be safe - SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(new SpecialPermission()); - } Context ctx = Context.enter(); try { ctx.setWrapFactory(wrapFactory); ctx.setOptimizationLevel(optimizationLevel); - ctx.setSecurityController(new PolicySecurityController()); - return ctx.compileString(script, generateScriptName(), 1, - new CodeSource(new URL("file:" + BootstrapInfo.UNTRUSTED_CODEBASE), (Certificate[]) null)); - } catch (MalformedURLException e) { - throw new RuntimeException(e); + return ctx.compileString(script, generateScriptName(), 1, DOMAIN); } finally { Context.exit(); } diff --git a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java index 887317fb744..410099de0a7 100644 --- a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java +++ b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptSecurityTests.java @@ -23,8 +23,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; -import org.junit.After; -import org.junit.Before; import org.mozilla.javascript.WrappedException; import java.util.HashMap; @@ -37,14 +35,18 @@ public class JavaScriptSecurityTests extends ESTestCase { private JavaScriptScriptEngineService se; - @Before - public void setup() { + @Override + public void setUp() throws Exception { + super.setUp(); se = new JavaScriptScriptEngineService(Settings.Builder.EMPTY_SETTINGS); + // otherwise will exit your VM and other bad stuff + assumeTrue("test requires security manager to be enabled", System.getSecurityManager() != null); } - @After - public void close() { + @Override + public void tearDown() throws Exception { se.close(); + super.tearDown(); } /** runs a script */ @@ -86,4 +88,13 @@ public class JavaScriptSecurityTests extends ESTestCase { // no files assertFailure("java.io.File.createTempFile(\"test\", \"tmp\")"); } + + public void testDefinitelyNotOK() { + // no mucking with security controller + assertFailure("var ctx = org.mozilla.javascript.Context.getCurrentContext(); " + + "ctx.setSecurityController(new org.mozilla.javascript.PolicySecurityController());"); + // no compiling scripts from scripts + assertFailure("var ctx = org.mozilla.javascript.Context.getCurrentContext(); " + + "ctx.compileString(\"1 + 1\", \"foobar\", 1, null); "); + } } diff --git a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java index fd60607e2e6..c4a80a41916 100644 --- a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java +++ b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonSecurityTests.java @@ -23,8 +23,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; -import org.junit.After; -import org.junit.Before; import org.python.core.PyException; import java.util.HashMap; @@ -37,17 +35,21 @@ public class PythonSecurityTests extends ESTestCase { private PythonScriptEngineService se; - @Before - public void setup() { + @Override + public void setUp() throws Exception { + super.setUp(); se = new PythonScriptEngineService(Settings.Builder.EMPTY_SETTINGS); + // otherwise will exit your VM and other bad stuff + assumeTrue("test requires security manager to be enabled", System.getSecurityManager() != null); } - @After - public void close() { + @Override + public void tearDown() throws Exception { // We need to clear some system properties System.clearProperty("python.cachedir.skip"); System.clearProperty("python.console.encoding"); se.close(); + super.tearDown(); } /** runs a script */