From 04a138db5ddf3f28ffd0d7f81054b05c3c6aa02d Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Thu, 12 Jan 2012 14:19:21 +0200 Subject: [PATCH] Allow to provide timeout parameter in request body (as well as URI parameter), closes #1604. --- .../action/search/SearchRequest.java | 34 --------- .../search/type/TransportSearchHelper.java | 1 - .../action/search/SearchRequestBuilder.java | 4 +- .../rest/action/search/RestSearchAction.java | 7 +- .../search/builder/SearchSourceBuilder.java | 23 ++++++ .../search/internal/ContextIndexSearcher.java | 7 +- .../internal/InternalSearchRequest.java | 22 ------ .../search/internal/SearchContext.java | 12 +++- .../search/query/QueryPhase.java | 2 + .../search/query/TimeoutParseElement.java | 41 +++++++++++ .../search/timeout/SearchTimeoutTests.java | 71 +++++++++++++++++++ 11 files changed, 157 insertions(+), 67 deletions(-) create mode 100644 src/main/java/org/elasticsearch/search/query/TimeoutParseElement.java create mode 100644 src/test/java/org/elasticsearch/test/integration/search/timeout/SearchTimeoutTests.java diff --git a/src/main/java/org/elasticsearch/action/search/SearchRequest.java b/src/main/java/org/elasticsearch/action/search/SearchRequest.java index 5dc611a78e6..2727b8df6bc 100644 --- a/src/main/java/org/elasticsearch/action/search/SearchRequest.java +++ b/src/main/java/org/elasticsearch/action/search/SearchRequest.java @@ -44,7 +44,6 @@ import java.util.Arrays; import java.util.Map; import static org.elasticsearch.action.Actions.addValidationError; -import static org.elasticsearch.common.unit.TimeValue.readTimeValue; import static org.elasticsearch.search.Scroll.readScroll; /** @@ -89,8 +88,6 @@ public class SearchRequest implements ActionRequest { private String[] types = Strings.EMPTY_ARRAY; - private TimeValue timeout; - private boolean listenerThreaded = false; private SearchOperationThreading operationThreading = SearchOperationThreading.THREAD_PER_SHARD; @@ -501,28 +498,6 @@ public class SearchRequest implements ActionRequest { return scroll(new Scroll(TimeValue.parseTimeValue(keepAlive, null))); } - /** - * An optional timeout to control how long search is allowed to take. - */ - public TimeValue timeout() { - return timeout; - } - - /** - * An optional timeout to control how long search is allowed to take. - */ - public SearchRequest timeout(TimeValue timeout) { - this.timeout = timeout; - return this; - } - - /** - * An optional timeout to control how long search is allowed to take. - */ - public SearchRequest timeout(String timeout) { - return timeout(TimeValue.parseTimeValue(timeout, null)); - } - @Override public void readFrom(StreamInput in) throws IOException { operationThreading = SearchOperationThreading.fromId(in.readByte()); @@ -546,9 +521,6 @@ public class SearchRequest implements ActionRequest { if (in.readBoolean()) { scroll = readScroll(in); } - if (in.readBoolean()) { - timeout = readTimeValue(in); - } BytesHolder bytes = in.readBytesReference(); sourceUnsafe = false; @@ -606,12 +578,6 @@ public class SearchRequest implements ActionRequest { out.writeBoolean(true); scroll.writeTo(out); } - if (timeout == null) { - out.writeBoolean(false); - } else { - out.writeBoolean(true); - timeout.writeTo(out); - } out.writeBytesHolder(source, sourceOffset, sourceLength); out.writeBytesHolder(extraSource, extraSourceOffset, extraSourceLength); out.writeVInt(types.length); diff --git a/src/main/java/org/elasticsearch/action/search/type/TransportSearchHelper.java b/src/main/java/org/elasticsearch/action/search/type/TransportSearchHelper.java index 3cd71fd8231..c7425f1942b 100644 --- a/src/main/java/org/elasticsearch/action/search/type/TransportSearchHelper.java +++ b/src/main/java/org/elasticsearch/action/search/type/TransportSearchHelper.java @@ -66,7 +66,6 @@ public abstract class TransportSearchHelper { internalRequest.source(request.source(), request.sourceOffset(), request.sourceLength()); internalRequest.extraSource(request.extraSource(), request.extraSourceOffset(), request.extraSourceLength()); internalRequest.scroll(request.scroll()); - internalRequest.timeout(request.timeout()); internalRequest.filteringAliases(filteringAliases); internalRequest.types(request.types()); internalRequest.nowInMillis(nowInMillis); diff --git a/src/main/java/org/elasticsearch/client/action/search/SearchRequestBuilder.java b/src/main/java/org/elasticsearch/client/action/search/SearchRequestBuilder.java index ca85f603bb6..6428625162c 100644 --- a/src/main/java/org/elasticsearch/client/action/search/SearchRequestBuilder.java +++ b/src/main/java/org/elasticsearch/client/action/search/SearchRequestBuilder.java @@ -115,7 +115,7 @@ public class SearchRequestBuilder extends BaseRequestBuilder fieldNames; private List scriptFields; private List partialFields; @@ -276,6 +279,22 @@ public class SearchSourceBuilder implements ToXContent { return this; } + /** + * An optional timeout to control how long search is allowed to take. + */ + public SearchSourceBuilder timeout(TimeValue timeout) { + this.timeoutInMillis = timeout.millis(); + return this; + } + + /** + * An optional timeout to control how long search is allowed to take. + */ + public SearchSourceBuilder timeout(String timeout) { + this.timeoutInMillis = TimeValue.parseTimeValue(timeout, null).millis(); + return this; + } + /** * Adds a sort against the given field name and the sort ordering. * @@ -564,6 +583,10 @@ public class SearchSourceBuilder implements ToXContent { builder.field("size", size); } + if (timeoutInMillis != -1) { + builder.field("timeout", timeoutInMillis); + } + if (queryBuilder != null) { builder.field("query"); queryBuilder.toXContent(builder, params); diff --git a/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java b/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java index 7f19148d2cb..91e6bc4a858 100644 --- a/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java +++ b/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java @@ -158,9 +158,9 @@ public class ContextIndexSearcher extends ExtendedIndexSearcher { // since that is where the filter should only work collector = new FilteredCollector(collector, searchContext.parsedFilter()); } - if (searchContext.timeout() != null) { + if (searchContext.timeoutInMillis() != -1) { // TODO: change to use our own counter that uses the scheduler in ThreadPool - collector = new TimeLimitingCollector(collector, TimeLimitingCollector.getGlobalCounter(), searchContext.timeout().millis()); + collector = new TimeLimitingCollector(collector, TimeLimitingCollector.getGlobalCounter(), searchContext.timeoutInMillis()); } if (scopeCollectors != null) { List collectors = scopeCollectors.get(processingScope); @@ -185,8 +185,7 @@ public class ContextIndexSearcher extends ExtendedIndexSearcher { } // we only compute the doc id set once since within a context, we execute the same query always... - if (searchContext.timeout() != null) { - searchContext.queryResult().searchTimedOut(false); + if (searchContext.timeoutInMillis() != -1) { try { super.search(weight, combinedFilter, collector); } catch (TimeLimitingCollector.TimeExceededException e) { diff --git a/src/main/java/org/elasticsearch/search/internal/InternalSearchRequest.java b/src/main/java/org/elasticsearch/search/internal/InternalSearchRequest.java index 66aef768097..bb2b5ba22c9 100644 --- a/src/main/java/org/elasticsearch/search/internal/InternalSearchRequest.java +++ b/src/main/java/org/elasticsearch/search/internal/InternalSearchRequest.java @@ -26,12 +26,10 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Streamable; -import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.search.Scroll; import java.io.IOException; -import static org.elasticsearch.common.unit.TimeValue.readTimeValue; import static org.elasticsearch.search.Scroll.readScroll; /** @@ -63,8 +61,6 @@ public class InternalSearchRequest implements Streamable { private Scroll scroll; - private TimeValue timeout; - private String[] types = Strings.EMPTY_ARRAY; private String[] filteringAliases; @@ -169,15 +165,6 @@ public class InternalSearchRequest implements Streamable { return this; } - public TimeValue timeout() { - return timeout; - } - - public InternalSearchRequest timeout(TimeValue timeout) { - this.timeout = timeout; - return this; - } - public String[] filteringAliases() { return filteringAliases; } @@ -203,9 +190,6 @@ public class InternalSearchRequest implements Streamable { if (in.readBoolean()) { scroll = readScroll(in); } - if (in.readBoolean()) { - timeout = readTimeValue(in); - } BytesHolder bytes = in.readBytesReference(); source = bytes.bytes(); @@ -248,12 +232,6 @@ public class InternalSearchRequest implements Streamable { out.writeBoolean(true); scroll.writeTo(out); } - if (timeout == null) { - out.writeBoolean(false); - } else { - out.writeBoolean(true); - timeout.writeTo(out); - } out.writeBytesHolder(source, sourceOffset, sourceLength); out.writeBytesHolder(extraSource, extraSourceOffset, extraSourceLength); out.writeVInt(types.length); diff --git a/src/main/java/org/elasticsearch/search/internal/SearchContext.java b/src/main/java/org/elasticsearch/search/internal/SearchContext.java index 68e644542d0..ba563034294 100644 --- a/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -28,7 +28,6 @@ import org.elasticsearch.ElasticSearchException; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.lease.Releasable; -import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.analysis.AnalysisService; import org.elasticsearch.index.cache.field.data.FieldDataCache; import org.elasticsearch.index.cache.filter.FilterCache; @@ -112,6 +111,9 @@ public class SearchContext implements Releasable { private float queryBoost = 1.0f; + // timeout in millis + private long timeoutInMillis = -1; + private List groupStats; @@ -337,8 +339,12 @@ public class SearchContext implements Releasable { return indexService.cache().idCache(); } - public TimeValue timeout() { - return request.timeout(); + public long timeoutInMillis() { + return timeoutInMillis; + } + + public void timeoutInMillis(long timeoutInMillis) { + this.timeoutInMillis = timeoutInMillis; } public SearchContext minimumScore(float minimumScore) { diff --git a/src/main/java/org/elasticsearch/search/query/QueryPhase.java b/src/main/java/org/elasticsearch/search/query/QueryPhase.java index 31021f33f03..87bb304849e 100644 --- a/src/main/java/org/elasticsearch/search/query/QueryPhase.java +++ b/src/main/java/org/elasticsearch/search/query/QueryPhase.java @@ -68,6 +68,7 @@ public class QueryPhase implements SearchPhase { .put("track_scores", new TrackScoresParseElement()) .put("min_score", new MinScoreParseElement()) .put("minScore", new MinScoreParseElement()) + .put("timeout", new TimeoutParseElement()) .putAll(facetPhase.parseElements()); return parseElements.build(); } @@ -94,6 +95,7 @@ public class QueryPhase implements SearchPhase { } public void execute(SearchContext searchContext) throws QueryPhaseExecutionException { + searchContext.queryResult().searchTimedOut(false); // set the filter on the searcher if (searchContext.scopePhases() != null) { // we have scoped queries, refresh the id cache diff --git a/src/main/java/org/elasticsearch/search/query/TimeoutParseElement.java b/src/main/java/org/elasticsearch/search/query/TimeoutParseElement.java new file mode 100644 index 00000000000..5c9acb567c2 --- /dev/null +++ b/src/main/java/org/elasticsearch/search/query/TimeoutParseElement.java @@ -0,0 +1,41 @@ +/* + * Licensed to ElasticSearch and Shay Banon 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.unit.TimeValue; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.search.SearchParseElement; +import org.elasticsearch.search.internal.SearchContext; + +/** + */ +public class TimeoutParseElement implements SearchParseElement { + + @Override + public void parse(XContentParser parser, SearchContext context) throws Exception { + XContentParser.Token token = parser.currentToken(); + if (token == XContentParser.Token.VALUE_NUMBER) { + context.timeoutInMillis(parser.longValue()); + } else { + context.timeoutInMillis(TimeValue.parseTimeValue(parser.text(), null).millis()); + } + } +} + diff --git a/src/test/java/org/elasticsearch/test/integration/search/timeout/SearchTimeoutTests.java b/src/test/java/org/elasticsearch/test/integration/search/timeout/SearchTimeoutTests.java new file mode 100644 index 00000000000..66d58beb386 --- /dev/null +++ b/src/test/java/org/elasticsearch/test/integration/search/timeout/SearchTimeoutTests.java @@ -0,0 +1,71 @@ +/* + * Licensed to ElasticSearch and Shay Banon 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.test.integration.search.timeout; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.client.Client; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.integration.AbstractNodesTests; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; +import static org.elasticsearch.index.query.FilterBuilders.scriptFilter; +import static org.elasticsearch.index.query.QueryBuilders.filteredQuery; +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +/** + */ +public class SearchTimeoutTests extends AbstractNodesTests { + + private Client client; + + @BeforeClass + public void createNodes() throws Exception { + Settings settings = settingsBuilder().put("index.number_of_shards", 2).put("index.number_of_replicas", 0).build(); + startNode("node1", settings); + client = client("node1"); + } + + @AfterClass + public void closeNodes() { + client.close(); + closeAllNodes(); + } + + @Test + public void simpleTimeoutTest() throws Exception { + client.admin().indices().prepareDelete().execute().actionGet(); + + for (int i = 0; i < 10; i++) { + client.prepareIndex("test", "type", Integer.toString(i)).setSource("field", "value").execute().actionGet(); + } + client.admin().indices().prepareRefresh().execute().actionGet(); + + SearchResponse searchResponse = client.prepareSearch("test") + .setTimeout("10ms") + .setQuery(filteredQuery(matchAllQuery(), scriptFilter("Thread.sleep(100); return true;"))) + .execute().actionGet(); + assertThat(searchResponse.timedOut(), equalTo(true)); + } +}