diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfig.java index c5a59a283105f..46d8c8a22ca31 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfig.java @@ -247,21 +247,20 @@ public void validateMappings(Map> fieldCa if (fieldCaps.get(dateType).isAggregatable()) { return; } else { - validationException.addValidationError("The field [" + field + "] must be aggregatable across all indices, " + + validationException.addValidationError("The field [" + field + "] must be aggregatable, " + "but is not."); } } } if (matchesDateType == false) { validationException.addValidationError("The field referenced by a date_histo group must be one of type [" + - Strings.collectionToCommaDelimitedString(RollupField.DATE_FIELD_MAPPER_TYPES) + "] across all " + - "indices in the index pattern. Found: " + fieldCaps.keySet().toString() + " for field [" + field + "]"); + Strings.collectionToCommaDelimitedString(RollupField.DATE_FIELD_MAPPER_TYPES) + "]." + + " Found: " + fieldCaps.keySet().toString() + " for field [" + field + "]"); } } else { validationException.addValidationError("Could not find one of [" + Strings.collectionToCommaDelimitedString(RollupField.DATE_FIELD_MAPPER_TYPES) + "] fields with name [" + - field + "] in any of the indices matching " + - "the index pattern."); + field + "]."); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionConfigTests.java index 17c7c7fcba7bc..10f7578b2d626 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionConfigTests.java @@ -6,17 +6,26 @@ */ package org.elasticsearch.xpack.core.rollup; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.fieldcaps.FieldCapabilities; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.test.AbstractSerializingTestCase; import org.elasticsearch.xpack.core.rollup.job.MetricConfig; +import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Random; import static java.util.Collections.emptyList; import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class RollupActionConfigTests extends AbstractSerializingTestCase { @@ -54,4 +63,49 @@ public void testEmptyMetrics() { new RollupActionConfig(groupConfig, randomBoolean() ? null : emptyList())); assertThat(e.getMessage(), equalTo("At least one metric must be configured")); } + + public void testValidateMapping() { + ActionRequestValidationException e = new ActionRequestValidationException(); + Map> responseMap = new HashMap<>(); + String type = getRandomType(); + + // Have to mock fieldcaps because the ctor's aren't public... + FieldCapabilities myFieldCaps = mock(FieldCapabilities.class); + when(myFieldCaps.isAggregatable()).thenReturn(true); + responseMap.put("my_field", Collections.singletonMap(type, myFieldCaps)); + responseMap.put("date_field", Collections.singletonMap("date", myFieldCaps)); + responseMap.put("group_field", Collections.singletonMap("keyword", myFieldCaps)); + responseMap.put("metric_field", Collections.singletonMap("short", myFieldCaps)); + + RollupActionConfig config = new RollupActionConfig(new RollupActionGroupConfig( + new RollupActionDateHistogramGroupConfig.FixedInterval("date_field", DateHistogramInterval.DAY), + null, new TermsGroupConfig("group_field")), + List.of(new MetricConfig("metric_field", List.of("max")))); + config.validateMappings(responseMap, e); + assertThat(e.validationErrors().size(), equalTo(0)); + } + + private String getRandomType() { + int n = randomIntBetween(0,8); + if (n == 0) { + return "keyword"; + } else if (n == 1) { + return "text"; + } else if (n == 2) { + return "long"; + } else if (n == 3) { + return "integer"; + } else if (n == 4) { + return "short"; + } else if (n == 5) { + return "float"; + } else if (n == 6) { + return "double"; + } else if (n == 7) { + return "scaled_float"; + } else if (n == 8) { + return "half_float"; + } + return "long"; + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfigSerializingTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfigSerializingTests.java index 1aecc52ff2a6a..b310a6de44ba1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfigSerializingTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfigSerializingTests.java @@ -12,7 +12,6 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.test.AbstractSerializingTestCase; -import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; import java.io.IOException; import java.util.Collections; @@ -46,11 +45,10 @@ public void testValidateNoMapping() { ActionRequestValidationException e = new ActionRequestValidationException(); Map> responseMap = new HashMap<>(); - DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field", - new DateHistogramInterval("1d"), null, null); + RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field", + new DateHistogramInterval("1d")); config.validateMappings(responseMap, e); - assertThat(e.validationErrors().get(0), equalTo("Could not find one of [date,date_nanos] fields with name [my_field] in " + - "any of the indices matching the index pattern.")); + assertThat(e.validationErrors().get(0), equalTo("Could not find one of [date,date_nanos] fields with name [my_field].")); } public void testValidateNomatchingField() { @@ -61,11 +59,10 @@ public void testValidateNomatchingField() { FieldCapabilities fieldCaps = mock(FieldCapabilities.class); responseMap.put("some_other_field", Collections.singletonMap("date", fieldCaps)); - DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field", - new DateHistogramInterval("1d"), null, null); + RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field", + new DateHistogramInterval("1d")); config.validateMappings(responseMap, e); - assertThat(e.validationErrors().get(0), equalTo("Could not find one of [date,date_nanos] fields with name [my_field] in " + - "any of the indices matching the index pattern.")); + assertThat(e.validationErrors().get(0), equalTo("Could not find one of [date,date_nanos] fields with name [my_field].")); } public void testValidateFieldWrongType() { @@ -76,11 +73,11 @@ public void testValidateFieldWrongType() { FieldCapabilities fieldCaps = mock(FieldCapabilities.class); responseMap.put("my_field", Collections.singletonMap("keyword", fieldCaps)); - DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field", - new DateHistogramInterval("1d"), null, null); + RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field", + new DateHistogramInterval("1d")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("The field referenced by a date_histo group must be one of type " + - "[date,date_nanos] across all indices in the index pattern. Found: [keyword] for field [my_field]")); + "[date,date_nanos]. Found: [keyword] for field [my_field]")); } public void testValidateFieldMixtureTypes() { @@ -94,11 +91,11 @@ public void testValidateFieldMixtureTypes() { types.put("keyword", fieldCaps); responseMap.put("my_field", types); - DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field", - new DateHistogramInterval("1d"), null, null); + RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field", + new DateHistogramInterval("1d")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().get(0), equalTo("The field referenced by a date_histo group must be one of type " + - "[date,date_nanos] across all indices in the index pattern. Found: [date, keyword] for field [my_field]")); + "[date,date_nanos]. Found: [date, keyword] for field [my_field]")); } public void testValidateFieldMatchingNotAggregatable() { @@ -110,10 +107,10 @@ public void testValidateFieldMatchingNotAggregatable() { when(fieldCaps.isAggregatable()).thenReturn(false); responseMap.put("my_field", Collections.singletonMap("date", fieldCaps)); - DateHistogramGroupConfig config =new DateHistogramGroupConfig.CalendarInterval("my_field", - new DateHistogramInterval("1d"), null, null); + RollupActionDateHistogramGroupConfig config =new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field", + new DateHistogramInterval("1d")); config.validateMappings(responseMap, e); - assertThat(e.validationErrors().get(0), equalTo("The field [my_field] must be aggregatable across all indices, but is not.")); + assertThat(e.validationErrors().get(0), equalTo("The field [my_field] must be aggregatable, but is not.")); } public void testValidateMatchingField() { @@ -125,8 +122,8 @@ public void testValidateMatchingField() { when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("date", fieldCaps)); - DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field", - new DateHistogramInterval("1d"), null, null); + RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field", + new DateHistogramInterval("1d")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); } @@ -140,8 +137,8 @@ public void testValidateWeek() { when(fieldCaps.isAggregatable()).thenReturn(true); responseMap.put("my_field", Collections.singletonMap("date", fieldCaps)); - DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field", - new DateHistogramInterval("1w"), null, null); + RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field", + new DateHistogramInterval("1w")); config.validateMappings(responseMap, e); assertThat(e.validationErrors().size(), equalTo(0)); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionGroupConfigSerializingTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionGroupConfigSerializingTests.java index bbdc19355a491..e69d064b75488 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionGroupConfigSerializingTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionGroupConfigSerializingTests.java @@ -6,13 +6,24 @@ */ package org.elasticsearch.xpack.core.rollup; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.fieldcaps.FieldCapabilities; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.test.AbstractSerializingTestCase; +import org.elasticsearch.xpack.core.rollup.job.HistogramGroupConfig; +import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomRollupActionGroupConfig; +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class RollupActionGroupConfigSerializingTests extends AbstractSerializingTestCase { @@ -30,4 +41,41 @@ protected Writeable.Reader instanceReader() { protected RollupActionGroupConfig createTestInstance() { return randomRollupActionGroupConfig(random()); } + + public void testValidatesDateHistogramConfig() { + ActionRequestValidationException e = new ActionRequestValidationException(); + Map> responseMap = new HashMap<>(); + // Have to mock fieldcaps because the ctor's aren't public... + FieldCapabilities fieldCaps = mock(FieldCapabilities.class); + when(fieldCaps.isAggregatable()).thenReturn(true); + responseMap.put("date_field", Collections.singletonMap("not_date", fieldCaps)); + RollupActionGroupConfig config = new RollupActionGroupConfig( + new RollupActionDateHistogramGroupConfig.FixedInterval("date_field", DateHistogramInterval.DAY)); + config.validateMappings(responseMap, e); + assertThat(e.validationErrors().size(), equalTo(1)); + } + + public void testValidatesAllSubConfigs() { + ActionRequestValidationException e = new ActionRequestValidationException(); + Map> responseMap = new HashMap<>(); + // Have to mock fieldcaps because the ctor's aren't public... + FieldCapabilities fieldCaps = mock(FieldCapabilities.class); + when(fieldCaps.isAggregatable()).thenReturn(false); + responseMap.put("date_field", Collections.singletonMap("date", fieldCaps)); + responseMap.put("terms_field", Collections.singletonMap("keyword", fieldCaps)); + responseMap.put("histogram_field", Collections.singletonMap("keyword", fieldCaps)); + RollupActionGroupConfig config = new RollupActionGroupConfig( + new RollupActionDateHistogramGroupConfig.FixedInterval("date_field", DateHistogramInterval.DAY), + new HistogramGroupConfig(132, "histogram_field"), new TermsGroupConfig("terms_field")); + config.validateMappings(responseMap, e); + // all fields are non-aggregatable + assertThat(e.validationErrors().size(), equalTo(3)); + assertThat(e.validationErrors().get(0), + equalTo("The field [date_field] must be aggregatable, but is not.")); + assertThat(e.validationErrors().get(1), + equalTo("The field referenced by a histo group must be a [numeric] type, " + + "but found [keyword] for field [histogram_field]")); + assertThat(e.validationErrors().get(2), + equalTo("The field [terms_field] must be aggregatable across all indices, but is not.")); + } } diff --git a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java index 11d17584c6934..ae61e8fdef9fc 100644 --- a/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java +++ b/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java @@ -13,6 +13,7 @@ import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeRequest; import org.elasticsearch.action.admin.indices.shrink.ResizeType; +import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction; @@ -46,6 +47,7 @@ import org.elasticsearch.xpack.core.ClientHelper; import org.elasticsearch.xpack.core.rollup.RollupActionConfig; import org.elasticsearch.xpack.core.rollup.RollupActionGroupConfig; +import org.elasticsearch.xpack.core.rollup.action.RollupActionRequestValidationException; import org.elasticsearch.xpack.core.rollup.action.RollupIndexerAction; import org.elasticsearch.xpack.core.rollup.job.HistogramGroupConfig; import org.elasticsearch.xpack.core.rollup.job.MetricConfig; @@ -104,6 +106,10 @@ protected void masterOperation(Task task, RollupAction.Request request, ClusterS return; } + FieldCapabilitiesRequest fieldCapsRequest = new FieldCapabilitiesRequest() + .indices(originalIndexName) + .fields(request.getRollupConfig().getAllFields().toArray(new String[0])); + fieldCapsRequest.setParentTask(clusterService.localNode().getId(), task.getId()); CreateIndexRequest req = new CreateIndexRequest(tmpIndexName, Settings.builder() .put(IndexMetadata.SETTING_INDEX_HIDDEN, true).build()) .mapping(mapping); @@ -115,35 +121,52 @@ protected void masterOperation(Task task, RollupAction.Request request, ClusterS UpdateSettingsRequest updateSettingsReq = new UpdateSettingsRequest( Settings.builder().put(IndexMetadata.SETTING_BLOCKS_WRITE, true).build(), tmpIndexName); - // 1. create hidden temporary index - // 2. run rollup indexer - // 3. make temp index read-only - // 4. shrink index - // 5. delete temporary index + // 1. validate Rollup Config against Field Caps + // 2. create hidden temporary index + // 3. run rollup indexer + // 4. make temp index read-only + // 5. shrink index + // 6. delete temporary index // at any point if there is an issue, then cleanup temp index - client.admin().indices().create(req, ActionListener.wrap(createIndexResponse -> - client.execute(RollupIndexerAction.INSTANCE, rollupIndexerRequest, ActionListener.wrap(indexerResp -> { - if (indexerResp.isCreated()) { - client.admin().indices().updateSettings(updateSettingsReq, ActionListener.wrap(updateSettingsResponse -> { - if (updateSettingsResponse.isAcknowledged()) { - client.admin().indices().resizeIndex(resizeRequest, ActionListener.wrap(resizeResponse -> { - if (resizeResponse.isAcknowledged()) { - publishMetadata(request.getRollupConfig(), originalIndexName, tmpIndexName, rollupIndexName, listener); - } else { - deleteTmpIndex(originalIndexName, tmpIndexName, listener, - new ElasticsearchException("Unable to resize temp rollup index [" + tmpIndexName + "]")); - } - }, e -> deleteTmpIndex(originalIndexName, tmpIndexName, listener, e))); - } else { - deleteTmpIndex(originalIndexName, tmpIndexName, listener, - new ElasticsearchException("Unable to update settings of temp rollup index [" + tmpIndexName + "]")); - } - }, e -> deleteTmpIndex(originalIndexName, tmpIndexName, listener, e))); - } else { - deleteTmpIndex(originalIndexName, tmpIndexName, listener, - new ElasticsearchException("Unable to index into temp rollup index [" + tmpIndexName + "]")); - } - }, e -> deleteTmpIndex(originalIndexName, tmpIndexName, listener, e))), listener::onFailure)); + + client.fieldCaps(fieldCapsRequest, ActionListener.wrap(fieldCapsResponse -> { + RollupActionRequestValidationException validationException = new RollupActionRequestValidationException(); + if (fieldCapsResponse.get().size() == 0) { + validationException.addValidationError("Could not find any fields in the index [" + + originalIndexName + "] that were configured in job"); + listener.onFailure(validationException); + return; + } + request.getRollupConfig().validateMappings(fieldCapsResponse.get(), validationException); + if (validationException.validationErrors().size() > 0) { + listener.onFailure(validationException); + return; + } + client.admin().indices().create(req, ActionListener.wrap(createIndexResponse -> + client.execute(RollupIndexerAction.INSTANCE, rollupIndexerRequest, ActionListener.wrap(indexerResp -> { + if (indexerResp.isCreated()) { + client.admin().indices().updateSettings(updateSettingsReq, ActionListener.wrap(updateSettingsResponse -> { + if (updateSettingsResponse.isAcknowledged()) { + client.admin().indices().resizeIndex(resizeRequest, ActionListener.wrap(resizeResponse -> { + if (resizeResponse.isAcknowledged()) { + publishMetadata(request.getRollupConfig(), originalIndexName, tmpIndexName, + rollupIndexName, listener); + } else { + deleteTmpIndex(originalIndexName, tmpIndexName, listener, + new ElasticsearchException("Unable to resize temp rollup index [" + tmpIndexName + "]")); + } + }, e -> deleteTmpIndex(originalIndexName, tmpIndexName, listener, e))); + } else { + deleteTmpIndex(originalIndexName, tmpIndexName, listener, + new ElasticsearchException("Unable to update settings of temp rollup index [" + tmpIndexName + "]")); + } + }, e -> deleteTmpIndex(originalIndexName, tmpIndexName, listener, e))); + } else { + deleteTmpIndex(originalIndexName, tmpIndexName, listener, + new ElasticsearchException("Unable to index into temp rollup index [" + tmpIndexName + "]")); + } + }, e -> deleteTmpIndex(originalIndexName, tmpIndexName, listener, e))), listener::onFailure)); + }, listener::onFailure)); } @Override diff --git a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/v2/RollupActionSingleNodeTests.java b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/v2/RollupActionSingleNodeTests.java index 58b28c42d7ae0..be61ddccfca52 100644 --- a/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/v2/RollupActionSingleNodeTests.java +++ b/x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/v2/RollupActionSingleNodeTests.java @@ -92,6 +92,7 @@ public void setup() { "date_1", "type=date", "numeric_1", "type=double", "numeric_2", "type=float", + "numeric_nonaggregatable", "type=double,doc_values=false", "categorical_1", "type=keyword").get(); } @@ -259,6 +260,21 @@ public void testAvgMetric() throws IOException { assertRollupIndex(config); } + public void testValidationCheck() throws IOException { + RollupActionDateHistogramGroupConfig dateHistogramGroupConfig = randomRollupActionDateHistogramGroupConfig("date_1"); + SourceSupplier sourceSupplier = () -> XContentFactory.jsonBuilder().startObject() + .field("date_1", randomDateForInterval(dateHistogramGroupConfig.getInterval())) + // use integers to ensure that avg is comparable between rollup and original + .field("numeric_nonaggregatable", randomInt()) + .endObject(); + RollupActionConfig config = new RollupActionConfig( + new RollupActionGroupConfig(dateHistogramGroupConfig, null, null), + Collections.singletonList(new MetricConfig("numeric_nonaggregatable", Collections.singletonList("avg")))); + bulkIndex(sourceSupplier); + Exception e = expectThrows(Exception.class, () -> rollup(config)); + assertThat(e.getMessage(), containsString("The field [numeric_nonaggregatable] must be aggregatable")); + } + private RollupActionDateHistogramGroupConfig randomRollupActionDateHistogramGroupConfig(String field) { RollupActionDateHistogramGroupConfig randomConfig = ConfigTestHelpers.randomRollupActionDateHistogramGroupConfig(random()); if (randomConfig instanceof RollupActionDateHistogramGroupConfig.FixedInterval) {