Explain api: move query parsing to the coordinating node
Similarly to what we did with the search api, we can now also move query parsing on the coordinating node for the explain api. Given that the explain api is a single shard operation (compared to search which is instead a broadcast operation), this doesn't change a lot in how the api works internally. The main benefit is that we can simplify the java api by requiring a structured query object to be provided rather than a bytes array that will get parsed on the data node. Previously if you specified a QueryBuilder it would be serialized in json format and would get reparsed on the data node, while now it doesn't go through parsing anymore (as expected), given that after the query-refactoring we are able to properly stream queries natively. Closes #14270
This commit is contained in:
parent
a56b108817
commit
ea750de39f
|
@ -21,13 +21,11 @@ package org.elasticsearch.action.explain;
|
|||
|
||||
import org.elasticsearch.action.ActionRequestValidationException;
|
||||
import org.elasticsearch.action.ValidateActions;
|
||||
import org.elasticsearch.action.support.QuerySourceBuilder;
|
||||
import org.elasticsearch.action.support.single.shard.SingleShardRequest;
|
||||
import org.elasticsearch.client.Requests;
|
||||
import org.elasticsearch.common.Strings;
|
||||
import org.elasticsearch.common.bytes.BytesReference;
|
||||
import org.elasticsearch.common.io.stream.StreamInput;
|
||||
import org.elasticsearch.common.io.stream.StreamOutput;
|
||||
import org.elasticsearch.index.query.QueryBuilder;
|
||||
import org.elasticsearch.search.fetch.source.FetchSourceContext;
|
||||
|
||||
import java.io.IOException;
|
||||
|
@ -41,7 +39,7 @@ public class ExplainRequest extends SingleShardRequest<ExplainRequest> {
|
|||
private String id;
|
||||
private String routing;
|
||||
private String preference;
|
||||
private BytesReference source;
|
||||
private QueryBuilder<?> query;
|
||||
private String[] fields;
|
||||
private FetchSourceContext fetchSourceContext;
|
||||
|
||||
|
@ -102,17 +100,12 @@ public class ExplainRequest extends SingleShardRequest<ExplainRequest> {
|
|||
return this;
|
||||
}
|
||||
|
||||
public BytesReference source() {
|
||||
return source;
|
||||
public QueryBuilder<?> query() {
|
||||
return query;
|
||||
}
|
||||
|
||||
public ExplainRequest source(QuerySourceBuilder sourceBuilder) {
|
||||
this.source = sourceBuilder.buildAsBytes(Requests.CONTENT_TYPE);
|
||||
return this;
|
||||
}
|
||||
|
||||
public ExplainRequest source(BytesReference source) {
|
||||
this.source = source;
|
||||
public ExplainRequest query(QueryBuilder<?> query) {
|
||||
this.query = query;
|
||||
return this;
|
||||
}
|
||||
|
||||
|
@ -159,8 +152,8 @@ public class ExplainRequest extends SingleShardRequest<ExplainRequest> {
|
|||
if (id == null) {
|
||||
validationException = ValidateActions.addValidationError("id is missing", validationException);
|
||||
}
|
||||
if (source == null) {
|
||||
validationException = ValidateActions.addValidationError("source is missing", validationException);
|
||||
if (query == null) {
|
||||
validationException = ValidateActions.addValidationError("query is missing", validationException);
|
||||
}
|
||||
return validationException;
|
||||
}
|
||||
|
@ -172,7 +165,7 @@ public class ExplainRequest extends SingleShardRequest<ExplainRequest> {
|
|||
id = in.readString();
|
||||
routing = in.readOptionalString();
|
||||
preference = in.readOptionalString();
|
||||
source = in.readBytesReference();
|
||||
query = in.readQuery();
|
||||
filteringAlias = in.readStringArray();
|
||||
if (in.readBoolean()) {
|
||||
fields = in.readStringArray();
|
||||
|
@ -189,7 +182,7 @@ public class ExplainRequest extends SingleShardRequest<ExplainRequest> {
|
|||
out.writeString(id);
|
||||
out.writeOptionalString(routing);
|
||||
out.writeOptionalString(preference);
|
||||
out.writeBytesReference(source);
|
||||
out.writeQuery(query);
|
||||
out.writeStringArray(filteringAlias);
|
||||
if (fields != null) {
|
||||
out.writeBoolean(true);
|
||||
|
|
|
@ -19,12 +19,10 @@
|
|||
|
||||
package org.elasticsearch.action.explain;
|
||||
|
||||
import org.elasticsearch.action.support.QuerySourceBuilder;
|
||||
import org.elasticsearch.action.support.single.shard.SingleShardOperationRequestBuilder;
|
||||
import org.elasticsearch.client.ElasticsearchClient;
|
||||
import org.elasticsearch.common.Nullable;
|
||||
import org.elasticsearch.common.Strings;
|
||||
import org.elasticsearch.common.bytes.BytesReference;
|
||||
import org.elasticsearch.index.query.QueryBuilder;
|
||||
import org.elasticsearch.search.fetch.source.FetchSourceContext;
|
||||
|
||||
|
@ -33,8 +31,6 @@ import org.elasticsearch.search.fetch.source.FetchSourceContext;
|
|||
*/
|
||||
public class ExplainRequestBuilder extends SingleShardOperationRequestBuilder<ExplainRequest, ExplainResponse, ExplainRequestBuilder> {
|
||||
|
||||
private QuerySourceBuilder sourceBuilder;
|
||||
|
||||
ExplainRequestBuilder(ElasticsearchClient client, ExplainAction action) {
|
||||
super(client, action, new ExplainRequest());
|
||||
}
|
||||
|
@ -87,15 +83,7 @@ public class ExplainRequestBuilder extends SingleShardOperationRequestBuilder<Ex
|
|||
* Sets the query to get a score explanation for.
|
||||
*/
|
||||
public ExplainRequestBuilder setQuery(QueryBuilder query) {
|
||||
sourceBuilder().setQuery(query);
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the query to get a score explanation for.
|
||||
*/
|
||||
public ExplainRequestBuilder setQuery(BytesReference query) {
|
||||
sourceBuilder().setQuery(query);
|
||||
request.query(query);
|
||||
return this;
|
||||
}
|
||||
|
||||
|
@ -151,28 +139,4 @@ public class ExplainRequestBuilder extends SingleShardOperationRequestBuilder<Ex
|
|||
}
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the full source of the explain request (for example, wrapping an actual query).
|
||||
*/
|
||||
public ExplainRequestBuilder setSource(BytesReference source) {
|
||||
request().source(source);
|
||||
return this;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected ExplainRequest beforeExecute(ExplainRequest request) {
|
||||
if (sourceBuilder != null) {
|
||||
request.source(sourceBuilder);
|
||||
}
|
||||
return request;
|
||||
}
|
||||
|
||||
private QuerySourceBuilder sourceBuilder() {
|
||||
if (sourceBuilder == null) {
|
||||
sourceBuilder = new QuerySourceBuilder();
|
||||
}
|
||||
return sourceBuilder;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -121,7 +121,7 @@ public class TransportExplainAction extends TransportSingleShardAction<ExplainRe
|
|||
SearchContext.setCurrent(context);
|
||||
|
||||
try {
|
||||
context.parsedQuery(indexService.queryParserService().parseTopLevelQuery(request.source()));
|
||||
context.parsedQuery(indexService.queryParserService().toQuery(request.query()));
|
||||
context.preProcess();
|
||||
int topLevelDocId = result.docIdAndVersion().docId + result.docIdAndVersion().context.docBase;
|
||||
Explanation explanation = context.searcher().explain(context.query(), topLevelDocId);
|
||||
|
|
|
@ -22,17 +22,16 @@ package org.elasticsearch.rest.action.explain;
|
|||
import org.apache.lucene.search.Explanation;
|
||||
import org.elasticsearch.action.explain.ExplainRequest;
|
||||
import org.elasticsearch.action.explain.ExplainResponse;
|
||||
import org.elasticsearch.action.support.QuerySourceBuilder;
|
||||
import org.elasticsearch.client.Client;
|
||||
import org.elasticsearch.common.Strings;
|
||||
import org.elasticsearch.common.bytes.BytesReference;
|
||||
import org.elasticsearch.common.inject.Inject;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilderString;
|
||||
import org.elasticsearch.index.get.GetResult;
|
||||
import org.elasticsearch.index.query.Operator;
|
||||
import org.elasticsearch.index.query.QueryBuilders;
|
||||
import org.elasticsearch.index.query.QueryStringQueryBuilder;
|
||||
import org.elasticsearch.index.query.QueryBuilder;
|
||||
import org.elasticsearch.indices.query.IndicesQueriesRegistry;
|
||||
import org.elasticsearch.rest.*;
|
||||
import org.elasticsearch.rest.action.support.RestActions;
|
||||
import org.elasticsearch.rest.action.support.RestBuilderListener;
|
||||
|
@ -50,9 +49,12 @@ import static org.elasticsearch.rest.RestStatus.OK;
|
|||
*/
|
||||
public class RestExplainAction extends BaseRestHandler {
|
||||
|
||||
private final IndicesQueriesRegistry indicesQueriesRegistry;
|
||||
|
||||
@Inject
|
||||
public RestExplainAction(Settings settings, RestController controller, Client client) {
|
||||
public RestExplainAction(Settings settings, RestController controller, Client client, IndicesQueriesRegistry indicesQueriesRegistry) {
|
||||
super(settings, controller, client);
|
||||
this.indicesQueriesRegistry = indicesQueriesRegistry;
|
||||
controller.registerHandler(GET, "/{index}/{type}/{id}/_explain", this);
|
||||
controller.registerHandler(POST, "/{index}/{type}/{id}/_explain", this);
|
||||
}
|
||||
|
@ -65,22 +67,11 @@ public class RestExplainAction extends BaseRestHandler {
|
|||
explainRequest.preference(request.param("preference"));
|
||||
String queryString = request.param("q");
|
||||
if (RestActions.hasBodyContent(request)) {
|
||||
explainRequest.source(RestActions.getRestContent(request));
|
||||
BytesReference restContent = RestActions.getRestContent(request);
|
||||
explainRequest.query(RestActions.getQueryContent(restContent, indicesQueriesRegistry, parseFieldMatcher));
|
||||
} else if (queryString != null) {
|
||||
QueryStringQueryBuilder queryStringBuilder = QueryBuilders.queryStringQuery(queryString);
|
||||
queryStringBuilder.defaultField(request.param("df"));
|
||||
queryStringBuilder.analyzer(request.param("analyzer"));
|
||||
queryStringBuilder.analyzeWildcard(request.paramAsBoolean("analyze_wildcard", false));
|
||||
queryStringBuilder.lowercaseExpandedTerms(request.paramAsBoolean("lowercase_expanded_terms", true));
|
||||
queryStringBuilder.lenient(request.paramAsBoolean("lenient", null));
|
||||
String defaultOperator = request.param("default_operator");
|
||||
if (defaultOperator != null) {
|
||||
queryStringBuilder.defaultOperator(Operator.fromString(defaultOperator));
|
||||
}
|
||||
|
||||
QuerySourceBuilder querySourceBuilder = new QuerySourceBuilder();
|
||||
querySourceBuilder.setQuery(queryStringBuilder);
|
||||
explainRequest.source(querySourceBuilder);
|
||||
QueryBuilder<?> query = RestActions.urlParamsToQueryBuilder(request);
|
||||
explainRequest.query(query);
|
||||
}
|
||||
|
||||
String sField = request.param("fields");
|
||||
|
|
|
@ -76,7 +76,6 @@ import org.elasticsearch.action.search.SearchResponse;
|
|||
import org.elasticsearch.action.search.SearchType;
|
||||
import org.elasticsearch.action.suggest.SuggestAction;
|
||||
import org.elasticsearch.action.suggest.SuggestRequest;
|
||||
import org.elasticsearch.action.support.QuerySourceBuilder;
|
||||
import org.elasticsearch.action.termvectors.MultiTermVectorsAction;
|
||||
import org.elasticsearch.action.termvectors.MultiTermVectorsRequest;
|
||||
import org.elasticsearch.action.termvectors.TermVectorsAction;
|
||||
|
@ -96,33 +95,16 @@ import org.elasticsearch.test.ESIntegTestCase;
|
|||
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
|
||||
import org.elasticsearch.test.ESIntegTestCase.Scope;
|
||||
import org.elasticsearch.threadpool.ThreadPool;
|
||||
import org.elasticsearch.transport.Transport;
|
||||
import org.elasticsearch.transport.TransportChannel;
|
||||
import org.elasticsearch.transport.TransportModule;
|
||||
import org.elasticsearch.transport.TransportRequest;
|
||||
import org.elasticsearch.transport.TransportRequestHandler;
|
||||
import org.elasticsearch.transport.TransportService;
|
||||
import org.elasticsearch.transport.*;
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
|
||||
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.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.*;
|
||||
import java.util.function.Supplier;
|
||||
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
|
||||
import static org.hamcrest.Matchers.emptyIterable;
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.hamcrest.Matchers.greaterThan;
|
||||
import static org.hamcrest.Matchers.hasItem;
|
||||
import static org.hamcrest.Matchers.instanceOf;
|
||||
import static org.hamcrest.Matchers.*;
|
||||
|
||||
@ClusterScope(scope = Scope.SUITE, numClientNodes = 1, minNumDataNodes = 2)
|
||||
public class IndicesRequestTests extends ESIntegTestCase {
|
||||
|
@ -307,7 +289,7 @@ public class IndicesRequestTests extends ESIntegTestCase {
|
|||
String explainShardAction = ExplainAction.NAME + "[s]";
|
||||
interceptTransportActions(explainShardAction);
|
||||
|
||||
ExplainRequest explainRequest = new ExplainRequest(randomIndexOrAlias(), "type", "id").source(new QuerySourceBuilder().setQuery(QueryBuilders.matchAllQuery()));
|
||||
ExplainRequest explainRequest = new ExplainRequest(randomIndexOrAlias(), "type", "id").query(QueryBuilders.matchAllQuery());
|
||||
internalCluster().clientNodeClient().explain(explainRequest).actionGet();
|
||||
|
||||
clearInterceptedActions();
|
||||
|
@ -684,24 +666,6 @@ public class IndicesRequestTests extends ESIntegTestCase {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static void assertSameIndicesOptionalRequests(String[] indices, String... actions) {
|
||||
assertSameIndices(indices, true, actions);
|
||||
}
|
||||
|
||||
private static void assertSameIndices(String[] indices, boolean optional, String... actions) {
|
||||
for (String action : actions) {
|
||||
List<TransportRequest> requests = consumeTransportRequests(action);
|
||||
if (!optional) {
|
||||
assertThat("no internal requests intercepted for action [" + action + "]", requests.size(), greaterThan(0));
|
||||
}
|
||||
for (TransportRequest internalRequest : requests) {
|
||||
assertThat(internalRequest, instanceOf(IndicesRequest.class));
|
||||
assertThat(internalRequest.getClass().getName(), ((IndicesRequest)internalRequest).indices(), equalTo(indices));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static void assertIndicesSubset(List<String> indices, String... actions) {
|
||||
//indices returned by each bulk shard request need to be a subset of the original indices
|
||||
for (String action : actions) {
|
||||
|
@ -820,26 +784,26 @@ public class IndicesRequestTests extends ESIntegTestCase {
|
|||
|
||||
@Override
|
||||
public <Request extends TransportRequest> void registerRequestHandler(String action, Supplier<Request> request, String executor, boolean forceExecution, TransportRequestHandler<Request> handler) {
|
||||
super.registerRequestHandler(action, request, executor, forceExecution, new InterceptingRequestHandler(action, handler));
|
||||
super.registerRequestHandler(action, request, executor, forceExecution, new InterceptingRequestHandler<>(action, handler));
|
||||
}
|
||||
|
||||
@Override
|
||||
public <Request extends TransportRequest> void registerRequestHandler(String action, Supplier<Request> requestFactory, String executor, TransportRequestHandler<Request> handler) {
|
||||
super.registerRequestHandler(action, requestFactory, executor, new InterceptingRequestHandler(action, handler));
|
||||
super.registerRequestHandler(action, requestFactory, executor, new InterceptingRequestHandler<>(action, handler));
|
||||
}
|
||||
|
||||
private class InterceptingRequestHandler implements TransportRequestHandler {
|
||||
private class InterceptingRequestHandler<T extends TransportRequest> implements TransportRequestHandler<T> {
|
||||
|
||||
private final TransportRequestHandler requestHandler;
|
||||
private final TransportRequestHandler<T> requestHandler;
|
||||
private final String action;
|
||||
|
||||
InterceptingRequestHandler(String action, TransportRequestHandler requestHandler) {
|
||||
InterceptingRequestHandler(String action, TransportRequestHandler<T> requestHandler) {
|
||||
this.requestHandler = requestHandler;
|
||||
this.action = action;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void messageReceived(TransportRequest request, TransportChannel channel) throws Exception {
|
||||
public void messageReceived(T request, TransportChannel channel) throws Exception {
|
||||
synchronized (InterceptingTransportService.this) {
|
||||
if (actions.contains(action)) {
|
||||
List<TransportRequest> requestList = requests.get(action);
|
||||
|
|
Loading…
Reference in New Issue