From 4f176032264dcecb85d39eb447e1b928a145a211 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Wed, 12 Feb 2020 16:44:11 -0500 Subject: [PATCH 1/5] [ML] rewrite deprecated datafeed aggs to use new [fixed|calendar]interval --- .../xpack/core/ml/datafeed/AggProvider.java | 37 +++++++ .../core/ml/datafeed/AggProviderTests.java | 77 ++++++++++++++ .../core/ml/datafeed/DatafeedConfigTests.java | 100 +++++++++++------- .../core/ml/datafeed/DatafeedUpdateTests.java | 35 +++--- .../ml/integration/DatafeedWithAggsIT.java | 4 +- .../action/TransportUpdateDatafeedAction.java | 2 - .../mixed_cluster/40_ml_datafeed_crud.yml | 20 ++-- .../test/old_cluster/40_ml_datafeed_crud.yml | 11 +- .../upgraded_cluster/40_ml_datafeed_crud.yml | 16 +-- 9 files changed, 207 insertions(+), 95 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/AggProvider.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/AggProvider.java index 1c39c6d985d45..7fd5b5cb51a61 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/AggProvider.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/AggProvider.java @@ -15,6 +15,8 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.aggregations.AggregatorFactories; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.xpack.core.ml.job.messages.Messages; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.core.ml.utils.XContentObjectTransformer; @@ -35,6 +37,12 @@ class AggProvider implements Writeable, ToXContentObject { static AggProvider fromXContent(XContentParser parser, boolean lenient) throws IOException { Map aggs = parser.mapOrdered(); + // NOTE: Always rewrite potentially old date histogram intervals. + // This should occur in 8.x+ but not 7.x. + // 7.x is BWC with versions that do not support the new date_histogram fields + if (lenient) { + rewriteDateHistogramInterval(aggs, false); + } AggregatorFactories.Builder parsedAggs = null; Exception exception = null; try { @@ -56,6 +64,35 @@ static AggProvider fromXContent(XContentParser parser, boolean lenient) throws I return new AggProvider(aggs, parsedAggs, exception); } + @SuppressWarnings("unchecked") + static boolean rewriteDateHistogramInterval(Map aggs, boolean inDateHistogram) { + boolean didRewrite = false; + if (aggs.containsKey(Histogram.INTERVAL_FIELD.getPreferredName()) && inDateHistogram) { + Object currentInterval = aggs.remove(Histogram.INTERVAL_FIELD.getPreferredName()); + if (DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(currentInterval.toString()) != null) { + aggs.put("calendar_interval", currentInterval.toString()); + didRewrite = true; + } else if (currentInterval instanceof Number) { + aggs.put("fixed_interval", ((Number)currentInterval).longValue() + "ms"); + didRewrite = true; + } else if (currentInterval instanceof String) { + aggs.put("fixed_interval", currentInterval.toString()); + didRewrite = true; + } else { + throw ExceptionsHelper.badRequestException(Messages.DATAFEED_CONFIG_AGG_BAD_FORMAT, + new IllegalArgumentException("unable to parse date_histogram interval parameter")); + } + } + for(Map.Entry entry : aggs.entrySet()) { + if (entry.getValue() instanceof Map) { + boolean rewrite = rewriteDateHistogramInterval((Map)entry.getValue(), + entry.getKey().equals(DateHistogramAggregationBuilder.NAME)); + didRewrite = didRewrite || rewrite; + } + } + return didRewrite; + } + static AggProvider fromParsedAggs(AggregatorFactories.Builder parsedAggs) throws IOException { return parsedAggs == null ? null : diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/AggProviderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/AggProviderTests.java index d6fc9ec9bc467..aacb9596a4e1c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/AggProviderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/AggProviderTests.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.Collections; +import java.util.HashMap; import java.util.Map; import static org.hamcrest.Matchers.equalTo; @@ -89,6 +90,82 @@ public void testEmptyAggMap() throws IOException { assertThat(e.getMessage(), equalTo("Datafeed aggregations are not parsable")); } + public void testRewriteBadNumericInterval() { + long numericInterval = randomNonNegativeLong(); + Map maxTime = Collections.singletonMap("max", Collections.singletonMap("field", "time")); + Map numericDeprecated = new HashMap<>(){{ + put("interval", numericInterval); + put("field", "foo"); + put("aggs", Collections.singletonMap("time", maxTime)); + }}; + Map expected = new HashMap<>() {{ + put("fixed_interval", numericInterval + "ms"); + put("field", "foo"); + put("aggs", Collections.singletonMap("time", maxTime)); + }}; + Map deprecated = Collections.singletonMap("buckets", Collections.singletonMap("date_histogram", numericDeprecated)); + assertTrue(AggProvider.rewriteDateHistogramInterval(deprecated, false)); + assertThat(deprecated, equalTo(Collections.singletonMap("buckets", Collections.singletonMap("date_histogram", expected)))); + + numericDeprecated = new HashMap<>(){{ + put("interval", numericInterval + "ms"); + put("field", "foo"); + put("aggs", Collections.singletonMap("time", maxTime)); + }}; + deprecated = Collections.singletonMap("date_histogram", Collections.singletonMap("date_histogram", numericDeprecated)); + assertTrue(AggProvider.rewriteDateHistogramInterval(deprecated, false)); + assertThat(deprecated, + equalTo(Collections.singletonMap("date_histogram", Collections.singletonMap("date_histogram", expected)))); + } + + public void testRewriteBadCalendarInterval() { + String calendarInterval = "1w"; + Map maxTime = Collections.singletonMap("max", Collections.singletonMap("field", "time")); + Map calendarDeprecated = new HashMap<>(){{ + put("interval", calendarInterval); + put("field", "foo"); + put("aggs", Collections.singletonMap("time", maxTime)); + }}; + Map expected = new HashMap<>() {{ + put("calendar_interval", calendarInterval); + put("field", "foo"); + put("aggs", Collections.singletonMap("time", maxTime)); + }}; + Map deprecated = Collections.singletonMap("buckets", + Collections.singletonMap("date_histogram", calendarDeprecated)); + assertTrue(AggProvider.rewriteDateHistogramInterval(deprecated, false)); + assertThat(deprecated, equalTo(Collections.singletonMap("buckets", Collections.singletonMap("date_histogram", expected)))); + + calendarDeprecated = new HashMap<>(){{ + put("interval", calendarInterval); + put("field", "foo"); + put("aggs", Collections.singletonMap("time", maxTime)); + }}; + deprecated = Collections.singletonMap("date_histogram", Collections.singletonMap("date_histogram", calendarDeprecated)); + assertTrue(AggProvider.rewriteDateHistogramInterval(deprecated, false)); + assertThat(deprecated, + equalTo(Collections.singletonMap("date_histogram", Collections.singletonMap("date_histogram", expected)))); + } + + public void testRewriteWhenNoneMustOccur() { + String calendarInterval = "1w"; + Map maxTime = Collections.singletonMap("max", Collections.singletonMap("field", "time")); + Map calendarDeprecated = new HashMap<>(){{ + put("calendar_interval", calendarInterval); + put("field", "foo"); + put("aggs", Collections.singletonMap("time", maxTime)); + }}; + Map expected = new HashMap<>() {{ + put("calendar_interval", calendarInterval); + put("field", "foo"); + put("aggs", Collections.singletonMap("time", maxTime)); + }}; + Map current = Collections.singletonMap("buckets", Collections.singletonMap("date_histogram", calendarDeprecated)); + assertFalse(AggProvider.rewriteDateHistogramInterval(current, false)); + assertThat(current, + equalTo(Collections.singletonMap("buckets", Collections.singletonMap("date_histogram", expected)))); + } + @Override protected AggProvider mutateInstance(AggProvider instance) throws IOException { Exception parsingException = instance.getParsingException(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java index 4c8bd4168df20..5f5dd77f1d51f 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java @@ -73,22 +73,6 @@ public class DatafeedConfigTests extends AbstractSerializingTestCase { - @AwaitsFix(bugUrl = "Tests need to be updated to use calendar/fixed interval explicitly") - public void testIntervalWarnings() { - /* - Placeholder test for visibility. Datafeeds use calendar and fixed intervals through the deprecated - methods. The randomized creation + final superclass tests made it impossible to add warning assertions, - so warnings have been disabled on this test. - - When fixed, `enableWarningsCheck()` should be removed. - */ - } - - @Override - protected boolean enableWarningsCheck() { - return false; - } - @Override protected DatafeedConfig createTestInstance() { return createRandomizedDatafeedConfig(randomAlphaOfLength(10)); @@ -234,7 +218,7 @@ protected DatafeedConfig doParseInstance(XContentParser parser) { " }\n" + "}"; - private static final String MULTIPLE_AGG_DEF_DATAFEED = "{\n" + + private static final String AGG_WITH_OLD_DATE_HISTOGRAM_INTERVAL = "{\n" + " \"datafeed_id\": \"farequote-datafeed\",\n" + " \"job_id\": \"farequote\",\n" + " \"frequency\": \"1h\",\n" + @@ -252,12 +236,33 @@ protected DatafeedConfig doParseInstance(XContentParser parser) { " }\n" + " }\n" + " }\n" + + " }\n" + + "}"; + + private static final String MULTIPLE_AGG_DEF_DATAFEED = "{\n" + + " \"datafeed_id\": \"farequote-datafeed\",\n" + + " \"job_id\": \"farequote\",\n" + + " \"frequency\": \"1h\",\n" + + " \"indices\": [\"farequote1\", \"farequote2\"],\n" + + " \"aggregations\": {\n" + + " \"buckets\": {\n" + + " \"date_histogram\": {\n" + + " \"field\": \"time\",\n" + + " \"fixed_interval\": \"360s\",\n" + + " \"time_zone\": \"UTC\"\n" + + " },\n" + + " \"aggregations\": {\n" + + " \"time\": {\n" + + " \"max\": {\"field\": \"time\"}\n" + + " }\n" + + " }\n" + + " }\n" + " }," + " \"aggs\": {\n" + " \"buckets2\": {\n" + " \"date_histogram\": {\n" + " \"field\": \"time\",\n" + - " \"interval\": \"360s\",\n" + + " \"fixed_interval\": \"360s\",\n" + " \"time_zone\": \"UTC\"\n" + " },\n" + " \"aggregations\": {\n" + @@ -313,6 +318,25 @@ public void testPastAggConfigParse() throws IOException { } } + public void testPastAggConfigOldDateHistogramParse() throws IOException { + try(XContentParser parser = XContentFactory.xContent(XContentType.JSON) + .createParser(xContentRegistry(), + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + AGG_WITH_OLD_DATE_HISTOGRAM_INTERVAL)) { + + DatafeedConfig datafeedConfig = DatafeedConfig.LENIENT_PARSER.apply(parser, null).build(); + assertThat(datafeedConfig.getParsedAggregations(xContentRegistry()), is(not(nullValue()))); + } + + try(XContentParser parser = XContentFactory.xContent(XContentType.JSON) + .createParser(xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, ANACHRONISTIC_AGG_DATAFEED)) { + + XContentParseException e = expectThrows(XContentParseException.class, + () -> DatafeedConfig.STRICT_PARSER.apply(parser, null).build()); + assertEquals("[25:3] [datafeed_config] failed to parse field [aggregations]", e.getMessage()); + } + } + public void testFutureMetadataParse() throws IOException { XContentParser parser = XContentFactory.xContent(XContentType.JSON) .createParser(xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, FUTURE_DATAFEED); @@ -503,7 +527,9 @@ public void testHasAggregations_NonEmpty() { builder.setIndices(Collections.singletonList("myIndex")); MaxAggregationBuilder maxTime = AggregationBuilders.max("time").field("time"); builder.setParsedAggregations(new AggregatorFactories.Builder().addAggregator( - AggregationBuilders.dateHistogram("time").interval(300000).subAggregation(maxTime).field("time"))); + AggregationBuilders.dateHistogram("time") + .fixedInterval(new DateHistogramInterval(300000 + "ms")) + .subAggregation(maxTime).field("time"))); DatafeedConfig datafeedConfig = builder.build(); assertThat(datafeedConfig.hasAggregations(), is(true)); @@ -535,21 +561,13 @@ public void testBuild_GivenHistogramWithDefaultInterval() { public void testBuild_GivenDateHistogramWithInvalidTimeZone() { MaxAggregationBuilder maxTime = AggregationBuilders.max("time").field("time"); DateHistogramAggregationBuilder dateHistogram = AggregationBuilders.dateHistogram("bucket").field("time") - .interval(300000L).timeZone(ZoneId.of("CET")).subAggregation(maxTime); + .fixedInterval(new DateHistogramInterval("30000ms")).timeZone(ZoneId.of("CET")).subAggregation(maxTime); ElasticsearchException e = expectThrows(ElasticsearchException.class, () -> createDatafeedWithDateHistogram(dateHistogram)); assertThat(e.getMessage(), equalTo("ML requires date_histogram.time_zone to be UTC")); } - @AwaitsFix(bugUrl = "Needs ML to look at and fix. Unclear how this should be handled, interval is not an optional param") - public void testBuild_GivenDateHistogramWithDefaultInterval() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> createDatafeedWithDateHistogram((String) null)); - - assertThat(e.getMessage(), containsString("Aggregation interval must be greater than 0")); - } - public void testBuild_GivenValidDateHistogram() { long millisInDay = 24 * 3600000L; @@ -566,13 +584,6 @@ public void testBuild_GivenValidDateHistogram() { equalTo(7 * millisInDay + 1)); } - public void testBuild_GivenDateHistogramWithMoreThanCalendarWeek() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> createDatafeedWithDateHistogram("8d")); - - assertThat(e.getMessage(), containsString("When specifying a date_histogram calendar interval [8d]")); - } - public void testDefaultChunkingConfig_GivenAggregations() { assertThat(createDatafeedWithDateHistogram("1s").getChunkingConfig(), equalTo(ChunkingConfig.newManual(TimeValue.timeValueSeconds(1000)))); @@ -711,7 +722,7 @@ public void testSerializationOfComplexAggs() throws IOException { new Script("params.bytes > 0 ? params.bytes : null")); DateHistogramAggregationBuilder dateHistogram = AggregationBuilders.dateHistogram("histogram_buckets") - .field("timestamp").interval(300000).timeZone(ZoneOffset.UTC) + .field("timestamp").fixedInterval(new DateHistogramInterval("300000ms")).timeZone(ZoneOffset.UTC) .subAggregation(maxTime) .subAggregation(avgAggregationBuilder) .subAggregation(derivativePipelineAggregationBuilder) @@ -763,7 +774,7 @@ public void testSerializationOfComplexAggsBetweenVersions() throws IOException { new Script("params.bytes > 0 ? params.bytes : null")); DateHistogramAggregationBuilder dateHistogram = AggregationBuilders.dateHistogram("histogram_buckets") - .field("timestamp").interval(300000).timeZone(ZoneOffset.UTC) + .field("timestamp").fixedInterval(new DateHistogramInterval("30000ms")).timeZone(ZoneOffset.UTC) .subAggregation(maxTime) .subAggregation(avgAggregationBuilder) .subAggregation(derivativePipelineAggregationBuilder) @@ -813,7 +824,11 @@ private static DatafeedConfig createDatafeedWithDateHistogram(String interval) { MaxAggregationBuilder maxTime = AggregationBuilders.max("time").field("time"); DateHistogramAggregationBuilder dateHistogram = AggregationBuilders.dateHistogram("buckets").subAggregation(maxTime).field("time"); if (interval != null) { - dateHistogram.dateHistogramInterval(new DateHistogramInterval(interval)); + if (DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(interval) != null) { + dateHistogram.calendarInterval(new DateHistogramInterval(interval)); + } else { + dateHistogram.fixedInterval(new DateHistogramInterval(interval)); + } } return createDatafeedWithDateHistogram(dateHistogram); } @@ -822,7 +837,7 @@ private static DatafeedConfig createDatafeedWithDateHistogram(Long interval) { MaxAggregationBuilder maxTime = AggregationBuilders.max("time").field("time"); DateHistogramAggregationBuilder dateHistogram = AggregationBuilders.dateHistogram("buckets").subAggregation(maxTime).field("time"); if (interval != null) { - dateHistogram.interval(interval); + dateHistogram.fixedInterval(new DateHistogramInterval(interval + "ms")); } return createDatafeedWithDateHistogram(dateHistogram); } @@ -879,9 +894,12 @@ protected DatafeedConfig mutateInstance(DatafeedConfig instance) throws IOExcept } else { AggregatorFactories.Builder aggBuilder = new AggregatorFactories.Builder(); String timeField = randomAlphaOfLength(10); + long fixedInterval = between(10000, 3600000); + aggBuilder - .addAggregator(new DateHistogramAggregationBuilder(timeField).field(timeField).interval(between(10000, 3600000)) - .subAggregation(new MaxAggregationBuilder(timeField).field(timeField))); + .addAggregator(new DateHistogramAggregationBuilder(timeField).field(timeField) + .fixedInterval(new DateHistogramInterval(fixedInterval + "ms")) + .subAggregation(new MaxAggregationBuilder(timeField).field(timeField))); builder.setParsedAggregations(aggBuilder); if (instance.getScriptFields().isEmpty() == false) { builder.setScriptFields(Collections.emptyList()); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdateTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdateTests.java index e92b09c169877..b988e65716c06 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdateTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdateTests.java @@ -31,6 +31,7 @@ import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.PipelineAggregatorBuilders; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.search.aggregations.metrics.AvgAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.MaxAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.BucketScriptPipelineAggregationBuilder; @@ -57,22 +58,6 @@ public class DatafeedUpdateTests extends AbstractSerializingTestCase { - @AwaitsFix(bugUrl = "Tests need to be updated to use calendar/fixed interval explicitly") - public void testIntervalWarnings() { - /* - Placeholder test for visibility. Datafeeds use calendar and fixed intervals through the deprecated - methods. The randomized creation + final superclass tests made it impossible to add warning assertions, - so warnings have been disabled on this test. - - When fixed, `enableWarningsCheck()` should be removed. - */ - } - - @Override - protected boolean enableWarningsCheck() { - return false; - } - @Override protected DatafeedUpdate createTestInstance() { return createRandomized(DatafeedConfigTests.randomValidDatafeedId()); @@ -157,7 +142,7 @@ protected NamedXContentRegistry xContentRegistry() { " \"buckets\": {\n" + " \"date_histogram\": {\n" + " \"field\": \"time\",\n" + - " \"interval\": \"360s\",\n" + + " \"fixed_interval\": \"360s\",\n" + " \"time_zone\": \"UTC\"\n" + " },\n" + " \"aggregations\": {\n" + @@ -171,7 +156,7 @@ protected NamedXContentRegistry xContentRegistry() { " \"buckets2\": {\n" + " \"date_histogram\": {\n" + " \"field\": \"time\",\n" + - " \"interval\": \"360s\",\n" + + " \"fixed_interval\": \"360s\",\n" + " \"time_zone\": \"UTC\"\n" + " },\n" + " \"aggregations\": {\n" + @@ -196,7 +181,8 @@ public void testMultipleDefinedAggParse() throws IOException { public void testApply_failBecauseTargetDatafeedHasDifferentId() { DatafeedConfig datafeed = DatafeedConfigTests.createRandomizedDatafeedConfig("foo"); - expectThrows(IllegalArgumentException.class, () -> createRandomized(datafeed.getId() + "_2").apply(datafeed, null)); + expectThrows(IllegalArgumentException.class, () -> createRandomized(datafeed.getId() + "_2") + .apply(datafeed, null)); } public void testApply_failBecauseJobIdChanged() { @@ -212,14 +198,16 @@ public void testApply_failBecauseJobIdChanged() { .setJobId("bar") .build(); ElasticsearchStatusException ex = expectThrows( - ElasticsearchStatusException.class, () -> datafeedUpdateWithChangedJobId.apply(datafeed, Collections.emptyMap())); + ElasticsearchStatusException.class, + () -> datafeedUpdateWithChangedJobId.apply(datafeed, Collections.emptyMap())); assertThat(ex.status(), equalTo(RestStatus.BAD_REQUEST)); assertThat(ex.getMessage(), equalTo(DatafeedUpdate.ERROR_MESSAGE_ON_JOB_ID_UPDATE)); } public void testApply_givenEmptyUpdate() { DatafeedConfig datafeed = DatafeedConfigTests.createRandomizedDatafeedConfig("foo"); - DatafeedConfig updatedDatafeed = new DatafeedUpdate.Builder(datafeed.getId()).build().apply(datafeed, Collections.emptyMap()); + DatafeedConfig updatedDatafeed = new DatafeedUpdate.Builder(datafeed.getId()).build() + .apply(datafeed, Collections.emptyMap()); assertThat(datafeed, equalTo(updatedDatafeed)); } @@ -317,7 +305,7 @@ public void testSerializationOfComplexAggsBetweenVersions() throws IOException { new Script("params.bytes > 0 ? params.bytes : null")); DateHistogramAggregationBuilder dateHistogram = AggregationBuilders.dateHistogram("histogram_buckets") - .field("timestamp").interval(300000).timeZone(ZoneOffset.UTC) + .field("timestamp").fixedInterval(new DateHistogramInterval("300000ms")).timeZone(ZoneOffset.UTC) .subAggregation(maxTime) .subAggregation(avgAggregationBuilder) .subAggregation(derivativePipelineAggregationBuilder) @@ -400,7 +388,8 @@ protected DatafeedUpdate mutateInstance(DatafeedUpdate instance) throws IOExcept } else { AggregatorFactories.Builder aggBuilder = new AggregatorFactories.Builder(); String timeField = randomAlphaOfLength(10); - aggBuilder.addAggregator(new DateHistogramAggregationBuilder(timeField).field(timeField).interval(between(10000, 3600000)) + DateHistogramInterval interval = new DateHistogramInterval(between(10000, 3600000) + "ms"); + aggBuilder.addAggregator(new DateHistogramAggregationBuilder(timeField).field(timeField).fixedInterval(interval) .subAggregation(new MaxAggregationBuilder(timeField).field(timeField))); builder.setAggregations(AggProvider.fromParsedAggs(aggBuilder)); if (instance.getScriptFields().isEmpty() == false) { diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedWithAggsIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedWithAggsIT.java index c93316afec169..0c864152a8e49 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedWithAggsIT.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedWithAggsIT.java @@ -12,6 +12,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.search.aggregations.AggregationBuilders; import org.elasticsearch.search.aggregations.AggregatorFactories; +import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.xpack.core.ml.action.GetBucketsAction; import org.elasticsearch.xpack.core.ml.action.GetDatafeedsStatsAction; import org.elasticsearch.xpack.core.ml.action.StopDatafeedAction; @@ -65,7 +66,8 @@ public void testRealtime() throws Exception { datafeedBuilder.setIndices(Collections.singletonList(dataIndex)); AggregatorFactories.Builder aggs = new AggregatorFactories.Builder(); - aggs.addAggregator(AggregationBuilders.dateHistogram("time").field("time").interval(1000) + aggs.addAggregator(AggregationBuilders.dateHistogram("time").field("time") + .fixedInterval(new DateHistogramInterval("1000ms")) .subAggregation(AggregationBuilders.max("time").field("time"))); datafeedBuilder.setParsedAggregations(aggs); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java index 3e81a1950d785..41dd9a76c1dbd 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java @@ -38,7 +38,6 @@ public class TransportUpdateDatafeedAction extends TransportMasterNodeAction { - private final Client client; private final DatafeedConfigProvider datafeedConfigProvider; private final JobConfigProvider jobConfigProvider; private final MlConfigMigrationEligibilityCheck migrationEligibilityCheck; @@ -51,7 +50,6 @@ public TransportUpdateDatafeedAction(Settings settings, TransportService transpo super(UpdateDatafeedAction.NAME, transportService, clusterService, threadPool, actionFilters, UpdateDatafeedAction.Request::new, indexNameExpressionResolver); - this.client = client; this.datafeedConfigProvider = new DatafeedConfigProvider(client, xContentRegistry); this.jobConfigProvider = new JobConfigProvider(client, xContentRegistry); this.migrationEligibilityCheck = new MlConfigMigrationEligibilityCheck(settings, clusterService); diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml index 4d2254a1ba8c3..5295ec09b7f8f 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml @@ -1,7 +1,10 @@ setup: - - skip: - version: "all" - reason: "AwaitsFix https://github.com/elastic/elasticsearch/issues/42258" + - do: + cluster.health: + wait_for_status: green + wait_for_nodes: 3 + # wait for long enough that we give delayed unassigned shards to stop being delayed + timeout: 70s --- "Test old cluster datafeed without aggs": @@ -23,7 +26,8 @@ setup: --- "Test old cluster datafeed with aggs": - skip: - features: "warnings" + version: "all" + reason: "If we hit the old node we get a warning. If we hit the new node, we don't" - do: warnings: - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' @@ -88,9 +92,6 @@ setup: --- "Put job and datafeed with aggs in mixed cluster": - - skip: - features: "warnings" - - do: ml.put_job: job_id: mixed-cluster-datafeed-job-with-aggs @@ -112,9 +113,6 @@ setup: } - do: - warnings: - - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' - - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' ml.put_datafeed: datafeed_id: mixed-cluster-datafeed-with-aggs body: > @@ -126,7 +124,7 @@ setup: "buckets": { "date_histogram": { "field": "time", - "interval": "30s", + "fixed_interval": "30s", "time_zone": "UTC" }, "aggregations": { diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml index 62a9d33a511e6..2978fc68050d2 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml @@ -1,7 +1,10 @@ setup: - - skip: - version: "all" - reason: "AwaitsFix https://github.com/elastic/elasticsearch/issues/42258" + - do: + cluster.health: + wait_for_status: green + wait_for_nodes: 3 + # wait for long enough that we give delayed unassigned shards to stop being delayed + timeout: 70s --- "Put job and datafeed without aggs in old cluster": @@ -188,6 +191,8 @@ setup: } - do: + warnings: + - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' ml.get_datafeed_stats: datafeed_id: old-cluster-datafeed-with-aggs - match: { datafeeds.0.state: stopped} diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/40_ml_datafeed_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/40_ml_datafeed_crud.yml index 4b742e10de61f..b3a538093242f 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/40_ml_datafeed_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/40_ml_datafeed_crud.yml @@ -1,8 +1,4 @@ setup: - - skip: - version: "all" - reason: "AwaitsFix https://github.com/elastic/elasticsearch/issues/42258" - - do: cluster.health: wait_for_status: green @@ -109,8 +105,6 @@ setup: --- "Test old and mixed cluster datafeeds with aggs": - - skip: - features: "warnings" - do: indices.create: index: airline-data @@ -121,8 +115,6 @@ setup: type: date - do: - warnings: - - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' ml.get_datafeeds: datafeed_id: old-cluster-datafeed-with-aggs - match: { datafeeds.0.datafeed_id: "old-cluster-datafeed-with-aggs"} @@ -130,6 +122,7 @@ setup: - gte: { datafeeds.0.scroll_size: 2000 } - is_false: datafeeds.0.script_fields - match: { datafeeds.0.aggregations.buckets.date_histogram.field: time } + - match: { datafeeds.0.aggregations.buckets.date_histogram.fixed_interval: "30s" } - match: { datafeeds.0.aggregations.buckets.aggregations.time.max.field: time } - do: @@ -139,8 +132,6 @@ setup: - is_false: datafeeds.0.node - do: - warnings: - - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' ml.get_datafeeds: datafeed_id: mixed-cluster-datafeed-with-aggs - match: { datafeeds.0.datafeed_id: "mixed-cluster-datafeed-with-aggs"} @@ -148,6 +139,7 @@ setup: - gte: { datafeeds.0.scroll_size: 2000 } - is_false: datafeeds.0.script_fields - match: { datafeeds.0.aggregations.buckets.date_histogram.field: time } + - match: { datafeeds.0.aggregations.buckets.date_histogram.fixed_interval: "30s" } - match: { datafeeds.0.aggregations.buckets.aggregations.time.max.field: time } - do: @@ -161,8 +153,6 @@ setup: job_id: old-cluster-datafeed-job-with-aggs - do: - warnings: - - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' ml.start_datafeed: datafeed_id: old-cluster-datafeed-with-aggs start: 0 @@ -189,8 +179,6 @@ setup: job_id: mixed-cluster-datafeed-job-with-aggs - do: - warnings: - - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' ml.start_datafeed: datafeed_id: mixed-cluster-datafeed-with-aggs start: 0 From 612b900397cf45aa762be8d230a671d148e254ba Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Thu, 20 Feb 2020 07:46:10 -0500 Subject: [PATCH 2/5] fixing bwc test --- .../rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml index 5295ec09b7f8f..0d6644bc8e6fc 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml @@ -26,6 +26,7 @@ setup: --- "Test old cluster datafeed with aggs": - skip: + feature: warnings version: "all" reason: "If we hit the old node we get a warning. If we hit the new node, we don't" - do: From 00367ea646351e3576532098b64e6483d3895d63 Mon Sep 17 00:00:00 2001 From: Benjamin Trent Date: Thu, 20 Feb 2020 08:02:49 -0500 Subject: [PATCH 3/5] Update 40_ml_datafeed_crud.yml --- .../rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml index 0d6644bc8e6fc..8b56393711175 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml @@ -26,7 +26,7 @@ setup: --- "Test old cluster datafeed with aggs": - skip: - feature: warnings + features: warnings version: "all" reason: "If we hit the old node we get a warning. If we hit the new node, we don't" - do: From c60f8c5f706dbd3cefc55e36d17cff67c2cc1eae Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Thu, 20 Feb 2020 08:57:07 -0500 Subject: [PATCH 4/5] adjusting bwc tests for rolling upgrade from 8.0.0 --- .../test/old_cluster/40_ml_datafeed_crud.yml | 79 ++++++++++++++++++- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml index 2978fc68050d2..c313ea131a4d6 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml @@ -124,10 +124,10 @@ setup: - is_false: datafeeds.0.node --- -"Put job and datafeed with aggs in old cluster - deprecated interval with warning": +"Put job and datafeed with aggs in old cluster - deprecated interval with warning before 8.0.0": - skip: - version: " - 7.1.99" - reason: calendar_interval introduced in 7.2.0 + version: "7.99.99 - " + reason: at version 8.0.0, deprecation warnings don't occur on get_stats features: warnings - do: @@ -197,3 +197,76 @@ setup: datafeed_id: old-cluster-datafeed-with-aggs - match: { datafeeds.0.state: stopped} - is_false: datafeeds.0.node + +--- +"Put job and datafeed with aggs in old cluster - deprecated interval with warning after 8.0.0": + - skip: + version: " - 7.99.99" + reason: at version 8.0.0, deprecation warnings don't occur on get_stats + features: warnings + + - do: + ml.put_job: + job_id: old-cluster-datafeed-job-with-aggs + body: > + { + "description":"Cluster upgrade", + "analysis_config" : { + "bucket_span": "60s", + "summary_count_field_name": "doc_count", + "detectors" :[{"function":"count"}] + }, + "analysis_limits" : { + "model_memory_limit": "50mb" + }, + "data_description" : { + "format":"xcontent", + "time_field":"time" + } + } + - match: { job_id: old-cluster-datafeed-job-with-aggs } + + - do: + warnings: + - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' + ml.put_datafeed: + datafeed_id: old-cluster-datafeed-with-aggs + body: > + { + "job_id":"old-cluster-datafeed-job-with-aggs", + "indices":["airline-data"], + "scroll_size": 2000, + "aggregations": { + "buckets": { + "date_histogram": { + "field": "time", + "interval": "30s", + "time_zone": "UTC" + }, + "aggregations": { + "time": { + "max": {"field": "time"} + }, + "airline": { + "terms": { + "field": "airline", + "size": 100 + }, + "aggregations": { + "responsetime": { + "avg": { + "field": "responsetime" + } + } + } + } + } + } + } + } + + - do: + ml.get_datafeed_stats: + datafeed_id: old-cluster-datafeed-with-aggs + - match: { datafeeds.0.state: stopped} + - is_false: datafeeds.0.node From 5b2d978a07261ec1fe88d4f18227eb710c38681c Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Mon, 24 Feb 2020 09:40:30 -0500 Subject: [PATCH 5/5] fixing bwc tests + adding back large calendar interval test --- .../xpack/core/ml/datafeed/DatafeedConfigTests.java | 7 +++++++ .../test/mixed_cluster/40_ml_datafeed_crud.yml | 1 + .../rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml | 7 ++----- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java index 5f5dd77f1d51f..672dc65c43c21 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java @@ -584,6 +584,13 @@ public void testBuild_GivenValidDateHistogram() { equalTo(7 * millisInDay + 1)); } + public void testBuild_GivenDateHistogramWithMoreThanCalendarWeek() { + ElasticsearchException e = expectThrows(ElasticsearchException.class, + () -> createDatafeedWithDateHistogram("month")); + + assertThat(e.getMessage(), containsString("When specifying a date_histogram calendar interval [month]")); + } + public void testDefaultChunkingConfig_GivenAggregations() { assertThat(createDatafeedWithDateHistogram("1s").getChunkingConfig(), equalTo(ChunkingConfig.newManual(TimeValue.timeValueSeconds(1000)))); diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml index 8b56393711175..6ff05357887ef 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/40_ml_datafeed_crud.yml @@ -27,6 +27,7 @@ setup: "Test old cluster datafeed with aggs": - skip: features: warnings + #TODO remove skip when master is bumped to 9.0.0 version: "all" reason: "If we hit the old node we get a warning. If we hit the new node, we don't" - do: diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml index c313ea131a4d6..491435f43fb93 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/40_ml_datafeed_crud.yml @@ -126,7 +126,7 @@ setup: --- "Put job and datafeed with aggs in old cluster - deprecated interval with warning before 8.0.0": - skip: - version: "7.99.99 - " + version: "8.0.0 - " reason: at version 8.0.0, deprecation warnings don't occur on get_stats features: warnings @@ -203,7 +203,6 @@ setup: - skip: version: " - 7.99.99" reason: at version 8.0.0, deprecation warnings don't occur on get_stats - features: warnings - do: ml.put_job: @@ -227,8 +226,6 @@ setup: - match: { job_id: old-cluster-datafeed-job-with-aggs } - do: - warnings: - - '[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.' ml.put_datafeed: datafeed_id: old-cluster-datafeed-with-aggs body: > @@ -240,7 +237,7 @@ setup: "buckets": { "date_histogram": { "field": "time", - "interval": "30s", + "fixed_interval": "30s", "time_zone": "UTC" }, "aggregations": {