Skip to content

Commit

Permalink
Propagate index errors in field_caps (#70245) (#71328)
Browse files Browse the repository at this point in the history
Currently we don't report any exceptions occuring during field_caps requests back to the user.
This PR adds a new failure section to the response which contains exceptions per index.
In addition the response contains another field, `failed_indices`, with the number of indices that threw
an exception. If all of the requested indices fail, the whole request fails, otherwise the request succeeds
and it is up to the caller to check for potential errors in the response body.

Closes #68994
  • Loading branch information
Christoph Büscher committed Apr 6, 2021
1 parent 189fe33 commit 83f1bfe
Show file tree
Hide file tree
Showing 11 changed files with 632 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,27 @@ setup:
- match: {fields.day_of_week.keyword.type: keyword}
- match: {fields.day_of_week.keyword.searchable: true}
- match: {fields.day_of_week.keyword.aggregatable: true}

---
"Field caps with errors in runtime mappings section throws":

- skip:
version: " - 7.11.99"
reason: Runtime mappings support was added in 7.12

- do:
catch: bad_request
field_caps:
index: test-*
fields: "*"
body:
runtime_mappings:
day_of_week:
type: keyword
script:
source: "bad syntax"

- match: { error.type: "script_exception" }
- match: { error.reason: "compile error" }
- match: { error.script : "bad syntax" }
- match: { error.lang : "painless" }
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,13 @@
- is_false: fields.geo.keyword.on_aggregatable_indices

- do:
catch: missing
field_caps:
index: 'my_remote_cluster:some_index_that_doesnt_exist'
fields: [number]
- match: { 'fields': {} } # empty response - this index doesn't exists

- match: { error.type: "index_not_found_exception" }
- match: { error.reason: "no such index [some_index_that_doesnt_exist]" }

- do:
field_caps:
Expand All @@ -86,6 +89,23 @@
- match: {fields.number.keyword.aggregatable: true}
- match: {fields.number.keyword.type: keyword}

- do:
catch: bad_request
field_caps:
index: 'my_remote_cluster:field_caps_index_1'
fields: [number]
body:
runtime_mappings:
day_of_week:
type: keyword
script:
source: "bad syntax"

- match: { error.type: "script_exception" }
- match: { error.reason: "compile error" }
- match: { error.script : "bad syntax" }
- match: { error.lang : "painless" }

---
"Get field caps from remote cluster with index filter":
- skip:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.search.fieldcaps;

import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesFailure;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.search.fieldcaps.FieldCapabilitiesIT.ExceptionOnRewriteQueryBuilder;
import org.elasticsearch.search.fieldcaps.FieldCapabilitiesIT.ExceptionOnRewriteQueryPlugin;
import org.elasticsearch.test.AbstractMultiClustersTestCase;
import org.elasticsearch.transport.RemoteTransportException;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;

public class CCSFieldCapabilitiesIT extends AbstractMultiClustersTestCase {

@Override
protected Collection<String> remoteClusterAlias() {
return Arrays.asList("remote_cluster");
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins(String clusterAlias) {
final List<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins(clusterAlias));
plugins.add(ExceptionOnRewriteQueryPlugin.class);
return plugins;
}

public void testFailuresFromRemote() {
final Client localClient = client(LOCAL_CLUSTER);
final Client remoteClient = client("remote_cluster");
String localIndex = "local_test";
assertAcked(localClient.admin().indices().prepareCreate(localIndex));
localClient.prepareIndex(localIndex, "_doc").setId("1").setSource("foo", "bar").get();
localClient.admin().indices().prepareRefresh(localIndex).get();

String remoteErrorIndex = "remote_test_error";
assertAcked(remoteClient.admin().indices().prepareCreate(remoteErrorIndex));
remoteClient.prepareIndex(remoteErrorIndex, "_doc").setId("2").setSource("foo", "bar").get();
remoteClient.admin().indices().prepareRefresh(remoteErrorIndex).get();

// regular field_caps across clusters
FieldCapabilitiesResponse response = client().prepareFieldCaps("*", "remote_cluster:*").setFields("*").get();
assertThat(Arrays.asList(response.getIndices()), containsInAnyOrder(localIndex, "remote_cluster:" + remoteErrorIndex));

// adding an index filter so remote call should fail
response = client().prepareFieldCaps("*", "remote_cluster:*")
.setFields("*")
.setIndexFilter(new ExceptionOnRewriteQueryBuilder())
.get();
assertThat(response.getIndices()[0], equalTo(localIndex));
assertThat(response.getFailedIndices()[0], equalTo("remote_cluster:*"));
FieldCapabilitiesFailure failure = response.getFailures()
.stream()
.filter(f -> Arrays.asList(f.getIndices()).contains("remote_cluster:*"))
.findFirst().get();
Exception ex = failure.getException();
assertEquals(RemoteTransportException.class, ex.getClass());
Throwable cause = ExceptionsHelper.unwrapCause(ex);
assertEquals(IllegalArgumentException.class, cause.getClass());
assertEquals("I throw because I choose to.", cause.getMessage());

// if we only query the remote we should get back an exception only
ex = expectThrows(
IllegalArgumentException.class,
() -> client().prepareFieldCaps("remote_cluster:*")
.setFields("*")
.setIndexFilter(new ExceptionOnRewriteQueryBuilder())
.get());
assertEquals("I throw because I choose to.", ex.getMessage());

// add an index that doesn't fail to the remote
assertAcked(remoteClient.admin().indices().prepareCreate("okay_remote_index"));
remoteClient.prepareIndex("okay_remote_index", "_doc").setId("2").setSource("foo", "bar").get();
remoteClient.admin().indices().prepareRefresh("okay_remote_index").get();

response = client().prepareFieldCaps("*", "remote_cluster:*")
.setFields("*")
.setIndexFilter(new ExceptionOnRewriteQueryBuilder())
.get();
assertThat(Arrays.asList(response.getIndices()), containsInAnyOrder(localIndex, "remote_cluster:okay_remote_index"));
assertThat(response.getFailedIndices()[0], equalTo("remote_cluster:" + remoteErrorIndex));
failure = response.getFailures()
.stream()
.filter(f -> Arrays.asList(f.getIndices()).contains("remote_cluster:" + remoteErrorIndex))
.findFirst().get();
ex = failure.getException();
assertEquals(RemoteTransportException.class, ex.getClass());
cause = ExceptionsHelper.unwrapCause(ex);
assertEquals(IllegalArgumentException.class, cause.getClass());
assertEquals("I throw because I choose to.", cause.getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,49 @@

package org.elasticsearch.search.fieldcaps;

import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.Query;
import org.elasticsearch.action.fieldcaps.FieldCapabilities;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MetadataFieldMapper;
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.query.AbstractQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.QueryRewriteContext;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.plugins.MapperPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.SearchPlugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.transport.RemoteTransportException;
import org.junit.Before;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.function.Predicate;

import static java.util.Collections.singletonList;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
import static org.hamcrest.Matchers.containsInAnyOrder;

public class FieldCapabilitiesIT extends ESIntegTestCase {

@Override
@Before
public void setUp() throws Exception {
super.setUp();
Expand Down Expand Up @@ -92,7 +107,7 @@ public void setUp() throws Exception {

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.singleton(TestMapperPlugin.class);
return Arrays.asList(TestMapperPlugin.class, ExceptionOnRewriteQueryPlugin.class);
}

public void testFieldAlias() {
Expand Down Expand Up @@ -258,13 +273,121 @@ public void testMetadataFields() {
}
}

public void testWithRunntimeMappings() throws InterruptedException {
Map<String, Object> runtimeFields = new HashMap<>();
runtimeFields.put("day_of_week", Collections.singletonMap("type", "keyword"));
FieldCapabilitiesResponse response = client().prepareFieldCaps().setFields("*").setRuntimeFields(runtimeFields).get();
Map<String, FieldCapabilities> runtimeField = response.getField("day_of_week");
assertNotNull(runtimeField);
assertEquals("day_of_week", runtimeField.get("keyword").getName());
assertEquals("keyword", runtimeField.get("keyword").getType());
assertTrue(runtimeField.get("keyword").isSearchable());
assertTrue(runtimeField.get("keyword").isAggregatable());
}

public void testFailures() throws InterruptedException {
// in addition to the existing "old_index" and "new_index", create two where the test query throws an error on rewrite
assertAcked(prepareCreate("index1-error"));
assertAcked(prepareCreate("index2-error"));
ensureGreen("index1-error", "index2-error");
FieldCapabilitiesResponse response = client().prepareFieldCaps()
.setFields("*")
.setIndexFilter(new ExceptionOnRewriteQueryBuilder())
.get();
assertEquals(1, response.getFailures().size());
assertEquals(2, response.getFailedIndices().length);
assertThat(response.getFailures().get(0).getIndices(), arrayContainingInAnyOrder("index1-error", "index2-error"));
Exception failure = response.getFailures().get(0).getException();
assertEquals(RemoteTransportException.class, failure.getClass());
assertEquals(IllegalArgumentException.class, failure.getCause().getClass());
assertEquals("I throw because I choose to.", failure.getCause().getMessage());

// the "indices" section should not include failed ones
assertThat(Arrays.asList(response.getIndices()), containsInAnyOrder("old_index", "new_index"));

// if all requested indices failed, we fail the request by throwing the exception
IllegalArgumentException ex = expectThrows(
IllegalArgumentException.class,
() -> client().prepareFieldCaps("index1-error", "index2-error")
.setFields("*")
.setIndexFilter(new ExceptionOnRewriteQueryBuilder())
.get());
assertEquals("I throw because I choose to.", ex.getMessage());
}

private void assertIndices(FieldCapabilitiesResponse response, String... indices) {
assertNotNull(response.getIndices());
Arrays.sort(indices);
Arrays.sort(response.getIndices());
assertArrayEquals(indices, response.getIndices());
}

/**
* Adds an "exception" query that throws on rewrite if the index name contains the string "error"
*/
public static class ExceptionOnRewriteQueryPlugin extends Plugin implements SearchPlugin {

public ExceptionOnRewriteQueryPlugin() {}

@Override
public List<QuerySpec<?>> getQueries() {
return singletonList(
new QuerySpec<>("exception", ExceptionOnRewriteQueryBuilder::new, p -> new ExceptionOnRewriteQueryBuilder())
);
}
}

static class ExceptionOnRewriteQueryBuilder extends AbstractQueryBuilder<ExceptionOnRewriteQueryBuilder> {

public static final String NAME = "exception";

ExceptionOnRewriteQueryBuilder() {}

ExceptionOnRewriteQueryBuilder(StreamInput in) throws IOException {
super(in);
}

@Override
protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException {
SearchExecutionContext searchExecutionContext = queryRewriteContext.convertToSearchExecutionContext();
if (searchExecutionContext != null) {
if (searchExecutionContext.indexMatches("*error*")) {
throw new IllegalArgumentException("I throw because I choose to.");
};
}
return this;
}

@Override
protected void doWriteTo(StreamOutput out) {}

@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(NAME);
builder.endObject();
}

@Override
protected Query doToQuery(SearchExecutionContext context) {
return new MatchAllDocsQuery();
}

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

@Override
protected int doHashCode() {
return 0;
}

@Override
public String getWriteableName() {
return NAME;
}
}

public static final class TestMapperPlugin extends Plugin implements MapperPlugin {
@Override
public Map<String, MetadataFieldMapper.TypeParser> getMetadataMappers() {
Expand Down

0 comments on commit 83f1bfe

Please sign in to comment.