From 2ee128d87ae2575fd4b6680ab98d7e4419306397 Mon Sep 17 00:00:00 2001 From: Diwas Joshi Date: Tue, 27 Mar 2018 02:03:43 +0530 Subject: [PATCH 1/5] soft limit added for max number of search contexts --- .../common/settings/ClusterSettings.java | 1 + .../java/org/elasticsearch/node/Node.java | 8 ++++ .../elasticsearch/search/SearchService.java | 22 +++++++--- .../node/InternalSettingsPreparerTests.java | 7 ++++ .../search/SearchServiceTests.java | 40 ++++++++++++++++++- 5 files changed, 71 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 804340d63ed11..402b11402bb4d 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -372,6 +372,7 @@ public void apply(Settings value, Settings current, Settings previous) { Node.NODE_INGEST_SETTING, Node.NODE_ATTRIBUTES, Node.NODE_LOCAL_STORAGE_SETTING, + Node.MAX_SEARCH_CONTEXT_SETTING, TransportMasterNodeReadAction.FORCE_LOCAL_SETTING, AutoCreateIndex.AUTO_CREATE_INDEX_SETTING, BaseRestHandler.MULTI_ALLOW_EXPLICIT_INDEX, diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 081c588525ed9..b6e5bfc1e447c 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -214,6 +214,14 @@ public class Node implements Closeable { } }, Setting.Property.NodeScope); + /** + * A setting describing the maximum number of search contexts that can be created. The default + * maximum of 100 is defensive to prevent generating too many search contexts. + */ + public static final Setting MAX_SEARCH_CONTEXT_SETTING = + Setting.intSetting("node.max_search_context", 100, 0, Property.NodeScope); + + /** * Adds a default node name to the given setting, if it doesn't already exist * @return the given setting if node name is already set, or a new copy with a default node name set. diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 047e7f47e62ec..6cb93f867ac28 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -55,6 +55,8 @@ import org.elasticsearch.index.shard.SearchOperationListener; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; +import org.elasticsearch.node.Node; +import org.elasticsearch.node.NodeService; import org.elasticsearch.node.ResponseCollectorService; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.SearchScript; @@ -161,7 +163,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private volatile TimeValue defaultSearchTimeout; private volatile boolean defaultAllowPartialSearchResults; - + private volatile boolean lowLevelCancellation; private final Cancellable keepAliveReaper; @@ -198,10 +200,10 @@ public SearchService(ClusterService clusterService, IndicesService indicesServic clusterService.getClusterSettings().addSettingsUpdateConsumer(DEFAULT_SEARCH_TIMEOUT_SETTING, this::setDefaultSearchTimeout); defaultAllowPartialSearchResults = DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS.get(settings); - clusterService.getClusterSettings().addSettingsUpdateConsumer(DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS, + clusterService.getClusterSettings().addSettingsUpdateConsumer(DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS, this::setDefaultAllowPartialSearchResults); - - + + lowLevelCancellation = LOW_LEVEL_CANCELLATION_SETTING.get(settings); clusterService.getClusterSettings().addSettingsUpdateConsumer(LOW_LEVEL_CANCELLATION_SETTING, this::setLowLevelCancellation); } @@ -227,10 +229,10 @@ private void setDefaultSearchTimeout(TimeValue defaultSearchTimeout) { private void setDefaultAllowPartialSearchResults(boolean defaultAllowPartialSearchResults) { this.defaultAllowPartialSearchResults = defaultAllowPartialSearchResults; } - + public boolean defaultAllowPartialSearchResults() { return defaultAllowPartialSearchResults; - } + } private void setLowLevelCancellation(Boolean lowLevelCancellation) { this.lowLevelCancellation = lowLevelCancellation; @@ -566,6 +568,14 @@ final SearchContext createAndPutContext(ShardSearchRequest request) throws IOExc final SearchContext createContext(ShardSearchRequest request) throws IOException { final DefaultSearchContext context = createSearchContext(request, defaultSearchTimeout); + System.out.println("----activecontexts----" + activeContexts.size()); + if (activeContexts.size() >= Node.MAX_SEARCH_CONTEXT_SETTING.get(settings)) { + throw new IllegalStateException( + "Trying to create too many search contexts. Must be less than or equal to: [" + + Node.MAX_SEARCH_CONTEXT_SETTING.get(settings) + "]. This limit can be set by changing the [" + + Node.MAX_SEARCH_CONTEXT_SETTING.getKey() + "] node level setting."); + } + try { if (request.scroll() != null) { context.scrollContext(new ScrollContext()); diff --git a/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java b/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java index b8c3c158e4dd0..234dfd2ffd3c7 100644 --- a/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java +++ b/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java @@ -77,6 +77,13 @@ public void testEmptySettings() { assertTrue(configDir, configDir.startsWith(home)); } + public void testMaxSearchContexts() { + Settings settings = InternalSettingsPreparer.prepareSettings(Settings.EMPTY); + assertEquals(100, settings.get("node.max_search_context")); + settings = InternalSettingsPreparer.prepareSettings(Settings.builder().put("cluster.max_search_context", "20").build()); + assertEquals(20, settings.get("node.max_search_context")); + } + public void testDefaultClusterName() { Settings settings = InternalSettingsPreparer.prepareSettings(Settings.EMPTY); assertEquals("elasticsearch", settings.get("cluster.name")); diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index f5552ee0d2e46..7034a969cc69d 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -45,6 +45,7 @@ import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.indices.IndicesService; +import org.elasticsearch.node.Node; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.script.MockScriptEngine; @@ -343,6 +344,43 @@ searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_A } } + + /** + * test that creating more than the allowed number of search contexts throws an exception + */ + public void testMaxSearchContexts() throws IOException { + createIndex("index"); + final SearchService service = getInstanceFromNode(SearchService.class); + final IndicesService indicesService = getInstanceFromNode(IndicesService.class); + final IndexService indexService = indicesService.indexServiceSafe(resolveIndex("index")); + final IndexShard indexShard = indexService.getShard(0); + + for (int i = 0; i < Node.MAX_SEARCH_CONTEXT_SETTING.get(Settings.EMPTY); i++) { + SearchContext context = service.createAndPutContext( + new ShardSearchLocalRequest( + indexShard.shardId(), + 1, + SearchType.DEFAULT, + new SearchSourceBuilder(), + new String[0], + false, + new AliasFilter(null, Strings.EMPTY_ARRAY), + 1.0f, true) + ); + } + + try (SearchContext context = service.createAndPutContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + new SearchSourceBuilder(), new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))) { + assertNotNull(context); + } catch (IllegalStateException ex) { + assertEquals( + "Trying to create too many search contexts. Must be less than or equal to: [100]. " + + "This limit can be set by changing the [node.max_search_context] node level setting.", + ex.getMessage()); + } + } + + public static class FailOnRewriteQueryPlugin extends Plugin implements SearchPlugin { @Override public List> getQueries() { @@ -409,7 +447,7 @@ public void testCanMatch() throws IOException { Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, - new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, + new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, From 044d013ad20e57393378b9f030f4191e90b487a0 Mon Sep 17 00:00:00 2001 From: Diwas Joshi Date: Mon, 2 Apr 2018 02:07:49 +0530 Subject: [PATCH 2/5] review fixes --- .../main/java/org/elasticsearch/search/SearchService.java | 5 ++--- .../java/org/elasticsearch/search/SearchServiceTests.java | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 6cb93f867ac28..f4d8864ce3ee9 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -163,7 +163,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private volatile TimeValue defaultSearchTimeout; private volatile boolean defaultAllowPartialSearchResults; - + private volatile boolean lowLevelCancellation; private final Cancellable keepAliveReaper; @@ -233,7 +233,7 @@ private void setDefaultAllowPartialSearchResults(boolean defaultAllowPartialSear public boolean defaultAllowPartialSearchResults() { return defaultAllowPartialSearchResults; } - + private void setLowLevelCancellation(Boolean lowLevelCancellation) { this.lowLevelCancellation = lowLevelCancellation; } @@ -568,7 +568,6 @@ final SearchContext createAndPutContext(ShardSearchRequest request) throws IOExc final SearchContext createContext(ShardSearchRequest request) throws IOException { final DefaultSearchContext context = createSearchContext(request, defaultSearchTimeout); - System.out.println("----activecontexts----" + activeContexts.size()); if (activeContexts.size() >= Node.MAX_SEARCH_CONTEXT_SETTING.get(settings)) { throw new IllegalStateException( "Trying to create too many search contexts. Must be less than or equal to: [" + diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index 7034a969cc69d..e5aebc3d03b79 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -447,7 +447,7 @@ public void testCanMatch() throws IOException { Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, - new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, + new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, From ec7eb5191bfbef809aef0668f9c4cb7af1b7484f Mon Sep 17 00:00:00 2001 From: Diwas Joshi Date: Tue, 10 Apr 2018 23:20:15 +0530 Subject: [PATCH 3/5] max_open_context moved to searchservice --- .../common/settings/ClusterSettings.java | 2 +- .../java/org/elasticsearch/node/Node.java | 8 ------- .../elasticsearch/search/SearchService.java | 23 ++++++++++++------ .../node/InternalSettingsPreparerTests.java | 7 ------ .../search/SearchServiceTests.java | 24 +++++++++---------- 5 files changed, 29 insertions(+), 35 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 402b11402bb4d..2b287a4ad9790 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -365,6 +365,7 @@ public void apply(Settings value, Settings current, Settings previous) { SearchService.MAX_KEEPALIVE_SETTING, MultiBucketConsumerService.MAX_BUCKET_SETTING, SearchService.LOW_LEVEL_CANCELLATION_SETTING, + SearchService.MAX_OPEN_CONTEXT, Node.WRITE_PORTS_FILE_SETTING, Node.NODE_NAME_SETTING, Node.NODE_DATA_SETTING, @@ -372,7 +373,6 @@ public void apply(Settings value, Settings current, Settings previous) { Node.NODE_INGEST_SETTING, Node.NODE_ATTRIBUTES, Node.NODE_LOCAL_STORAGE_SETTING, - Node.MAX_SEARCH_CONTEXT_SETTING, TransportMasterNodeReadAction.FORCE_LOCAL_SETTING, AutoCreateIndex.AUTO_CREATE_INDEX_SETTING, BaseRestHandler.MULTI_ALLOW_EXPLICIT_INDEX, diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index b6e5bfc1e447c..081c588525ed9 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -214,14 +214,6 @@ public class Node implements Closeable { } }, Setting.Property.NodeScope); - /** - * A setting describing the maximum number of search contexts that can be created. The default - * maximum of 100 is defensive to prevent generating too many search contexts. - */ - public static final Setting MAX_SEARCH_CONTEXT_SETTING = - Setting.intSetting("node.max_search_context", 100, 0, Property.NodeScope); - - /** * Adds a default node name to the given setting, if it doesn't already exist * @return the given setting if node name is already set, or a new copy with a default node name set. diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index f4d8864ce3ee9..45fd74df698b8 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -138,6 +138,14 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv Setting.boolSetting("search.default_allow_partial_results", true, Property.Dynamic, Property.NodeScope); + /** + * A setting describing the maximum number of search contexts that can be created. The default + * maximum of 100 is defensive to prevent generating too many search contexts. + */ + public static final Setting MAX_OPEN_CONTEXT = + Setting.intSetting("search.max_open_context", 100, 0, Property.NodeScope); + + private final ThreadPool threadPool; private final ClusterService clusterService; @@ -163,7 +171,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private volatile TimeValue defaultSearchTimeout; private volatile boolean defaultAllowPartialSearchResults; - + private volatile boolean lowLevelCancellation; private final Cancellable keepAliveReaper; @@ -549,6 +557,13 @@ private SearchContext findContext(long id, TransportRequest request) throws Sear } final SearchContext createAndPutContext(ShardSearchRequest request) throws IOException { + if (activeContexts.size() >= MAX_OPEN_CONTEXT.get(settings)) { + throw new ElasticsearchException( + "Trying to create too many search contexts. Must be less than or equal to: [" + + MAX_OPEN_CONTEXT.get(settings) + "]. This limit can be set by changing the [" + + MAX_OPEN_CONTEXT.getKey() + "] setting."); + } + SearchContext context = createContext(request); boolean success = false; try { @@ -568,12 +583,6 @@ final SearchContext createAndPutContext(ShardSearchRequest request) throws IOExc final SearchContext createContext(ShardSearchRequest request) throws IOException { final DefaultSearchContext context = createSearchContext(request, defaultSearchTimeout); - if (activeContexts.size() >= Node.MAX_SEARCH_CONTEXT_SETTING.get(settings)) { - throw new IllegalStateException( - "Trying to create too many search contexts. Must be less than or equal to: [" + - Node.MAX_SEARCH_CONTEXT_SETTING.get(settings) + "]. This limit can be set by changing the [" - + Node.MAX_SEARCH_CONTEXT_SETTING.getKey() + "] node level setting."); - } try { if (request.scroll() != null) { diff --git a/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java b/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java index 234dfd2ffd3c7..b8c3c158e4dd0 100644 --- a/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java +++ b/server/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java @@ -77,13 +77,6 @@ public void testEmptySettings() { assertTrue(configDir, configDir.startsWith(home)); } - public void testMaxSearchContexts() { - Settings settings = InternalSettingsPreparer.prepareSettings(Settings.EMPTY); - assertEquals(100, settings.get("node.max_search_context")); - settings = InternalSettingsPreparer.prepareSettings(Settings.builder().put("cluster.max_search_context", "20").build()); - assertEquals(20, settings.get("node.max_search_context")); - } - public void testDefaultClusterName() { Settings settings = InternalSettingsPreparer.prepareSettings(Settings.EMPTY); assertEquals("elasticsearch", settings.get("cluster.name")); diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index e5aebc3d03b79..b48c6e8cc1a99 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -22,6 +22,7 @@ import org.apache.lucene.search.Query; import org.apache.lucene.store.AlreadyClosedException; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.search.SearchPhaseExecutionException; @@ -348,14 +349,14 @@ searchSourceBuilder, new String[0], false, new AliasFilter(null, Strings.EMPTY_A /** * test that creating more than the allowed number of search contexts throws an exception */ - public void testMaxSearchContexts() throws IOException { + public void testMaxOpenContexts() throws IOException { createIndex("index"); final SearchService service = getInstanceFromNode(SearchService.class); final IndicesService indicesService = getInstanceFromNode(IndicesService.class); final IndexService indexService = indicesService.indexServiceSafe(resolveIndex("index")); final IndexShard indexShard = indexService.getShard(0); - for (int i = 0; i < Node.MAX_SEARCH_CONTEXT_SETTING.get(Settings.EMPTY); i++) { + for (int i = 0; i < service.MAX_OPEN_CONTEXT.get(Settings.EMPTY); i++) { SearchContext context = service.createAndPutContext( new ShardSearchLocalRequest( indexShard.shardId(), @@ -369,15 +370,14 @@ public void testMaxSearchContexts() throws IOException { ); } - try (SearchContext context = service.createAndPutContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, - new SearchSourceBuilder(), new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))) { - assertNotNull(context); - } catch (IllegalStateException ex) { - assertEquals( - "Trying to create too many search contexts. Must be less than or equal to: [100]. " + - "This limit can be set by changing the [node.max_search_context] node level setting.", - ex.getMessage()); - } + ElasticsearchException ex = expectThrows(ElasticsearchException.class, + () -> service.createAndPutContext(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.DEFAULT, + new SearchSourceBuilder(), new String[0], false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, true))); + assertEquals( + "Trying to create too many search contexts. Must be less than or equal to: [" + + service.MAX_OPEN_CONTEXT.get(Settings.EMPTY) + "]. " + + "This limit can be set by changing the [node.max_search_context] node level setting.", + ex.getMessage()); } @@ -447,7 +447,7 @@ public void testCanMatch() throws IOException { Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, - new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, + new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, From 9e84a7aaf0f462839d501900576757051d550e80 Mon Sep 17 00:00:00 2001 From: Diwas Joshi Date: Wed, 11 Apr 2018 17:15:37 +0530 Subject: [PATCH 4/5] counter for max open contexts --- .../main/java/org/elasticsearch/search/SearchService.java | 8 +++++--- .../java/org/elasticsearch/search/SearchServiceTests.java | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 45fd74df698b8..2a15b11ad86ce 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -55,8 +55,6 @@ import org.elasticsearch.index.shard.SearchOperationListener; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; -import org.elasticsearch.node.Node; -import org.elasticsearch.node.NodeService; import org.elasticsearch.node.ResponseCollectorService; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.SearchScript; @@ -180,6 +178,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private final ConcurrentMapLong activeContexts = ConcurrentCollections.newConcurrentMapLongWithAggressiveConcurrency(); + private int numActiveContexts = 0; + private final MultiBucketConsumerService multiBucketConsumerService; public SearchService(ClusterService clusterService, IndicesService indicesService, @@ -557,7 +557,7 @@ private SearchContext findContext(long id, TransportRequest request) throws Sear } final SearchContext createAndPutContext(ShardSearchRequest request) throws IOException { - if (activeContexts.size() >= MAX_OPEN_CONTEXT.get(settings)) { + if (numActiveContexts >= MAX_OPEN_CONTEXT.get(settings)) { throw new ElasticsearchException( "Trying to create too many search contexts. Must be less than or equal to: [" + MAX_OPEN_CONTEXT.get(settings) + "]. This limit can be set by changing the [" @@ -573,6 +573,7 @@ final SearchContext createAndPutContext(ShardSearchRequest request) throws IOExc } context.indexShard().getSearchOperationListener().onNewContext(context); success = true; + numActiveContexts++; return context; } finally { if (!success) { @@ -675,6 +676,7 @@ public boolean freeContext(long id) { } finally { context.close(); } + numActiveContexts--; return true; } return false; diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index b48c6e8cc1a99..2f8df1ab20b39 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -46,7 +46,6 @@ import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.indices.IndicesService; -import org.elasticsearch.node.Node; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.script.MockScriptEngine; @@ -376,7 +375,7 @@ public void testMaxOpenContexts() throws IOException { assertEquals( "Trying to create too many search contexts. Must be less than or equal to: [" + service.MAX_OPEN_CONTEXT.get(Settings.EMPTY) + "]. " + - "This limit can be set by changing the [node.max_search_context] node level setting.", + "This limit can be set by changing the [search.max_open_context] setting.", ex.getMessage()); } From f279cef2f018b597edd562ef208d2bb355d82270 Mon Sep 17 00:00:00 2001 From: Diwas Joshi Date: Mon, 16 Apr 2018 19:10:11 +0530 Subject: [PATCH 5/5] formatting fixes --- .../org/elasticsearch/search/SearchService.java | 14 +++++++------- .../elasticsearch/search/SearchServiceTests.java | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 2a15b11ad86ce..c84eea3437dc9 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -169,7 +169,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private volatile TimeValue defaultSearchTimeout; private volatile boolean defaultAllowPartialSearchResults; - + private volatile boolean lowLevelCancellation; private final Cancellable keepAliveReaper; @@ -208,10 +208,10 @@ public SearchService(ClusterService clusterService, IndicesService indicesServic clusterService.getClusterSettings().addSettingsUpdateConsumer(DEFAULT_SEARCH_TIMEOUT_SETTING, this::setDefaultSearchTimeout); defaultAllowPartialSearchResults = DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS.get(settings); - clusterService.getClusterSettings().addSettingsUpdateConsumer(DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS, + clusterService.getClusterSettings().addSettingsUpdateConsumer(DEFAULT_ALLOW_PARTIAL_SEARCH_RESULTS, this::setDefaultAllowPartialSearchResults); - - + + lowLevelCancellation = LOW_LEVEL_CANCELLATION_SETTING.get(settings); clusterService.getClusterSettings().addSettingsUpdateConsumer(LOW_LEVEL_CANCELLATION_SETTING, this::setLowLevelCancellation); } @@ -237,11 +237,11 @@ private void setDefaultSearchTimeout(TimeValue defaultSearchTimeout) { private void setDefaultAllowPartialSearchResults(boolean defaultAllowPartialSearchResults) { this.defaultAllowPartialSearchResults = defaultAllowPartialSearchResults; } - + public boolean defaultAllowPartialSearchResults() { return defaultAllowPartialSearchResults; - } - + } + private void setLowLevelCancellation(Boolean lowLevelCancellation) { this.lowLevelCancellation = lowLevelCancellation; } diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index 2f8df1ab20b39..2a48d0577628b 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -446,7 +446,7 @@ public void testCanMatch() throws IOException { Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH, - new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, + new SearchSourceBuilder(), Strings.EMPTY_ARRAY, false, new AliasFilter(null, Strings.EMPTY_ARRAY), 1f, allowPartialSearchResults))); assertTrue(service.canMatch(new ShardSearchLocalRequest(indexShard.shardId(), 1, SearchType.QUERY_THEN_FETCH,