Skip to content

Commit

Permalink
Remove the query builder serialization from QueryShardException messa…
Browse files Browse the repository at this point in the history
…ge (#51885)

QueryBuilders that throw exceptions on shards when building the Lucene query
returns the full serialization of the query builder in the exception message.
For large queries that fails to execute due to the max boolean clause, this means
that we keep a reference of these big messages for every shard that participate
in the request. In order to limit the memory needed to hold these query shard
exceptions in the coordinating node, this change removes the query builder
serialization from the shard exception. The query is known by the user so
there should be no need to repeat it on every shard exception. We could also
omit the entire stack trace for known bad request exception but it would deserve
a separate issue/pr.

Closes #51843
Closes #48910
  • Loading branch information
jimczi committed Feb 6, 2020
1 parent 62b5b95 commit 1dcf1df
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,10 @@ private ParsedQuery toQuery(QueryBuilder queryBuilder, CheckedFunction<QueryBuil
try {
QueryBuilder rewriteQuery = Rewriteable.rewrite(queryBuilder, this, true);
return new ParsedQuery(filterOrQuery.apply(rewriteQuery), copyNamedQueries());
} catch(QueryShardException | ParsingException e ) {
} catch(QueryShardException | ParsingException e) {
throw e;
} catch(Exception e) {
throw new QueryShardException(this, "failed to create query: {}", e, queryBuilder);
throw new QueryShardException(this, "failed to create query: {}", e, e.getMessage());
} finally {
reset();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@
*/
package org.elasticsearch.index.query;

import org.apache.lucene.search.Query;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.plain.AbstractAtomicOrdinalsFieldData;
Expand Down Expand Up @@ -73,6 +76,43 @@ public void testFailIfFieldMappingNotFound() {
assertThat(result.name(), equalTo("name"));
}

public void testToQueryFails() {
QueryShardContext context = createQueryShardContext(IndexMetaData.INDEX_UUID_NA_VALUE, null);
Exception exc = expectThrows(Exception.class,
() -> context.toQuery(new AbstractQueryBuilder() {
@Override
public String getWriteableName() {
return null;
}

@Override
protected void doWriteTo(StreamOutput out) throws IOException {

}

@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {

}

@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
throw new RuntimeException("boom");
}

@Override
protected boolean doEquals(AbstractQueryBuilder other) {
return false;
}

@Override
protected int doHashCode() {
return 0;
}
}));
assertThat(exc.getMessage(), equalTo("failed to create query: boom"));
}

public void testClusterAlias() throws IOException {
final String clusterAlias = randomBoolean() ? null : "remote_cluster";
QueryShardContext context = createQueryShardContext(IndexMetaData.INDEX_UUID_NA_VALUE, clusterAlias);
Expand Down

0 comments on commit 1dcf1df

Please sign in to comment.