Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Soft limit added for max number of search contexts #29252

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions server/src/main/java/org/elasticsearch/search/SearchService.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,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<Integer> MAX_OPEN_CONTEXT =
Setting.intSetting("search.max_open_context", 100, 0, Property.NodeScope);


private final ThreadPool threadPool;

private final ClusterService clusterService;
Expand Down Expand Up @@ -170,6 +178,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv

private final ConcurrentMapLong<SearchContext> activeContexts = ConcurrentCollections.newConcurrentMapLongWithAggressiveConcurrency();

private int numActiveContexts = 0;

private final MultiBucketConsumerService multiBucketConsumerService;

public SearchService(ClusterService clusterService, IndicesService indicesService,
Expand Down Expand Up @@ -547,6 +557,13 @@ private SearchContext findContext(long id, TransportRequest request) throws Sear
}

final SearchContext createAndPutContext(ShardSearchRequest request) throws IOException {
if (numActiveContexts >= MAX_OPEN_CONTEXT.get(settings)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this function be called by multiple threads at the same time? And if so, should numActiveContexts be volatile?

Copy link
Contributor

@jimczi jimczi Apr 16, 2018

Choose a reason for hiding this comment

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

Yes it can @mayya-sharipova but if we restrict the counting to scroll queries let's use an AtomicInteger. The overhead should be limited since we create the search context for a scroll query only on the initial request.

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 {
Expand All @@ -556,6 +573,7 @@ final SearchContext createAndPutContext(ShardSearchRequest request) throws IOExc
}
context.indexShard().getSearchOperationListener().onNewContext(context);
success = true;
numActiveContexts++;
return context;
} finally {
if (!success) {
Expand All @@ -566,6 +584,7 @@ final SearchContext createAndPutContext(ShardSearchRequest request) throws IOExc

final SearchContext createContext(ShardSearchRequest request) throws IOException {
final DefaultSearchContext context = createSearchContext(request, defaultSearchTimeout);

try {
if (request.scroll() != null) {
context.scrollContext(new ScrollContext());
Expand Down Expand Up @@ -657,6 +676,7 @@ public boolean freeContext(long id) {
} finally {
context.close();
}
numActiveContexts--;
return true;
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -343,6 +344,42 @@ 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 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 < service.MAX_OPEN_CONTEXT.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)
);
}

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 [search.max_open_context] setting.",
ex.getMessage());
}


public static class FailOnRewriteQueryPlugin extends Plugin implements SearchPlugin {
@Override
public List<QuerySpec<?>> getQueries() {
Expand Down