From bcd4281359dfdf336490d0e89db7af56c766e80c Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Thu, 6 Jun 2019 21:45:49 +0200 Subject: [PATCH 01/16] Add TimingStats to datafeed GetDatafeedStatsAction.Response --- .../client/ml/datafeed/DatafeedStats.java | 22 ++- .../ml/datafeed/DatafeedTimingStats.java | 99 +++++++++++++ .../ml/datafeed/DatafeedStatsTests.java | 3 +- .../ml/datafeed/DatafeedTimingStatsTests.java | 46 ++++++ .../ml/apis/datafeedresource.asciidoc | 6 + .../ml/apis/get-datafeed-stats.asciidoc | 6 +- .../ml/action/GetDatafeedsStatsAction.java | 31 +++- .../core/ml/datafeed/DatafeedTimingStats.java | 138 ++++++++++++++++++ .../persistence/AnomalyDetectorsIndex.java | 8 + .../GetDatafeedStatsActionResponseTests.java | 30 ++-- .../ml/datafeed/DatafeedTimingStatsTests.java | 108 ++++++++++++++ .../xpack/ml/MachineLearning.java | 19 ++- .../TransportGetDatafeedsStatsAction.java | 58 ++++++-- .../TransportPreviewDatafeedAction.java | 72 +++++---- .../action/TransportStartDatafeedAction.java | 26 +++- .../xpack/ml/datafeed/DatafeedJobBuilder.java | 51 ++++--- .../datafeed/DatafeedTimingStatsReporter.java | 57 ++++++++ .../extractor/DataExtractorFactory.java | 14 +- .../AbstractAggregationDataExtractor.java | 20 ++- .../aggregation/AggregationDataExtractor.java | 6 +- .../AggregationDataExtractorFactory.java | 14 +- .../aggregation/RollupDataExtractor.java | 7 +- .../RollupDataExtractorFactory.java | 17 ++- .../chunked/ChunkedDataExtractor.java | 23 ++- .../chunked/ChunkedDataExtractorFactory.java | 8 +- .../extractor/scroll/ScrollDataExtractor.java | 24 ++- .../scroll/ScrollDataExtractorFactory.java | 36 +++-- .../xpack/ml/job/JobManager.java | 9 +- .../job/persistence/JobResultsPersister.java | 13 ++ .../job/persistence/JobResultsProvider.java | 47 ++++++ .../ml/datafeed/DatafeedJobBuilderTests.java | 23 ++- .../DatafeedTimingStatsReporterTests.java | 61 ++++++++ .../extractor/DataExtractorFactoryTests.java | 39 +++-- .../AggregationDataExtractorFactoryTests.java | 6 +- .../AggregationDataExtractorTests.java | 9 +- .../ChunkedDataExtractorFactoryTests.java | 12 +- .../chunked/ChunkedDataExtractorTests.java | 11 +- .../scroll/ScrollDataExtractorTests.java | 9 +- .../xpack/ml/job/JobManagerTests.java | 16 +- .../persistence/JobResultsPersisterTests.java | 36 +++++ .../persistence/JobResultsProviderTests.java | 79 ++++++++++ .../test/ml/get_datafeed_stats.yml | 44 +++++- 42 files changed, 1176 insertions(+), 187 deletions(-) create mode 100644 client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStats.java create mode 100644 client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStatsTests.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java create mode 100644 x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java create mode 100644 x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedStats.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedStats.java index 8a9f9ae9a79dc..42527092558d9 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedStats.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedStats.java @@ -41,9 +41,12 @@ public class DatafeedStats implements ToXContentObject { private final NodeAttributes node; @Nullable private final String assignmentExplanation; + @Nullable + private final DatafeedTimingStats timingStats; public static final ParseField ASSIGNMENT_EXPLANATION = new ParseField("assignment_explanation"); public static final ParseField NODE = new ParseField("node"); + public static final ParseField TIMING_STATS = new ParseField("timing_stats"); public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("datafeed_stats", true, @@ -52,7 +55,8 @@ public class DatafeedStats implements ToXContentObject { DatafeedState datafeedState = DatafeedState.fromString((String)a[1]); NodeAttributes nodeAttributes = (NodeAttributes)a[2]; String assignmentExplanation = (String)a[3]; - return new DatafeedStats(datafeedId, datafeedState, nodeAttributes, assignmentExplanation); + DatafeedTimingStats timingStats = (DatafeedTimingStats)a[4]; + return new DatafeedStats(datafeedId, datafeedState, nodeAttributes, assignmentExplanation, timingStats); } ); static { @@ -60,14 +64,16 @@ public class DatafeedStats implements ToXContentObject { PARSER.declareString(ConstructingObjectParser.constructorArg(), DatafeedState.STATE); PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), NodeAttributes.PARSER, NODE); PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), ASSIGNMENT_EXPLANATION); + PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), DatafeedTimingStats.PARSER, TIMING_STATS); } public DatafeedStats(String datafeedId, DatafeedState datafeedState, @Nullable NodeAttributes node, - @Nullable String assignmentExplanation) { + @Nullable String assignmentExplanation, @Nullable DatafeedTimingStats timingStats) { this.datafeedId = Objects.requireNonNull(datafeedId); this.datafeedState = Objects.requireNonNull(datafeedState); this.node = node; this.assignmentExplanation = assignmentExplanation; + this.timingStats = timingStats; } public String getDatafeedId() { @@ -86,6 +92,10 @@ public String getAssignmentExplanation() { return assignmentExplanation; } + public DatafeedTimingStats getDatafeedTimingStats() { + return timingStats; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { builder.startObject(); @@ -110,13 +120,16 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par if (assignmentExplanation != null) { builder.field(ASSIGNMENT_EXPLANATION.getPreferredName(), assignmentExplanation); } + if (timingStats != null) { + builder.field(TIMING_STATS.getPreferredName(), timingStats); + } builder.endObject(); return builder; } @Override public int hashCode() { - return Objects.hash(datafeedId, datafeedState.toString(), node, assignmentExplanation); + return Objects.hash(datafeedId, datafeedState.toString(), node, assignmentExplanation, timingStats); } @Override @@ -131,6 +144,7 @@ public boolean equals(Object obj) { return Objects.equals(datafeedId, other.datafeedId) && Objects.equals(this.datafeedState, other.datafeedState) && Objects.equals(this.node, other.node) && - Objects.equals(this.assignmentExplanation, other.assignmentExplanation); + Objects.equals(this.assignmentExplanation, other.assignmentExplanation) && + Objects.equals(this.timingStats, other.timingStats); } } diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStats.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStats.java new file mode 100644 index 0000000000000..8428ffe01c9c1 --- /dev/null +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStats.java @@ -0,0 +1,99 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.client.ml.datafeed; + +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.Objects; + +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; + +public class DatafeedTimingStats implements ToXContentObject { + + public static final ParseField JOB_ID = new ParseField("job_id"); + public static final ParseField TOTAL_SEARCH_TIME_MS = new ParseField("total_search_time_ms"); + + public static final ParseField TYPE = new ParseField("datafeed_timing_stats"); + + public static final ConstructingObjectParser PARSER = createParser(); + + private static ConstructingObjectParser createParser() { + ConstructingObjectParser parser = + new ConstructingObjectParser<>("datafeed_timing_stats", true, a -> new DatafeedTimingStats((String) a[0], (double) a[1])); + parser.declareString(constructorArg(), JOB_ID); + parser.declareDouble(optionalConstructorArg(), TOTAL_SEARCH_TIME_MS); + return parser; + } + + private final String jobId; + private double totalSearchTimeMs; + + public DatafeedTimingStats(String jobId, double totalSearchTimeMs) { + this.jobId = Objects.requireNonNull(jobId); + this.totalSearchTimeMs = totalSearchTimeMs; + } + + public String getJobId() { + return jobId; + } + + public double getTotalSearchTimeMs() { + return totalSearchTimeMs; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { + builder.startObject(); + builder.field(JOB_ID.getPreferredName(), jobId); + builder.field(TOTAL_SEARCH_TIME_MS.getPreferredName(), totalSearchTimeMs); + builder.endObject(); + return builder; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + + DatafeedTimingStats other = (DatafeedTimingStats) obj; + return Objects.equals(this.jobId, other.jobId) + && Objects.equals(this.totalSearchTimeMs, other.totalSearchTimeMs); + } + + @Override + public int hashCode() { + return Objects.hash(jobId, totalSearchTimeMs); + } + + @Override + public String toString() { + return Strings.toString(this); + } +} diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedStatsTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedStatsTests.java index 50c0809d201ea..fd2b06a8aa5fd 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedStatsTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedStatsTests.java @@ -50,7 +50,8 @@ public static DatafeedStats createRandomInstance() { attributes); } String assignmentReason = randomBoolean() ? randomAlphaOfLength(10) : null; - return new DatafeedStats(datafeedId, datafeedState, nodeAttributes, assignmentReason); + DatafeedTimingStats timingStats = DatafeedTimingStatsTests.createRandomInstance(); + return new DatafeedStats(datafeedId, datafeedState, nodeAttributes, assignmentReason, timingStats); } @Override diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStatsTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStatsTests.java new file mode 100644 index 0000000000000..6bdbceb942265 --- /dev/null +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStatsTests.java @@ -0,0 +1,46 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.client.ml.datafeed; + +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractXContentTestCase; + +import java.io.IOException; + +public class DatafeedTimingStatsTests extends AbstractXContentTestCase { + + public static DatafeedTimingStats createRandomInstance() { + return new DatafeedTimingStats(randomAlphaOfLength(10), randomDouble()); + } + + @Override + protected DatafeedTimingStats createTestInstance() { + return createRandomInstance(); + } + + @Override + protected DatafeedTimingStats doParseInstance(XContentParser parser) throws IOException { + return DatafeedTimingStats.PARSER.apply(parser, null); + } + + @Override + protected boolean supportsUnknownFields() { + return true; + } +} diff --git a/docs/reference/ml/apis/datafeedresource.asciidoc b/docs/reference/ml/apis/datafeedresource.asciidoc index 5c1e3e74a6ae8..87b19b7433455 100644 --- a/docs/reference/ml/apis/datafeedresource.asciidoc +++ b/docs/reference/ml/apis/datafeedresource.asciidoc @@ -143,3 +143,9 @@ update their values: `started`::: The {dfeed} is actively receiving data. `stopped`::: The {dfeed} is stopped and will not receive data until it is re-started. + +`timing_stats`:: + (object) An object that provides statistical information about timing aspect of this datafeed. + + `job_id`::: A numerical character string that uniquely identifies the job. + `total_search_time_ms`::: Total time the datafeed spent searching in milliseconds. + diff --git a/docs/reference/ml/apis/get-datafeed-stats.asciidoc b/docs/reference/ml/apis/get-datafeed-stats.asciidoc index 6ce99785912af..db041a235779a 100644 --- a/docs/reference/ml/apis/get-datafeed-stats.asciidoc +++ b/docs/reference/ml/apis/get-datafeed-stats.asciidoc @@ -90,7 +90,11 @@ The API returns the following results: "ml.max_open_jobs": "20" } }, - "assignment_explanation": "" + "assignment_explanation": "", + "timing_stats": { + "job_id": "job-total-requests", + "total_search_time_ms": 120.5 + } } ] } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java index 9caf07a5bd8ff..f619c76f52f89 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java @@ -22,12 +22,15 @@ import org.elasticsearch.xpack.core.action.util.QueryPage; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import java.io.IOException; import java.util.Map; import java.util.Objects; +import static org.elasticsearch.Version.V_7_3_0; + public class GetDatafeedsStatsAction extends StreamableResponseActionType { public static final GetDatafeedsStatsAction INSTANCE = new GetDatafeedsStatsAction(); @@ -128,13 +131,16 @@ public static class DatafeedStats implements ToXContentObject, Writeable { private DiscoveryNode node; @Nullable private String assignmentExplanation; + @Nullable + private DatafeedTimingStats timingStats; public DatafeedStats(String datafeedId, DatafeedState datafeedState, @Nullable DiscoveryNode node, - @Nullable String assignmentExplanation) { + @Nullable String assignmentExplanation, @Nullable DatafeedTimingStats timingStats) { this.datafeedId = Objects.requireNonNull(datafeedId); this.datafeedState = Objects.requireNonNull(datafeedState); this.node = node; this.assignmentExplanation = assignmentExplanation; + this.timingStats = timingStats; } DatafeedStats(StreamInput in) throws IOException { @@ -142,6 +148,11 @@ public DatafeedStats(String datafeedId, DatafeedState datafeedState, @Nullable D datafeedState = DatafeedState.fromStream(in); node = in.readOptionalWriteable(DiscoveryNode::new); assignmentExplanation = in.readOptionalString(); + if (in.getVersion().onOrAfter(V_7_3_0)) { + timingStats = in.readOptionalWriteable(DatafeedTimingStats::new); + } else { + timingStats = null; + } } public String getDatafeedId() { @@ -160,6 +171,10 @@ public String getAssignmentExplanation() { return assignmentExplanation; } + public DatafeedTimingStats getTimingStats() { + return timingStats; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); @@ -184,6 +199,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (assignmentExplanation != null) { builder.field("assignment_explanation", assignmentExplanation); } + if (timingStats != null) { + builder.field("timing_stats", timingStats); + } builder.endObject(); return builder; } @@ -194,11 +212,14 @@ public void writeTo(StreamOutput out) throws IOException { datafeedState.writeTo(out); out.writeOptionalWriteable(node); out.writeOptionalString(assignmentExplanation); + if (out.getVersion().onOrAfter(V_7_3_0)) { + out.writeOptionalWriteable(timingStats); + } } @Override public int hashCode() { - return Objects.hash(datafeedId, datafeedState, node, assignmentExplanation); + return Objects.hash(datafeedId, datafeedState, node, assignmentExplanation, timingStats); } @Override @@ -210,10 +231,11 @@ public boolean equals(Object obj) { return false; } DatafeedStats other = (DatafeedStats) obj; - return Objects.equals(datafeedId, other.datafeedId) && + return Objects.equals(this.datafeedId, other.datafeedId) && Objects.equals(this.datafeedState, other.datafeedState) && Objects.equals(this.node, other.node) && - Objects.equals(this.assignmentExplanation, other.assignmentExplanation); + Objects.equals(this.assignmentExplanation, other.assignmentExplanation) && + Objects.equals(this.timingStats, other.timingStats); } } @@ -232,5 +254,4 @@ protected Reader getReader() { return DatafeedStats::new; } } - } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java new file mode 100644 index 0000000000000..e80733448ebf2 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java @@ -0,0 +1,138 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.ml.datafeed; + +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.Objects; + +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; + +public class DatafeedTimingStats implements ToXContentObject, Writeable { + + public static final ParseField JOB_ID = new ParseField("job_id"); + public static final ParseField TOTAL_SEARCH_TIME_MS = new ParseField("total_search_time_ms"); + + public static final ParseField TYPE = new ParseField("datafeed_timing_stats"); + + public static final ConstructingObjectParser PARSER = createParser(); + + private static ConstructingObjectParser createParser() { + ConstructingObjectParser parser = + new ConstructingObjectParser<>( + "datafeed_timing_stats", true, args -> new DatafeedTimingStats((String) args[0], (double) args[1])); + parser.declareString(constructorArg(), JOB_ID); + parser.declareDouble(optionalConstructorArg(), TOTAL_SEARCH_TIME_MS); + return parser; + } + + public static String documentId(String jobId) { + return jobId + "_datafeed_timing_stats"; + } + + private final String jobId; + private double totalSearchTimeMs; + + public DatafeedTimingStats(String jobId, double totalSearchTimeMs) { + this.jobId = Objects.requireNonNull(jobId); + this.totalSearchTimeMs = totalSearchTimeMs; + } + + public DatafeedTimingStats(String jobId) { + this(jobId, 0); + } + + public DatafeedTimingStats(StreamInput in) throws IOException { + jobId = in.readString(); + totalSearchTimeMs = in.readOptionalDouble(); + } + + public DatafeedTimingStats(DatafeedTimingStats other) { + this(other.jobId, other.totalSearchTimeMs); + } + + public String getJobId() { + return jobId; + } + + public double getTotalSearchTimeMs() { + return totalSearchTimeMs; + } + + public void incrementTotalSearchTimeMs(double totalSearchTimeMs) { + this.totalSearchTimeMs += totalSearchTimeMs; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(jobId); + out.writeOptionalDouble(totalSearchTimeMs); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { + builder.startObject(); + builder.field(JOB_ID.getPreferredName(), jobId); + builder.field(TOTAL_SEARCH_TIME_MS.getPreferredName(), totalSearchTimeMs); + builder.endObject(); + return builder; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + + DatafeedTimingStats other = (DatafeedTimingStats) obj; + return Objects.equals(this.jobId, other.jobId) + && Objects.equals(this.totalSearchTimeMs, other.totalSearchTimeMs); + } + + @Override + public int hashCode() { + return Objects.hash(jobId, totalSearchTimeMs); + } + + @Override + public String toString() { + return Strings.toString(this); + } + + /** + * Returns true if given stats objects differ from each other by more than 10% for at least one of the statistics. + */ + public static boolean differSignificantly(DatafeedTimingStats stats1, DatafeedTimingStats stats2) { + return differSignificantly(stats1.totalSearchTimeMs, stats2.totalSearchTimeMs); + } + + /** + * Returns {@code true} if one of the ratios { value1 / value2, value2 / value1 } is smaller than MIN_VALID_RATIO. + * This can be interpreted as values { value1, value2 } differing significantly from each other. + */ + private static boolean differSignificantly(double value1, double value2) { + return (value2 / value1 < MIN_VALID_RATIO) || (value1 / value2 < MIN_VALID_RATIO); + } + + /** + * Minimum ratio of values that is interpreted as values being similar. + * If the values ratio is less than MIN_VALID_RATIO, the values are interpreted as significantly different. + */ + private static final double MIN_VALID_RATIO = 0.9; +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/AnomalyDetectorsIndex.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/AnomalyDetectorsIndex.java index 7e61d42705a90..1c774f2c43577 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/AnomalyDetectorsIndex.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/AnomalyDetectorsIndex.java @@ -37,6 +37,14 @@ public static String jobResultsIndexPrefix() { return AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX; } + /** + * The name of the alias pointing to the indices where the jobs' results are stored + * @return The read alias + */ + public static String jobResultsIndexName() { + return AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT; + } + /** * The name of the alias pointing to the indices where the job's results are stored * @param jobId Job Id diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java index a44e1e53435c4..1f56c4c387cc7 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java @@ -18,6 +18,8 @@ import org.elasticsearch.xpack.core.ml.action.GetDatafeedsStatsAction.Response; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStatsTests; import java.io.IOException; import java.net.InetAddress; @@ -43,16 +45,13 @@ protected Response createTestInstance() { for (int j = 0; j < listSize; j++) { String datafeedId = randomAlphaOfLength(10); DatafeedState datafeedState = randomFrom(DatafeedState.values()); - - DiscoveryNode node = null; - if (randomBoolean()) { - node = new DiscoveryNode("_id", new TransportAddress(InetAddress.getLoopbackAddress(), 9300), Version.CURRENT); - } - String explanation = null; - if (randomBoolean()) { - explanation = randomAlphaOfLength(3); - } - Response.DatafeedStats datafeedStats = new Response.DatafeedStats(datafeedId, datafeedState, node, explanation); + DiscoveryNode node = + randomBoolean() + ? null + : new DiscoveryNode("_id", new TransportAddress(InetAddress.getLoopbackAddress(), 9300), Version.CURRENT); + String explanation = randomBoolean() ? null : randomAlphaOfLength(3); + DatafeedTimingStats timingStats = randomBoolean() ? null : DatafeedTimingStatsTests.createRandom(); + Response.DatafeedStats datafeedStats = new Response.DatafeedStats(datafeedId, datafeedState, node, explanation, timingStats); datafeedStatsList.add(datafeedStats); } @@ -78,7 +77,9 @@ public void testDatafeedStatsToXContent() throws IOException { Set.of(), Version.CURRENT); - Response.DatafeedStats stats = new Response.DatafeedStats("df-id", DatafeedState.STARTED, node, null); + DatafeedTimingStats timingStats = new DatafeedTimingStats("my-job-id", 123.456); + + Response.DatafeedStats stats = new Response.DatafeedStats("df-id", DatafeedState.STARTED, node, null, timingStats); XContentType xContentType = randomFrom(XContentType.values()); BytesReference bytes; @@ -89,7 +90,7 @@ public void testDatafeedStatsToXContent() throws IOException { Map dfStatsMap = XContentHelper.convertToMap(bytes, randomBoolean(), xContentType).v2(); - assertThat(dfStatsMap.size(), is(equalTo(3))); + assertThat(dfStatsMap.size(), is(equalTo(4))); assertThat(dfStatsMap, hasEntry("datafeed_id", "df-id")); assertThat(dfStatsMap, hasEntry("state", "started")); assertThat(dfStatsMap, hasKey("node")); @@ -105,5 +106,10 @@ public void testDatafeedStatsToXContent() throws IOException { assertThat(nodeAttributes.size(), is(equalTo(2))); assertThat(nodeAttributes, hasEntry("ml.enabled", "true")); assertThat(nodeAttributes, hasEntry("ml.max_open_jobs", "5")); + + Map timingStatsMap = (Map) dfStatsMap.get("timing_stats"); + assertThat(timingStatsMap.size(), is(equalTo(2))); + assertThat(timingStatsMap, hasEntry("job_id", "my-job-id")); + assertThat(timingStatsMap, hasEntry("total_search_time_ms", 123.456)); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java new file mode 100644 index 0000000000000..5c7b9414d9c8c --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java @@ -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; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.ml.datafeed; + +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractSerializingTestCase; + +import java.io.IOException; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +public class DatafeedTimingStatsTests extends AbstractSerializingTestCase { + + private static final String JOB_ID = "my-job-id"; + + public static DatafeedTimingStats createRandom() { + return new DatafeedTimingStats(randomAlphaOfLength(10), randomDouble()); + } + + @Override + protected DatafeedTimingStats createTestInstance(){ + return createRandom(); + } + + @Override + protected Writeable.Reader instanceReader() { + return DatafeedTimingStats::new; + } + + @Override + protected DatafeedTimingStats doParseInstance(XContentParser parser) { + return DatafeedTimingStats.PARSER.apply(parser, null); + } + + @Override + protected DatafeedTimingStats mutateInstance(DatafeedTimingStats instance) throws IOException { + String jobId = instance.getJobId(); + double totalSearchTimeMs = instance.getTotalSearchTimeMs(); + return new DatafeedTimingStats( + jobId + randomAlphaOfLength(5), + totalSearchTimeMs + randomDoubleBetween(1.0, 100.0, true)); + } + + public void testEquals() { + DatafeedTimingStats stats1 = new DatafeedTimingStats(JOB_ID, 100.0); + DatafeedTimingStats stats2 = new DatafeedTimingStats(JOB_ID, 100.0); + DatafeedTimingStats stats3 = new DatafeedTimingStats(JOB_ID, 200.0); + + assertTrue(stats1.equals(stats1)); + assertTrue(stats1.equals(stats2)); + assertFalse(stats2.equals(stats3)); + } + + public void testHashCode() { + DatafeedTimingStats stats1 = new DatafeedTimingStats(JOB_ID, 100.0); + DatafeedTimingStats stats2 = new DatafeedTimingStats(JOB_ID, 100.0); + DatafeedTimingStats stats3 = new DatafeedTimingStats(JOB_ID, 200.0); + + assertEquals(stats1.hashCode(), stats1.hashCode()); + assertEquals(stats1.hashCode(), stats2.hashCode()); + assertNotEquals(stats2.hashCode(), stats3.hashCode()); + } + + public void testConstructorsAndGetters() { + DatafeedTimingStats stats = new DatafeedTimingStats(JOB_ID, 123.456); + assertThat(stats.getJobId(), equalTo(JOB_ID)); + assertThat(stats.getTotalSearchTimeMs(), equalTo(123.456)); + + stats = new DatafeedTimingStats(JOB_ID); + assertThat(stats.getJobId(), equalTo(JOB_ID)); + assertThat(stats.getTotalSearchTimeMs(), equalTo(0.0)); + } + + public void testCopyConstructor() { + DatafeedTimingStats stats1 = new DatafeedTimingStats(JOB_ID, 123.456); + DatafeedTimingStats stats2 = new DatafeedTimingStats(stats1); + + assertThat(stats2.getJobId(), equalTo(JOB_ID)); + assertThat(stats2.getTotalSearchTimeMs(), equalTo(123.456)); + } + + public void testIncrementTotalSearchTimeMs() { + DatafeedTimingStats stats = new DatafeedTimingStats(JOB_ID, 100.0); + stats.incrementTotalSearchTimeMs(200.0); + assertThat(stats.getTotalSearchTimeMs(), equalTo(300.0)); + } + + public void testDocumentId() { + assertThat(DatafeedTimingStats.documentId("my-job-id"), equalTo("my-job-id_datafeed_timing_stats")); + } + + public void testTimingStatsDifferSignificantly() { + assertThat( + DatafeedTimingStats.differSignificantly(new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1000.0)), + is(false)); + assertThat( + DatafeedTimingStats.differSignificantly(new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1100.0)), + is(false)); + assertThat( + DatafeedTimingStats.differSignificantly(new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1120.0)), + is(true)); + } +} diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java index 3976a0e578f16..786e0385a8189 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java @@ -448,12 +448,15 @@ public Collection createComponents(Client client, ClusterService cluster Auditor auditor = new Auditor(client, clusterService.getNodeName()); JobResultsProvider jobResultsProvider = new JobResultsProvider(client, settings); + JobResultsPersister jobResultsPersister = new JobResultsPersister(client); + JobDataCountsPersister jobDataCountsPersister = new JobDataCountsPersister(client); JobConfigProvider jobConfigProvider = new JobConfigProvider(client, xContentRegistry); DatafeedConfigProvider datafeedConfigProvider = new DatafeedConfigProvider(client, xContentRegistry); UpdateJobProcessNotifier notifier = new UpdateJobProcessNotifier(client, clusterService, threadPool); JobManager jobManager = new JobManager(env, settings, jobResultsProvider, + jobResultsPersister, clusterService, auditor, threadPool, @@ -464,9 +467,6 @@ public Collection createComponents(Client client, ClusterService cluster // special holder for @link(MachineLearningFeatureSetUsage) which needs access to job manager if ML is enabled JobManagerHolder jobManagerHolder = new JobManagerHolder(jobManager); - JobDataCountsPersister jobDataCountsPersister = new JobDataCountsPersister(client); - JobResultsPersister jobResultsPersister = new JobResultsPersister(client); - NativeStorageProvider nativeStorageProvider = new NativeStorageProvider(environment, MIN_DISK_SPACE_OFF_HEAP.get(settings)); MlController mlController; @@ -509,8 +509,16 @@ public Collection createComponents(Client client, ClusterService cluster xContentRegistry, auditor, clusterService, jobManager, jobResultsProvider, jobResultsPersister, jobDataCountsPersister, autodetectProcessFactory, normalizerFactory, nativeStorageProvider); this.autodetectProcessManager.set(autodetectProcessManager); - DatafeedJobBuilder datafeedJobBuilder = new DatafeedJobBuilder(client, settings, xContentRegistry, - auditor, System::currentTimeMillis); + DatafeedJobBuilder datafeedJobBuilder = + new DatafeedJobBuilder( + client, + xContentRegistry, + auditor, + System::currentTimeMillis, + jobConfigProvider, + jobResultsProvider, + datafeedConfigProvider, + jobResultsPersister); DatafeedManager datafeedManager = new DatafeedManager(threadPool, client, clusterService, datafeedJobBuilder, System::currentTimeMillis, auditor, autodetectProcessManager); this.datafeedManager.set(datafeedManager); @@ -542,6 +550,7 @@ public Collection createComponents(Client client, ClusterService cluster mlLifeCycleService, new MlControllerHolder(mlController), jobResultsProvider, + jobResultsPersister, jobConfigProvider, datafeedConfigProvider, jobManager, diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java index 6c90b15dab13b..bd0432f890abc 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java @@ -24,24 +24,29 @@ import org.elasticsearch.xpack.core.action.util.QueryPage; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.ml.datafeed.persistence.DatafeedConfigProvider; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsProvider; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; public class TransportGetDatafeedsStatsAction extends TransportMasterNodeReadAction { private final DatafeedConfigProvider datafeedConfigProvider; + private final JobResultsProvider jobResultsProvider; @Inject public TransportGetDatafeedsStatsAction(TransportService transportService, ClusterService clusterService, ThreadPool threadPool, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver, - DatafeedConfigProvider datafeedConfigProvider) { + DatafeedConfigProvider datafeedConfigProvider, JobResultsProvider jobResultsProvider) { super(GetDatafeedsStatsAction.NAME, transportService, clusterService, threadPool, actionFilters, GetDatafeedsStatsAction.Request::new, indexNameExpressionResolver); this.datafeedConfigProvider = datafeedConfigProvider; + this.jobResultsProvider = jobResultsProvider; } @Override @@ -59,22 +64,37 @@ protected void masterOperation(Task task, GetDatafeedsStatsAction.Request reques ActionListener listener) throws Exception { logger.debug("Get stats for datafeed '{}'", request.getDatafeedId()); - datafeedConfigProvider.expandDatafeedIds(request.getDatafeedId(), request.allowNoDatafeeds(), ActionListener.wrap( - expandedDatafeedIds -> { - PersistentTasksCustomMetaData tasksInProgress = state.getMetaData().custom(PersistentTasksCustomMetaData.TYPE); - List results = expandedDatafeedIds.stream() - .map(datafeedId -> getDatafeedStats(datafeedId, state, tasksInProgress)) - .collect(Collectors.toList()); - QueryPage statsPage = new QueryPage<>(results, results.size(), - DatafeedConfig.RESULTS_FIELD); - listener.onResponse(new GetDatafeedsStatsAction.Response(statsPage)); + datafeedConfigProvider.expandDatafeedConfigs( + request.getDatafeedId(), + request.allowNoDatafeeds(), + ActionListener.wrap( + datafeedBuilders -> { + Map jobIdByDatafeedId = datafeedBuilders.stream() + .map(DatafeedConfig.Builder::build) + .collect(Collectors.toUnmodifiableMap(DatafeedConfig::getId, DatafeedConfig::getJobId)); + jobResultsProvider.datafeedTimingStats( + jobIdByDatafeedId.values(), + timingStatsByJobId -> { + PersistentTasksCustomMetaData tasksInProgress = state.getMetaData().custom(PersistentTasksCustomMetaData.TYPE); + List results = datafeedBuilders.stream() + .map(datafeedBuilder -> getDatafeedStats( + datafeedBuilder.getId(), state, tasksInProgress, jobIdByDatafeedId, timingStatsByJobId)) + .collect(Collectors.toList()); + QueryPage statsPage = + new QueryPage<>(results, results.size(), DatafeedConfig.RESULTS_FIELD); + listener.onResponse(new GetDatafeedsStatsAction.Response(statsPage)); + }, + listener::onFailure); }, - listener::onFailure - )); + listener::onFailure) + ); } - private static GetDatafeedsStatsAction.Response.DatafeedStats getDatafeedStats(String datafeedId, ClusterState state, - PersistentTasksCustomMetaData tasks) { + private static GetDatafeedsStatsAction.Response.DatafeedStats getDatafeedStats(String datafeedId, + ClusterState state, + PersistentTasksCustomMetaData tasks, + Map jobIdByDatafeedId, + Map timingStatsByJobId) { PersistentTasksCustomMetaData.PersistentTask task = MlTasks.getDatafeedTask(datafeedId, tasks); DatafeedState datafeedState = MlTasks.getDatafeedState(datafeedId, tasks); DiscoveryNode node = null; @@ -83,7 +103,15 @@ private static GetDatafeedsStatsAction.Response.DatafeedStats getDatafeedStats(S node = state.nodes().get(task.getExecutorNode()); explanation = task.getAssignment().getExplanation(); } - return new GetDatafeedsStatsAction.Response.DatafeedStats(datafeedId, datafeedState, node, explanation); + DatafeedTimingStats timingStats = null; + String jobId = jobIdByDatafeedId.get(datafeedId); + if (jobId != null) { + timingStats = timingStatsByJobId.get(jobId); + if (timingStats == null) { + timingStats = new DatafeedTimingStats(jobId); + } + } + return new GetDatafeedsStatsAction.Response.DatafeedStats(datafeedId, datafeedState, node, explanation, timingStats); } @Override diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPreviewDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPreviewDatafeedAction.java index 89ad54e9c1802..d2d2ffd4c07ce 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPreviewDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPreviewDatafeedAction.java @@ -20,14 +20,18 @@ import org.elasticsearch.xpack.core.ml.datafeed.ChunkingConfig; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; import org.elasticsearch.xpack.core.ml.datafeed.extractor.DataExtractor; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; import org.elasticsearch.xpack.ml.datafeed.persistence.DatafeedConfigProvider; import org.elasticsearch.xpack.ml.job.persistence.JobConfigProvider; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsProvider; import java.io.BufferedReader; import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import java.time.Clock; import java.util.Map; import java.util.Optional; import java.util.function.Supplier; @@ -39,56 +43,68 @@ public class TransportPreviewDatafeedAction extends HandledTransportAction) PreviewDatafeedAction.Request::new); this.threadPool = threadPool; this.client = client; this.jobConfigProvider = jobConfigProvider; this.datafeedConfigProvider = datafeedConfigProvider; + this.jobResultsProvider = jobResultsProvider; + this.jobResultsPersister = jobResultsPersister; this.xContentRegistry = xContentRegistry; } @Override protected void doExecute(Task task, PreviewDatafeedAction.Request request, ActionListener listener) { - datafeedConfigProvider.getDatafeedConfig(request.getDatafeedId(), ActionListener.wrap( - datafeedConfigBuilder -> { - DatafeedConfig datafeedConfig = datafeedConfigBuilder.build(); - jobConfigProvider.getJob(datafeedConfig.getJobId(), ActionListener.wrap( - jobBuilder -> { - DatafeedConfig.Builder previewDatafeed = buildPreviewDatafeed(datafeedConfig); - Map headers = threadPool.getThreadContext().getHeaders().entrySet().stream() - .filter(e -> ClientHelper.SECURITY_HEADER_FILTERS.contains(e.getKey())) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - previewDatafeed.setHeaders(headers); + datafeedConfigBuilder -> { + DatafeedConfig datafeedConfig = datafeedConfigBuilder.build(); + jobConfigProvider.getJob(datafeedConfig.getJobId(), ActionListener.wrap( + jobBuilder -> { + DatafeedConfig.Builder previewDatafeed = buildPreviewDatafeed(datafeedConfig); + Map headers = threadPool.getThreadContext().getHeaders().entrySet().stream() + .filter(e -> ClientHelper.SECURITY_HEADER_FILTERS.contains(e.getKey())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + previewDatafeed.setHeaders(headers); + jobResultsProvider.datafeedTimingStats( + jobBuilder.getId(), + timingStats -> { // NB: this is using the client from the transport layer, NOT the internal client. // This is important because it means the datafeed search will fail if the user // requesting the preview doesn't have permission to search the relevant indices. - DataExtractorFactory.create(client, previewDatafeed.build(), jobBuilder.build(), xContentRegistry, - new ActionListener() { - @Override - public void onResponse(DataExtractorFactory dataExtractorFactory) { - DataExtractor dataExtractor = dataExtractorFactory.newExtractor(0, Long.MAX_VALUE); - threadPool.generic().execute(() -> previewDatafeed(dataExtractor, listener)); - } + DataExtractorFactory.create( + client, + previewDatafeed.build(), + jobBuilder.build(), + xContentRegistry, + new DatafeedTimingStatsReporter(timingStats, Clock.systemUTC(), jobResultsPersister), + new ActionListener<>() { + @Override + public void onResponse(DataExtractorFactory dataExtractorFactory) { + DataExtractor dataExtractor = dataExtractorFactory.newExtractor(0, Long.MAX_VALUE); + threadPool.generic().execute(() -> previewDatafeed(dataExtractor, listener)); + } - @Override - public void onFailure(Exception e) { - listener.onFailure(e); - } - }); + @Override + public void onFailure(Exception e) { + listener.onFailure(e); + } + }); }, - listener::onFailure - )); - }, - listener::onFailure - )); + listener::onFailure); + }, + listener::onFailure)); + }, + listener::onFailure)); } /** Visible for testing */ diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java index bbb7eb3c9a969..e79c2eecc47b9 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java @@ -44,6 +44,7 @@ import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedJobValidator; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.JobState; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; @@ -51,12 +52,15 @@ import org.elasticsearch.xpack.ml.MlConfigMigrationEligibilityCheck; import org.elasticsearch.xpack.ml.datafeed.DatafeedManager; import org.elasticsearch.xpack.ml.datafeed.DatafeedNodeSelector; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; import org.elasticsearch.xpack.ml.datafeed.persistence.DatafeedConfigProvider; import org.elasticsearch.xpack.ml.job.persistence.JobConfigProvider; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.elasticsearch.xpack.ml.notifications.Auditor; import java.io.IOException; +import java.time.Clock; import java.util.ArrayList; import java.util.List; import java.util.Locale; @@ -80,6 +84,7 @@ public class TransportStartDatafeedAction extends TransportMasterNodeAction> listener) { - DataExtractorFactory.create(client, datafeed, job, xContentRegistry, ActionListener.wrap( - dataExtractorFactory -> - persistentTasksService.sendStartRequest(MlTasks.datafeedTaskId(params.getDatafeedId()), - MlTasks.DATAFEED_TASK_NAME, params, listener) - , listener::onFailure)); + DataExtractorFactory.create( + client, + datafeed, + job, + xContentRegistry, + // Creating fake {@link TimingStatsReporter} so that search API call is not needed. + new DatafeedTimingStatsReporter(new DatafeedTimingStats(job.getId()), Clock.systemUTC(), jobResultsPersister), + ActionListener.wrap( + unused -> + persistentTasksService.sendStartRequest( + MlTasks.datafeedTaskId(params.getDatafeedId()), MlTasks.DATAFEED_TASK_NAME, params, listener), + listener::onFailure)); } @Override diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilder.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilder.java index db862286ca987..66687a270ca3d 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilder.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilder.java @@ -8,12 +8,12 @@ import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Client; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.xpack.core.action.util.QueryPage; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedJobValidator; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.config.DataDescription; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.DataCounts; @@ -25,9 +25,11 @@ import org.elasticsearch.xpack.ml.datafeed.persistence.DatafeedConfigProvider; import org.elasticsearch.xpack.ml.job.persistence.BucketsQueryBuilder; import org.elasticsearch.xpack.ml.job.persistence.JobConfigProvider; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.elasticsearch.xpack.ml.job.persistence.JobResultsProvider; import org.elasticsearch.xpack.ml.notifications.Auditor; +import java.time.Clock; import java.util.Collections; import java.util.Objects; import java.util.concurrent.atomic.AtomicReference; @@ -37,36 +39,28 @@ public class DatafeedJobBuilder { private final Client client; - private final Settings settings; private final NamedXContentRegistry xContentRegistry; private final Auditor auditor; private final Supplier currentTimeSupplier; - - public DatafeedJobBuilder(Client client, Settings settings, NamedXContentRegistry xContentRegistry, - Auditor auditor, Supplier currentTimeSupplier) { + private final JobConfigProvider jobConfigProvider; + private final JobResultsProvider jobResultsProvider; + private final DatafeedConfigProvider datafeedConfigProvider; + private final JobResultsPersister jobResultsPersister; + + public DatafeedJobBuilder(Client client, NamedXContentRegistry xContentRegistry, Auditor auditor, Supplier currentTimeSupplier, + JobConfigProvider jobConfigProvider, JobResultsProvider jobResultsProvider, + DatafeedConfigProvider datafeedConfigProvider, JobResultsPersister jobResultsPersister) { this.client = client; - this.settings = Objects.requireNonNull(settings); this.xContentRegistry = Objects.requireNonNull(xContentRegistry); this.auditor = Objects.requireNonNull(auditor); this.currentTimeSupplier = Objects.requireNonNull(currentTimeSupplier); + this.jobConfigProvider = Objects.requireNonNull(jobConfigProvider); + this.jobResultsProvider = Objects.requireNonNull(jobResultsProvider); + this.datafeedConfigProvider = Objects.requireNonNull(datafeedConfigProvider); + this.jobResultsPersister = Objects.requireNonNull(jobResultsPersister); } void build(String datafeedId, ActionListener listener) { - - JobResultsProvider jobResultsProvider = new JobResultsProvider(client, settings); - JobConfigProvider jobConfigProvider = new JobConfigProvider(client, xContentRegistry); - DatafeedConfigProvider datafeedConfigProvider = new DatafeedConfigProvider(client, xContentRegistry); - - build(datafeedId, jobResultsProvider, jobConfigProvider, datafeedConfigProvider, listener); - } - - /** - * For testing only. - * Use {@link #build(String, ActionListener)} instead - */ - void build(String datafeedId, JobResultsProvider jobResultsProvider, JobConfigProvider jobConfigProvider, - DatafeedConfigProvider datafeedConfigProvider, ActionListener listener) { - AtomicReference jobHolder = new AtomicReference<>(); AtomicReference datafeedConfigHolder = new AtomicReference<>(); @@ -98,11 +92,21 @@ void build(String datafeedId, JobResultsProvider jobResultsProvider, JobConfigPr ); // Create data extractor factory + Consumer datafeedTimingStatsHandler = timingStats -> { + DataExtractorFactory.create( + client, + datafeedConfigHolder.get(), + jobHolder.get(), + xContentRegistry, + new DatafeedTimingStatsReporter(timingStats, Clock.systemUTC(), jobResultsPersister), + dataExtractorFactoryHandler); + }; + Consumer dataCountsHandler = dataCounts -> { if (dataCounts.getLatestRecordTimeStamp() != null) { context.latestRecordTimeMs = dataCounts.getLatestRecordTimeStamp().getTime(); } - DataExtractorFactory.create(client, datafeedConfigHolder.get(), jobHolder.get(), xContentRegistry, dataExtractorFactoryHandler); + jobResultsProvider.datafeedTimingStats(jobHolder.get().getId(), datafeedTimingStatsHandler, listener::onFailure); }; // Collect data counts @@ -118,7 +122,8 @@ void build(String datafeedId, JobResultsProvider jobResultsProvider, JobConfigPr Consumer jobIdConsumer = jobId -> { BucketsQueryBuilder latestBucketQuery = new BucketsQueryBuilder() .sortField(Result.TIMESTAMP.getPreferredName()) - .sortDescending(true).size(1) + .sortDescending(true) + .size(1) .includeInterim(false); jobResultsProvider.bucketsViaInternalClient(jobId, latestBucketQuery, bucketsHandler, e -> { if (e instanceof ResourceNotFoundException) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java new file mode 100644 index 0000000000000..4ebf90ad4a587 --- /dev/null +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java @@ -0,0 +1,57 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.ml.datafeed; + +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; + +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.util.Objects; +import java.util.function.Function; + +public class DatafeedTimingStatsReporter { + + private final Clock clock; + private final JobResultsPersister jobResultsPersister; + private DatafeedTimingStats persistedTimingStats; + private volatile DatafeedTimingStats currentTimingStats; + + public DatafeedTimingStatsReporter(DatafeedTimingStats timingStats, Clock clock, JobResultsPersister jobResultsPersister) { + Objects.requireNonNull(timingStats); + this.persistedTimingStats = new DatafeedTimingStats(timingStats); + this.currentTimingStats = new DatafeedTimingStats(timingStats); + this.clock = Objects.requireNonNull(clock); + this.jobResultsPersister = Objects.requireNonNull(jobResultsPersister); + } + + public DatafeedTimingStats getCurrentTimingStats() { + return new DatafeedTimingStats(currentTimingStats); + } + + /** + * Executes the given function and reports how much time did the execution take. + */ + public R executeWithReporting(Function function, T argument) { + Instant before = clock.instant(); + R result = function.apply(argument); + Instant after = clock.instant(); + Duration duration = Duration.between(before, after); + reportSearchDuration(duration); + return result; + } + + private void reportSearchDuration(Duration searchDuration) { + double searchDurationMs = searchDuration.toMillis(); + currentTimingStats.incrementTotalSearchTimeMs(searchDurationMs); + if (DatafeedTimingStats.differSignificantly(currentTimingStats, persistedTimingStats)) { + jobResultsPersister.persistDatafeedTimingStats(currentTimingStats); + persistedTimingStats = currentTimingStats; + currentTimingStats = new DatafeedTimingStats(persistedTimingStats); + } + } +} diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactory.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactory.java index 948cea5de85d7..40e819affa087 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactory.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactory.java @@ -16,9 +16,10 @@ import org.elasticsearch.xpack.core.ml.datafeed.extractor.DataExtractor; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.rollup.action.GetRollupIndexCapsAction; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.aggregation.AggregationDataExtractorFactory; -import org.elasticsearch.xpack.ml.datafeed.extractor.chunked.ChunkedDataExtractorFactory; import org.elasticsearch.xpack.ml.datafeed.extractor.aggregation.RollupDataExtractorFactory; +import org.elasticsearch.xpack.ml.datafeed.extractor.chunked.ChunkedDataExtractorFactory; import org.elasticsearch.xpack.ml.datafeed.extractor.scroll.ScrollDataExtractorFactory; public interface DataExtractorFactory { @@ -31,10 +32,11 @@ static void create(Client client, DatafeedConfig datafeed, Job job, NamedXContentRegistry xContentRegistry, + DatafeedTimingStatsReporter timingStatsReporter, ActionListener listener) { ActionListener factoryHandler = ActionListener.wrap( factory -> listener.onResponse(datafeed.getChunkingConfig().isEnabled() - ? new ChunkedDataExtractorFactory(client, datafeed, job, xContentRegistry, factory) : factory) + ? new ChunkedDataExtractorFactory(client, datafeed, job, xContentRegistry, factory, timingStatsReporter) : factory) , listener::onFailure ); @@ -42,13 +44,15 @@ static void create(Client client, response -> { if (response.getJobs().isEmpty()) { // This means no rollup indexes are in the config if (datafeed.hasAggregations()) { - factoryHandler.onResponse(new AggregationDataExtractorFactory(client, datafeed, job, xContentRegistry)); + factoryHandler.onResponse( + new AggregationDataExtractorFactory(client, datafeed, job, xContentRegistry, timingStatsReporter)); } else { - ScrollDataExtractorFactory.create(client, datafeed, job, xContentRegistry, factoryHandler); + ScrollDataExtractorFactory.create(client, datafeed, job, xContentRegistry, timingStatsReporter, factoryHandler); } } else { if (datafeed.hasAggregations()) { // Rollup indexes require aggregations - RollupDataExtractorFactory.create(client, datafeed, job, response.getJobs(), xContentRegistry, factoryHandler); + RollupDataExtractorFactory.create( + client, datafeed, job, response.getJobs(), xContentRegistry, timingStatsReporter, factoryHandler); } else { listener.onFailure(new IllegalArgumentException("Aggregations are required when using Rollup indices")); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AbstractAggregationDataExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AbstractAggregationDataExtractor.java index aa5c7ed6314b4..af94b1f8f4f43 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AbstractAggregationDataExtractor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AbstractAggregationDataExtractor.java @@ -18,6 +18,7 @@ import org.elasticsearch.xpack.core.ClientHelper; import org.elasticsearch.xpack.core.ml.datafeed.extractor.DataExtractor; import org.elasticsearch.xpack.core.ml.datafeed.extractor.ExtractorUtils; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -50,17 +51,20 @@ abstract class AbstractAggregationDataExtractor next() throws IOException { private Aggregations search() throws IOException { LOGGER.debug("[{}] Executing aggregated search", context.jobId); - SearchResponse searchResponse = executeSearchRequest(buildSearchRequest(buildBaseSearchSource())); + SearchResponse searchResponse = executeSearchRequestWithReporting(buildSearchRequest(buildBaseSearchSource())); LOGGER.debug("[{}] Search response was obtained", context.jobId); ExtractorUtils.checkSearchWasSuccessful(context.jobId, searchResponse); return validateAggs(searchResponse.getAggregations()); @@ -117,6 +121,10 @@ private void initAggregationProcessor(Aggregations aggs) throws IOException { aggregationToJsonProcessor.process(aggs); } + private SearchResponse executeSearchRequestWithReporting(T searchRequestBuilder) { + return timingStatsReporter.executeWithReporting(this::executeSearchRequest, searchRequestBuilder); + } + protected SearchResponse executeSearchRequest(T searchRequestBuilder) { return ClientHelper.executeWithHeaders(context.headers, ClientHelper.ML_ORIGIN, client, searchRequestBuilder::get); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractor.java index 74f05851c7d8a..031db35ebac15 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractor.java @@ -9,6 +9,7 @@ import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.client.Client; import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; /** * An implementation that extracts data from elasticsearch using search with aggregations on a client. @@ -18,8 +19,9 @@ */ class AggregationDataExtractor extends AbstractAggregationDataExtractor { - AggregationDataExtractor(Client client, AggregationDataExtractorContext dataExtractorContext) { - super(client, dataExtractorContext); + AggregationDataExtractor( + Client client, AggregationDataExtractorContext dataExtractorContext, DatafeedTimingStatsReporter timingStatsReporter) { + super(client, dataExtractorContext, timingStatsReporter); } @Override diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorFactory.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorFactory.java index de205b276a049..40d186542df04 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorFactory.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorFactory.java @@ -9,9 +9,10 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; import org.elasticsearch.xpack.core.ml.datafeed.extractor.DataExtractor; -import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.utils.Intervals; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; +import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; import java.util.Objects; @@ -21,12 +22,19 @@ public class AggregationDataExtractorFactory implements DataExtractorFactory { private final DatafeedConfig datafeedConfig; private final Job job; private final NamedXContentRegistry xContentRegistry; + private final DatafeedTimingStatsReporter timingStatsReporter; - public AggregationDataExtractorFactory(Client client, DatafeedConfig datafeedConfig, Job job, NamedXContentRegistry xContentRegistry) { + public AggregationDataExtractorFactory( + Client client, + DatafeedConfig datafeedConfig, + Job job, + NamedXContentRegistry xContentRegistry, + DatafeedTimingStatsReporter timingStatsReporter) { this.client = Objects.requireNonNull(client); this.datafeedConfig = Objects.requireNonNull(datafeedConfig); this.job = Objects.requireNonNull(job); this.xContentRegistry = xContentRegistry; + this.timingStatsReporter = Objects.requireNonNull(timingStatsReporter); } @Override @@ -43,6 +51,6 @@ public DataExtractor newExtractor(long start, long end) { Intervals.alignToFloor(end, histogramInterval), job.getAnalysisConfig().getSummaryCountFieldName().equals(DatafeedConfig.DOC_COUNT), datafeedConfig.getHeaders()); - return new AggregationDataExtractor(client, dataExtractorContext); + return new AggregationDataExtractor(client, dataExtractorContext, timingStatsReporter); } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractor.java index f5de574e99a96..9341161057717 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractor.java @@ -9,6 +9,7 @@ import org.elasticsearch.client.Client; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.xpack.core.rollup.action.RollupSearchAction; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; /** * An implementation that extracts data from elasticsearch using search with aggregations against rollup indexes on a client. @@ -18,8 +19,9 @@ */ class RollupDataExtractor extends AbstractAggregationDataExtractor { - RollupDataExtractor(Client client, AggregationDataExtractorContext dataExtractorContext) { - super(client, dataExtractorContext); + RollupDataExtractor( + Client client, AggregationDataExtractorContext dataExtractorContext, DatafeedTimingStatsReporter timingStatsReporter) { + super(client, dataExtractorContext, timingStatsReporter); } @Override @@ -28,5 +30,4 @@ protected RollupSearchAction.RequestBuilder buildSearchRequest(SearchSourceBuild return new RollupSearchAction.RequestBuilder(client, searchRequest); } - } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractorFactory.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractorFactory.java index 7a66ff49d62af..6c27fba4156f6 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractorFactory.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/RollupDataExtractorFactory.java @@ -21,6 +21,7 @@ import org.elasticsearch.xpack.core.rollup.action.RollableIndexCaps; import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps.RollupFieldCaps; import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; import java.time.ZoneId; @@ -45,12 +46,19 @@ public class RollupDataExtractorFactory implements DataExtractorFactory { private final DatafeedConfig datafeedConfig; private final Job job; private final NamedXContentRegistry xContentRegistry; - - private RollupDataExtractorFactory(Client client, DatafeedConfig datafeedConfig, Job job, NamedXContentRegistry xContentRegistry) { + private final DatafeedTimingStatsReporter timingStatsReporter; + + private RollupDataExtractorFactory( + Client client, + DatafeedConfig datafeedConfig, + Job job, + NamedXContentRegistry xContentRegistry, + DatafeedTimingStatsReporter timingStatsReporter) { this.client = Objects.requireNonNull(client); this.datafeedConfig = Objects.requireNonNull(datafeedConfig); this.job = Objects.requireNonNull(job); this.xContentRegistry = xContentRegistry; + this.timingStatsReporter = Objects.requireNonNull(timingStatsReporter); } @Override @@ -67,7 +75,7 @@ public DataExtractor newExtractor(long start, long end) { Intervals.alignToFloor(end, histogramInterval), job.getAnalysisConfig().getSummaryCountFieldName().equals(DatafeedConfig.DOC_COUNT), datafeedConfig.getHeaders()); - return new RollupDataExtractor(client, dataExtractorContext); + return new RollupDataExtractor(client, dataExtractorContext, timingStatsReporter); } public static void create(Client client, @@ -75,6 +83,7 @@ public static void create(Client client, Job job, Map rollupJobsWithCaps, NamedXContentRegistry xContentRegistry, + DatafeedTimingStatsReporter timingStatsReporter, ActionListener listener) { final AggregationBuilder datafeedHistogramAggregation = getHistogramAggregation( @@ -119,7 +128,7 @@ public static void create(Client client, return; } - listener.onResponse(new RollupDataExtractorFactory(client, datafeed, job, xContentRegistry)); + listener.onResponse(new RollupDataExtractorFactory(client, datafeed, job, xContentRegistry, timingStatsReporter)); } private static boolean validInterval(long datafeedInterval, ParsedRollupCaps rollupJobGroupConfig) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java index f1e1fe2a10a32..ad3d6c90e385e 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java @@ -23,6 +23,7 @@ import org.elasticsearch.xpack.core.ml.datafeed.extractor.DataExtractor; import org.elasticsearch.xpack.core.ml.datafeed.extractor.ExtractorUtils; import org.elasticsearch.xpack.core.rollup.action.RollupSearchAction; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; import org.elasticsearch.xpack.ml.datafeed.extractor.aggregation.RollupDataExtractorFactory; @@ -68,16 +69,22 @@ private interface DataSummary { private final DataExtractorFactory dataExtractorFactory; private final ChunkedDataExtractorContext context; private final DataSummaryFactory dataSummaryFactory; + private final DatafeedTimingStatsReporter timingStatsReporter; private long currentStart; private long currentEnd; private long chunkSpan; private boolean isCancelled; private DataExtractor currentExtractor; - public ChunkedDataExtractor(Client client, DataExtractorFactory dataExtractorFactory, ChunkedDataExtractorContext context) { + public ChunkedDataExtractor( + Client client, + DataExtractorFactory dataExtractorFactory, + ChunkedDataExtractorContext context, + DatafeedTimingStatsReporter timingStatsReporter) { this.client = Objects.requireNonNull(client); this.dataExtractorFactory = Objects.requireNonNull(dataExtractorFactory); this.context = Objects.requireNonNull(context); + this.timingStatsReporter = Objects.requireNonNull(timingStatsReporter); this.currentStart = context.start; this.currentEnd = context.start; this.isCancelled = false; @@ -122,6 +129,10 @@ private void setUpChunkedSearch() throws IOException { } } + private SearchResponse executeSearchRequestWithReporting(ActionRequestBuilder searchRequestBuilder) { + return timingStatsReporter.executeWithReporting(this::executeSearchRequest, searchRequestBuilder); + } + protected SearchResponse executeSearchRequest(ActionRequestBuilder searchRequestBuilder) { return ClientHelper.executeWithHeaders(context.headers, ClientHelper.ML_ORIGIN, client, searchRequestBuilder::get); } @@ -198,15 +209,15 @@ private DataSummary buildDataSummary() throws IOException { private DataSummary newScrolledDataSummary() throws IOException { SearchRequestBuilder searchRequestBuilder = rangeSearchRequest(); - SearchResponse response = executeSearchRequest(searchRequestBuilder); + SearchResponse searchResponse = executeSearchRequestWithReporting(searchRequestBuilder); LOGGER.debug("[{}] Scrolling Data summary response was obtained", context.jobId); - ExtractorUtils.checkSearchWasSuccessful(context.jobId, response); + ExtractorUtils.checkSearchWasSuccessful(context.jobId, searchResponse); - Aggregations aggregations = response.getAggregations(); + Aggregations aggregations = searchResponse.getAggregations(); long earliestTime = 0; long latestTime = 0; - long totalHits = response.getHits().getTotalHits().value; + long totalHits = searchResponse.getHits().getTotalHits().value; if (totalHits > 0) { Min min = aggregations.get(EARLIEST_TIME); earliestTime = (long) min.getValue(); @@ -220,7 +231,7 @@ private DataSummary newAggregatedDataSummary() throws IOException { // TODO: once RollupSearchAction is changed from indices:admin* to indices:data/read/* this branch is not needed ActionRequestBuilder searchRequestBuilder = dataExtractorFactory instanceof RollupDataExtractorFactory ? rollupRangeSearchRequest() : rangeSearchRequest(); - SearchResponse response = executeSearchRequest(searchRequestBuilder); + SearchResponse response = executeSearchRequestWithReporting(searchRequestBuilder); LOGGER.debug("[{}] Aggregating Data summary response was obtained", context.jobId); ExtractorUtils.checkSearchWasSuccessful(context.jobId, response); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorFactory.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorFactory.java index fb8da71faa3fe..028409f69398b 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorFactory.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorFactory.java @@ -9,6 +9,7 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; import org.elasticsearch.xpack.core.ml.datafeed.extractor.DataExtractor; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.utils.Intervals; @@ -22,17 +23,20 @@ public class ChunkedDataExtractorFactory implements DataExtractorFactory { private final Job job; private final DataExtractorFactory dataExtractorFactory; private final NamedXContentRegistry xContentRegistry; + private final DatafeedTimingStatsReporter timingStatsReporter; public ChunkedDataExtractorFactory(Client client, DatafeedConfig datafeedConfig, Job job, NamedXContentRegistry xContentRegistry, - DataExtractorFactory dataExtractorFactory) { + DataExtractorFactory dataExtractorFactory, + DatafeedTimingStatsReporter timingStatsReporter) { this.client = Objects.requireNonNull(client); this.datafeedConfig = Objects.requireNonNull(datafeedConfig); this.job = Objects.requireNonNull(job); this.dataExtractorFactory = Objects.requireNonNull(dataExtractorFactory); this.xContentRegistry = xContentRegistry; + this.timingStatsReporter = Objects.requireNonNull(timingStatsReporter); } @Override @@ -52,7 +56,7 @@ public DataExtractor newExtractor(long start, long end) { datafeedConfig.hasAggregations(), datafeedConfig.hasAggregations() ? datafeedConfig.getHistogramIntervalMillis(xContentRegistry) : null ); - return new ChunkedDataExtractor(client, dataExtractorFactory, dataExtractorContext); + return new ChunkedDataExtractor(client, dataExtractorFactory, dataExtractorContext, timingStatsReporter); } private ChunkedDataExtractorContext.TimeAligner newTimeAligner() { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractor.java index dea775c24ca36..26ca38913221c 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractor.java @@ -23,6 +23,7 @@ import org.elasticsearch.xpack.core.ClientHelper; import org.elasticsearch.xpack.core.ml.datafeed.extractor.DataExtractor; import org.elasticsearch.xpack.core.ml.datafeed.extractor.ExtractorUtils; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.fields.ExtractedField; import java.io.ByteArrayInputStream; @@ -47,6 +48,7 @@ class ScrollDataExtractor implements DataExtractor { private final Client client; private final ScrollDataExtractorContext context; + private final DatafeedTimingStatsReporter timingStatsReporter; private String scrollId; private boolean isCancelled; private boolean hasNext; @@ -54,9 +56,10 @@ class ScrollDataExtractor implements DataExtractor { protected Long lastTimestamp; private boolean searchHasShardFailure; - ScrollDataExtractor(Client client, ScrollDataExtractorContext dataExtractorContext) { + ScrollDataExtractor(Client client, ScrollDataExtractorContext dataExtractorContext, DatafeedTimingStatsReporter timingStatsReporter) { this.client = Objects.requireNonNull(client); - context = Objects.requireNonNull(dataExtractorContext); + this.context = Objects.requireNonNull(dataExtractorContext); + this.timingStatsReporter = Objects.requireNonNull(timingStatsReporter); hasNext = true; searchHasShardFailure = false; } @@ -107,11 +110,15 @@ private Optional tryNextStream() throws IOException { protected InputStream initScroll(long startTimestamp) throws IOException { LOGGER.debug("[{}] Initializing scroll", context.jobId); - SearchResponse searchResponse = executeSearchRequest(buildSearchRequest(startTimestamp)); + SearchResponse searchResponse = executeSearchRequestWithReporting(buildSearchRequest(startTimestamp)); LOGGER.debug("[{}] Search response was obtained", context.jobId); return processSearchResponse(searchResponse); } + private SearchResponse executeSearchRequestWithReporting(SearchRequestBuilder searchRequestBuilder) { + return timingStatsReporter.executeWithReporting(this::executeSearchRequest, searchRequestBuilder); + } + protected SearchResponse executeSearchRequest(SearchRequestBuilder searchRequestBuilder) { return ClientHelper.executeWithHeaders(context.headers, ClientHelper.ML_ORIGIN, client, searchRequestBuilder::get); } @@ -183,12 +190,13 @@ private InputStream continueScroll() throws IOException { LOGGER.debug("[{}] Continuing scroll with id [{}]", context.jobId, scrollId); SearchResponse searchResponse; try { - searchResponse = executeSearchScrollRequest(scrollId); + searchResponse = executeSearchScrollRequestWithReporting(scrollId); } catch (SearchPhaseExecutionException searchExecutionException) { if (searchHasShardFailure == false) { LOGGER.debug("[{}] Reinitializing scroll due to SearchPhaseExecutionException", context.jobId); markScrollAsErrored(); - searchResponse = executeSearchRequest(buildSearchRequest(lastTimestamp == null ? context.start : lastTimestamp)); + searchResponse = + executeSearchRequestWithReporting(buildSearchRequest(lastTimestamp == null ? context.start : lastTimestamp)); } else { throw searchExecutionException; } @@ -207,9 +215,13 @@ private void markScrollAsErrored() { searchHasShardFailure = true; } + private SearchResponse executeSearchScrollRequestWithReporting(String scrollId) { + return timingStatsReporter.executeWithReporting(this::executeSearchScrollRequest, scrollId); + } + protected SearchResponse executeSearchScrollRequest(String scrollId) { return ClientHelper.executeWithHeaders(context.headers, ClientHelper.ML_ORIGIN, client, - () -> new SearchScrollRequestBuilder(client, SearchScrollAction.INSTANCE) + () -> new SearchScrollRequestBuilder(client, SearchScrollAction.INSTANCE) .setScroll(SCROLL_TIMEOUT) .setScrollId(scrollId) .get()); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorFactory.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorFactory.java index ab912f54fe2a1..20ed7f664f92a 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorFactory.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorFactory.java @@ -19,6 +19,7 @@ import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.core.ml.utils.MlStrings; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; import org.elasticsearch.xpack.ml.datafeed.extractor.fields.TimeBasedExtractedFields; @@ -31,14 +32,16 @@ public class ScrollDataExtractorFactory implements DataExtractorFactory { private final Job job; private final TimeBasedExtractedFields extractedFields; private final NamedXContentRegistry xContentRegistry; + private final DatafeedTimingStatsReporter timingStatsReporter; private ScrollDataExtractorFactory(Client client, DatafeedConfig datafeedConfig, Job job, TimeBasedExtractedFields extractedFields, - NamedXContentRegistry xContentRegistry) { + NamedXContentRegistry xContentRegistry, DatafeedTimingStatsReporter timingStatsReporter) { this.client = Objects.requireNonNull(client); this.datafeedConfig = Objects.requireNonNull(datafeedConfig); this.job = Objects.requireNonNull(job); this.extractedFields = Objects.requireNonNull(extractedFields); this.xContentRegistry = xContentRegistry; + this.timingStatsReporter = Objects.requireNonNull(timingStatsReporter); } @Override @@ -53,30 +56,33 @@ public DataExtractor newExtractor(long start, long end) { start, end, datafeedConfig.getHeaders()); - return new ScrollDataExtractor(client, dataExtractorContext); + return new ScrollDataExtractor(client, dataExtractorContext, timingStatsReporter); } public static void create(Client client, DatafeedConfig datafeed, Job job, NamedXContentRegistry xContentRegistry, - ActionListener listener ) { + DatafeedTimingStatsReporter timingStatsReporter, + ActionListener listener) { // Step 2. Contruct the factory and notify listener ActionListener fieldCapabilitiesHandler = ActionListener.wrap( - fieldCapabilitiesResponse -> { - TimeBasedExtractedFields extractedFields = TimeBasedExtractedFields.build(job, datafeed, fieldCapabilitiesResponse); - listener.onResponse(new ScrollDataExtractorFactory(client, datafeed, job, extractedFields, xContentRegistry)); - }, e -> { - if (e instanceof IndexNotFoundException) { - listener.onFailure(new ResourceNotFoundException("datafeed [" + datafeed.getId() - + "] cannot retrieve data because index " + ((IndexNotFoundException) e).getIndex() + " does not exist")); - } else if (e instanceof IllegalArgumentException) { - listener.onFailure(ExceptionsHelper.badRequestException("[" + datafeed.getId() + "] " + e.getMessage())); - } else { - listener.onFailure(e); - } + fieldCapabilitiesResponse -> { + TimeBasedExtractedFields extractedFields = TimeBasedExtractedFields.build(job, datafeed, fieldCapabilitiesResponse); + listener.onResponse( + new ScrollDataExtractorFactory(client, datafeed, job, extractedFields, xContentRegistry, timingStatsReporter)); + }, + e -> { + if (e instanceof IndexNotFoundException) { + listener.onFailure(new ResourceNotFoundException("datafeed [" + datafeed.getId() + + "] cannot retrieve data because index " + ((IndexNotFoundException) e).getIndex() + " does not exist")); + } else if (e instanceof IllegalArgumentException) { + listener.onFailure(ExceptionsHelper.badRequestException("[" + datafeed.getId() + "] " + e.getMessage())); + } else { + listener.onFailure(e); } + } ); // Step 1. Get field capabilities necessary to build the information of how to extract fields diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java index 8d43ec9b75df4..7c2f15591b94f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java @@ -87,6 +87,7 @@ public class JobManager { private final Environment environment; private final JobResultsProvider jobResultsProvider; + private final JobResultsPersister jobResultsPersister; private final ClusterService clusterService; private final Auditor auditor; private final Client client; @@ -101,10 +102,11 @@ public class JobManager { * Create a JobManager */ public JobManager(Environment environment, Settings settings, JobResultsProvider jobResultsProvider, - ClusterService clusterService, Auditor auditor, ThreadPool threadPool, + JobResultsPersister jobResultsPersister, ClusterService clusterService, Auditor auditor, ThreadPool threadPool, Client client, UpdateJobProcessNotifier updateJobProcessNotifier, NamedXContentRegistry xContentRegistry) { this.environment = environment; this.jobResultsProvider = Objects.requireNonNull(jobResultsProvider); + this.jobResultsPersister = Objects.requireNonNull(jobResultsPersister); this.clusterService = Objects.requireNonNull(clusterService); this.auditor = Objects.requireNonNull(auditor); this.client = Objects.requireNonNull(client); @@ -573,12 +575,11 @@ public void revertSnapshot(RevertModelSnapshotAction.Request request, ActionList ModelSnapshot modelSnapshot) { final ModelSizeStats modelSizeStats = modelSnapshot.getModelSizeStats(); - final JobResultsPersister persister = new JobResultsPersister(client); // Step 3. After the model size stats is persisted, also persist the snapshot's quantiles and respond // ------- CheckedConsumer modelSizeStatsResponseHandler = response -> { - persister.persistQuantiles(modelSnapshot.getQuantiles(), WriteRequest.RefreshPolicy.IMMEDIATE, + jobResultsPersister.persistQuantiles(modelSnapshot.getQuantiles(), WriteRequest.RefreshPolicy.IMMEDIATE, ActionListener.wrap(quantilesResponse -> { // The quantiles can be large, and totally dominate the output - // it's clearer to remove them as they are not necessary for the revert op @@ -593,7 +594,7 @@ public void revertSnapshot(RevertModelSnapshotAction.Request request, ActionList CheckedConsumer updateHandler = response -> { if (response) { ModelSizeStats revertedModelSizeStats = new ModelSizeStats.Builder(modelSizeStats).setLogTime(new Date()).build(); - persister.persistModelSizeStats(revertedModelSizeStats, WriteRequest.RefreshPolicy.IMMEDIATE, ActionListener.wrap( + jobResultsPersister.persistModelSizeStats(revertedModelSizeStats, WriteRequest.RefreshPolicy.IMMEDIATE, ActionListener.wrap( modelSizeStatsResponseHandler, actionListener::onFailure)); } }; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java index b7d5214ecf304..f54964c120bbb 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; import org.elasticsearch.xpack.core.ml.job.persistence.ElasticsearchMappings; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.ModelSizeStats; @@ -325,6 +326,18 @@ public void commitStateWrites(String jobId) { } } + /** + * Persist datafeed timing stats + * + * @param timingStats datafeed timing stats to persist + */ + public IndexResponse persistDatafeedTimingStats(DatafeedTimingStats timingStats) { + String jobId = timingStats.getJobId(); + logger.trace("[{}] Persisting datafeed timing stats", jobId); + Persistable persistable = new Persistable(jobId, timingStats, DatafeedTimingStats.documentId(timingStats.getJobId())); + return persistable.persist(AnomalyDetectorsIndex.resultsWriteAlias(jobId)).actionGet(); + } + private XContentBuilder toXContentBuilder(ToXContent obj) throws IOException { XContentBuilder builder = jsonBuilder(); obj.toXContent(builder, ToXContent.EMPTY_PARAMS); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java index bb0ed2b3ddf0e..0b03f86c8a780 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java @@ -81,6 +81,7 @@ import org.elasticsearch.xpack.core.ml.action.GetRecordsAction; import org.elasticsearch.xpack.core.ml.calendars.Calendar; import org.elasticsearch.xpack.core.ml.calendars.ScheduledEvent; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; @@ -124,6 +125,7 @@ import java.util.Set; import java.util.function.BiFunction; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -442,6 +444,51 @@ private SearchRequestBuilder createTimingStatsSearch(String indexName, String jo .setQuery(QueryBuilders.idsQuery().addIds(TimingStats.documentId(jobId))); } + public void datafeedTimingStats(Collection jobIds, Consumer> handler, + Consumer errorHandler) { + String indexName = AnomalyDetectorsIndex.jobResultsIndexName(); + SearchRequest searchRequest = + client.prepareSearch(indexName) + .setSize(jobIds.size()) + .setIndicesOptions(IndicesOptions.lenientExpandOpen()) + .setQuery(QueryBuilders.idsQuery().addIds(jobIds.stream().map(DatafeedTimingStats::documentId).toArray(String[]::new))) + .request(); + executeAsyncWithOrigin( + client.threadPool().getThreadContext(), + ML_ORIGIN, + searchRequest, + ActionListener.wrap( + searchResponse -> { + Map timingStatsByJobId = + Arrays.stream(searchResponse.getHits().getHits()) + .map(hit -> parseSearchHit(hit, DatafeedTimingStats.PARSER, errorHandler)) + .collect(Collectors.toUnmodifiableMap(DatafeedTimingStats::getJobId, Function.identity())); + handler.accept(timingStatsByJobId); + }, + errorHandler + ), + client::search); + } + + public void datafeedTimingStats(String jobId, Consumer handler, Consumer errorHandler) { + String indexName = AnomalyDetectorsIndex.jobResultsAliasedName(jobId); + searchSingleResult( + jobId, + DatafeedTimingStats.TYPE.getPreferredName(), + createDatafeedTimingStatsSearch(indexName, jobId), + DatafeedTimingStats.PARSER, + result -> handler.accept(result.result), + errorHandler, + () -> new DatafeedTimingStats(jobId)); + } + + private SearchRequestBuilder createDatafeedTimingStatsSearch(String indexName, String jobId) { + return client.prepareSearch(indexName) + .setSize(1) + .setIndicesOptions(IndicesOptions.lenientExpandOpen()) + .setQuery(QueryBuilders.idsQuery().addIds(DatafeedTimingStats.documentId(jobId))); + } + public void getAutodetectParams(Job job, Consumer consumer, Consumer errorHandler) { String jobId = job.getId(); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilderTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilderTests.java index 0c9b50c064f96..cd86241793240 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilderTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilderTests.java @@ -21,6 +21,7 @@ import org.elasticsearch.xpack.core.ml.job.results.Bucket; import org.elasticsearch.xpack.ml.datafeed.persistence.DatafeedConfigProvider; import org.elasticsearch.xpack.ml.job.persistence.JobConfigProvider; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.elasticsearch.xpack.ml.job.persistence.JobResultsProvider; import org.elasticsearch.xpack.ml.notifications.Auditor; import org.junit.Before; @@ -48,6 +49,7 @@ public class DatafeedJobBuilderTests extends ESTestCase { private JobResultsProvider jobResultsProvider; private JobConfigProvider jobConfigProvider; private DatafeedConfigProvider datafeedConfigProvider; + private JobResultsPersister jobResultsPersister; private DatafeedJobBuilder datafeedJobBuilder; @@ -60,7 +62,7 @@ public void init() { when(client.settings()).thenReturn(Settings.EMPTY); auditor = mock(Auditor.class); taskHandler = mock(Consumer.class); - datafeedJobBuilder = new DatafeedJobBuilder(client, Settings.EMPTY, xContentRegistry(), auditor, System::currentTimeMillis); + jobResultsPersister = mock(JobResultsPersister.class); jobResultsProvider = mock(JobResultsProvider.class); Mockito.doAnswer(invocationOnMock -> { @@ -80,6 +82,16 @@ public void init() { jobConfigProvider = mock(JobConfigProvider.class); datafeedConfigProvider = mock(DatafeedConfigProvider.class); + datafeedJobBuilder = + new DatafeedJobBuilder( + client, + xContentRegistry(), + auditor, + System::currentTimeMillis, + jobConfigProvider, + jobResultsProvider, + datafeedConfigProvider, + jobResultsPersister); } public void testBuild_GivenScrollDatafeedAndNewJob() throws Exception { @@ -103,7 +115,7 @@ public void testBuild_GivenScrollDatafeedAndNewJob() throws Exception { givenJob(jobBuilder); givenDatafeed(datafeed); - datafeedJobBuilder.build("datafeed1", jobResultsProvider, jobConfigProvider, datafeedConfigProvider, datafeedJobHandler); + datafeedJobBuilder.build("datafeed1", datafeedJobHandler); assertBusy(() -> wasHandlerCalled.get()); } @@ -131,7 +143,7 @@ public void testBuild_GivenScrollDatafeedAndOldJobWithLatestRecordTimestampAfter givenJob(jobBuilder); givenDatafeed(datafeed); - datafeedJobBuilder.build("datafeed1", jobResultsProvider, jobConfigProvider, datafeedConfigProvider, datafeedJobHandler); + datafeedJobBuilder.build("datafeed1", datafeedJobHandler); assertBusy(() -> wasHandlerCalled.get()); } @@ -159,7 +171,7 @@ public void testBuild_GivenScrollDatafeedAndOldJobWithLatestBucketAfterLatestRec givenJob(jobBuilder); givenDatafeed(datafeed); - datafeedJobBuilder.build("datafeed1", jobResultsProvider, jobConfigProvider, datafeedConfigProvider, datafeedJobHandler); + datafeedJobBuilder.build("datafeed1", datafeedJobHandler); assertBusy(() -> wasHandlerCalled.get()); } @@ -184,8 +196,7 @@ public void testBuild_GivenBucketsRequestFails() { givenJob(jobBuilder); givenDatafeed(datafeed); - datafeedJobBuilder.build("datafeed1", jobResultsProvider, jobConfigProvider, datafeedConfigProvider, - ActionListener.wrap(datafeedJob -> fail(), taskHandler)); + datafeedJobBuilder.build("datafeed1", ActionListener.wrap(datafeedJob -> fail(), taskHandler)); verify(taskHandler).accept(error); } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java new file mode 100644 index 0000000000000..94870ef6d531e --- /dev/null +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java @@ -0,0 +1,61 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.ml.datafeed; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; +import org.elasticsearch.xpack.core.watcher.watch.ClockMock; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; +import org.junit.Before; + +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; + +public class DatafeedTimingStatsReporterTests extends ESTestCase { + + private static final String JOB_ID = "my-job-id"; + + private ClockMock clock; + private JobResultsPersister jobResultsPersister; + + @Before + public void setUpTests() { + clock = new ClockMock(); + jobResultsPersister = mock(JobResultsPersister.class); + } + + public void testExecuteWithReporting() { + DatafeedTimingStats timingStats = new DatafeedTimingStats(JOB_ID, 10000.0); + DatafeedTimingStatsReporter timingStatsReporter = new DatafeedTimingStatsReporter(timingStats, clock, jobResultsPersister); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 10000.0))); + + timingStatsReporter.executeWithReporting(this::advanceTime, 1); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 11000.0))); + verifyZeroInteractions(jobResultsPersister); + + timingStatsReporter.executeWithReporting(this::advanceTime, 1); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 12000.0))); + verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 12000.0)); + verifyNoMoreInteractions(jobResultsPersister); + + timingStatsReporter.executeWithReporting(this::advanceTime, 1); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 13000.0))); + verifyZeroInteractions(jobResultsPersister); + + timingStatsReporter.executeWithReporting(this::advanceTime, 1); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 14000.0))); + verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 14000.0)); + verifyNoMoreInteractions(jobResultsPersister); + } + + private Void advanceTime(int seconds) { + clock.fastForwardSeconds(seconds); + return null; + } +} diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactoryTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactoryTests.java index 04a70edae03bd..308a42242304b 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactoryTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactoryTests.java @@ -34,6 +34,7 @@ import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig; import org.elasticsearch.xpack.ml.datafeed.DatafeedManagerTests; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.aggregation.AggregationDataExtractorFactory; import org.elasticsearch.xpack.ml.datafeed.extractor.chunked.ChunkedDataExtractorFactory; import org.elasticsearch.xpack.ml.datafeed.extractor.aggregation.RollupDataExtractorFactory; @@ -62,6 +63,7 @@ public class DataExtractorFactoryTests extends ESTestCase { private GetRollupIndexCapsAction.Response getRollupIndexResponse; private Client client; + private DatafeedTimingStatsReporter timingStatsReporter; @Override protected NamedXContentRegistry xContentRegistry() { @@ -72,6 +74,7 @@ protected NamedXContentRegistry xContentRegistry() { @Before public void setUpTests() { client = mock(Client.class); + timingStatsReporter = mock(DatafeedTimingStatsReporter.class); ThreadPool threadPool = mock(ThreadPool.class); when(client.threadPool()).thenReturn(threadPool); when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); @@ -109,7 +112,8 @@ public void testCreateDataExtractorFactoryGivenDefaultScroll() { e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig, jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig, jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenScrollWithAutoChunk() { @@ -125,7 +129,8 @@ public void testCreateDataExtractorFactoryGivenScrollWithAutoChunk() { e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenScrollWithOffChunk() { @@ -141,7 +146,8 @@ public void testCreateDataExtractorFactoryGivenScrollWithOffChunk() { e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenDefaultAggregation() { @@ -159,7 +165,8 @@ public void testCreateDataExtractorFactoryGivenDefaultAggregation() { e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenAggregationWithOffChunk() { @@ -178,7 +185,8 @@ public void testCreateDataExtractorFactoryGivenAggregationWithOffChunk() { e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenDefaultAggregationWithAutoChunk() { @@ -197,7 +205,8 @@ public void testCreateDataExtractorFactoryGivenDefaultAggregationWithAutoChunk() e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenRollupAndValidAggregation() { @@ -220,7 +229,8 @@ public void testCreateDataExtractorFactoryGivenRollupAndValidAggregation() { }, e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenRollupAndRemoteIndex() { @@ -297,7 +307,8 @@ public void testCreateDataExtractorFactoryGivenRollupAndValidAggregationAndAutoC }, e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenRollupButNoAggregations() { @@ -317,7 +328,8 @@ public void testCreateDataExtractorFactoryGivenRollupButNoAggregations() { } ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenRollupWithBadInterval() { @@ -343,7 +355,8 @@ public void testCreateDataExtractorFactoryGivenRollupWithBadInterval() { assertWarnings("[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future."); } ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenRollupMissingTerms() { @@ -368,7 +381,8 @@ public void testCreateDataExtractorFactoryGivenRollupMissingTerms() { assertWarnings("[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future."); } ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenRollupMissingMetric() { @@ -393,7 +407,8 @@ public void testCreateDataExtractorFactoryGivenRollupMissingMetric() { assertWarnings("[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future."); } ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } private void givenAggregatableRollup(String field, String type, int minuteInterval, String... groupByTerms) { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorFactoryTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorFactoryTests.java index a082398772337..eda96b0bcf866 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorFactoryTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorFactoryTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.xpack.core.ml.job.config.DataDescription; import org.elasticsearch.xpack.core.ml.job.config.Detector; import org.elasticsearch.xpack.core.ml.job.config.Job; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.junit.Before; import java.util.Arrays; @@ -29,10 +30,12 @@ public class AggregationDataExtractorFactoryTests extends ESTestCase { private Client client; + private DatafeedTimingStatsReporter timingStatsReporter; @Before public void setUpMocks() { client = mock(Client.class); + timingStatsReporter = mock(DatafeedTimingStatsReporter.class); } @Override @@ -76,6 +79,7 @@ private AggregationDataExtractorFactory createFactory(long histogramInterval) { DatafeedConfig.Builder datafeedConfigBuilder = new DatafeedConfig.Builder("foo-feed", jobBuilder.getId()); datafeedConfigBuilder.setParsedAggregations(aggs); datafeedConfigBuilder.setIndices(Arrays.asList("my_index")); - return new AggregationDataExtractorFactory(client, datafeedConfigBuilder.build(), jobBuilder.build(new Date()), xContentRegistry()); + return new AggregationDataExtractorFactory( + client, datafeedConfigBuilder.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter); } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorTests.java index bb7d1d1729cd4..33456ca742ce7 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorTests.java @@ -17,7 +17,10 @@ import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.aggregation.AggregationTestUtils.Term; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.junit.Before; import java.io.BufferedReader; @@ -25,6 +28,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import java.time.Clock; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -54,13 +58,14 @@ public class AggregationDataExtractorTests extends ESTestCase { private List indices; private QueryBuilder query; private AggregatorFactories.Builder aggs; + private DatafeedTimingStatsReporter timingStatsReporter; private class TestDataExtractor extends AggregationDataExtractor { private SearchResponse nextResponse; TestDataExtractor(long start, long end) { - super(testClient, createContext(start, end)); + super(testClient, createContext(start, end), timingStatsReporter); } @Override @@ -88,6 +93,8 @@ public void setUpTests() { .addAggregator(AggregationBuilders.histogram("time").field("time").interval(1000).subAggregation( AggregationBuilders.terms("airline").field("airline").subAggregation( AggregationBuilders.avg("responsetime").field("responsetime")))); + timingStatsReporter = + new DatafeedTimingStatsReporter(new DatafeedTimingStats(jobId), Clock.systemUTC(), mock(JobResultsPersister.class)); } public void testExtraction() throws IOException { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorFactoryTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorFactoryTests.java index 9f345dfe77b40..d61142b6664ea 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorFactoryTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorFactoryTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; import org.elasticsearch.xpack.core.ml.job.config.AnalysisConfig; import org.elasticsearch.xpack.core.ml.job.config.DataDescription; @@ -31,6 +32,7 @@ public class ChunkedDataExtractorFactoryTests extends ESTestCase { private Client client; private DataExtractorFactory dataExtractorFactory; + private DatafeedTimingStatsReporter timingStatsReporter; @Override protected NamedXContentRegistry xContentRegistry() { @@ -42,6 +44,7 @@ protected NamedXContentRegistry xContentRegistry() { public void setUpMocks() { client = mock(Client.class); dataExtractorFactory = mock(DataExtractorFactory.class); + timingStatsReporter = mock(DatafeedTimingStatsReporter.class); } public void testNewExtractor_GivenAlignedTimes() { @@ -103,7 +106,12 @@ private ChunkedDataExtractorFactory createFactory(long histogramInterval) { DatafeedConfig.Builder datafeedConfigBuilder = new DatafeedConfig.Builder("foo-feed", jobBuilder.getId()); datafeedConfigBuilder.setParsedAggregations(aggs); datafeedConfigBuilder.setIndices(Arrays.asList("my_index")); - return new ChunkedDataExtractorFactory(client, datafeedConfigBuilder.build(), jobBuilder.build(new Date()), - xContentRegistry(), dataExtractorFactory); + return new ChunkedDataExtractorFactory( + client, + datafeedConfigBuilder.build(), + jobBuilder.build(new Date()), + xContentRegistry(), + dataExtractorFactory, + timingStatsReporter); } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorTests.java index 406f1a5fa9024..378bb59ad08c9 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorTests.java @@ -23,12 +23,16 @@ import org.elasticsearch.search.aggregations.metrics.Max; import org.elasticsearch.search.aggregations.metrics.Min; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.datafeed.extractor.DataExtractor; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.junit.Before; import java.io.IOException; import java.io.InputStream; +import java.time.Clock; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -54,17 +58,18 @@ public class ChunkedDataExtractorTests extends ESTestCase { private int scrollSize; private TimeValue chunkSpan; private DataExtractorFactory dataExtractorFactory; + private DatafeedTimingStatsReporter timingStatsReporter; private class TestDataExtractor extends ChunkedDataExtractor { private SearchResponse nextResponse; TestDataExtractor(long start, long end) { - super(client, dataExtractorFactory, createContext(start, end)); + super(client, dataExtractorFactory, createContext(start, end), timingStatsReporter); } TestDataExtractor(long start, long end, boolean hasAggregations, Long histogramInterval) { - super(client, dataExtractorFactory, createContext(start, end, hasAggregations, histogramInterval)); + super(client, dataExtractorFactory, createContext(start, end, hasAggregations, histogramInterval), timingStatsReporter); } @Override @@ -89,6 +94,8 @@ public void setUpTests() { scrollSize = 1000; chunkSpan = null; dataExtractorFactory = mock(DataExtractorFactory.class); + timingStatsReporter = + new DatafeedTimingStatsReporter(new DatafeedTimingStats(jobId), Clock.systemUTC(), mock(JobResultsPersister.class)); } public void testExtractionGivenNoData() throws IOException { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java index 131d47de38db6..e56c241e2e4f0 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java @@ -28,8 +28,11 @@ import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; +import org.elasticsearch.xpack.ml.datafeed.DatafeedTimingStatsReporter; import org.elasticsearch.xpack.ml.datafeed.extractor.fields.ExtractedField; import org.elasticsearch.xpack.ml.datafeed.extractor.fields.TimeBasedExtractedFields; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.junit.Before; import org.mockito.ArgumentCaptor; @@ -38,6 +41,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import java.time.Clock; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -71,6 +75,7 @@ public class ScrollDataExtractorTests extends ESTestCase { private int scrollSize; private long initScrollStartTime; private ActionFuture clearScrollFuture; + private DatafeedTimingStatsReporter timingStatsReporter; private class TestDataExtractor extends ScrollDataExtractor { @@ -81,7 +86,7 @@ private class TestDataExtractor extends ScrollDataExtractor { } TestDataExtractor(ScrollDataExtractorContext context) { - super(client, context); + super(client, context, timingStatsReporter); } @Override @@ -140,6 +145,8 @@ public void setUpTests() { clearScrollFuture = mock(ActionFuture.class); capturedClearScrollRequests = ArgumentCaptor.forClass(ClearScrollRequest.class); when(client.execute(same(ClearScrollAction.INSTANCE), capturedClearScrollRequests.capture())).thenReturn(clearScrollFuture); + timingStatsReporter = + new DatafeedTimingStatsReporter(new DatafeedTimingStats(jobId), Clock.systemUTC(), mock(JobResultsPersister.class)); } public void testSinglePageExtraction() throws IOException { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java index 65a6c6c88f6ea..5c8f9e3f51a68 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java @@ -58,6 +58,7 @@ import org.elasticsearch.xpack.ml.MachineLearning; import org.elasticsearch.xpack.ml.MlConfigMigrationEligibilityCheck; import org.elasticsearch.xpack.ml.job.categorization.CategorizationAnalyzerTests; +import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.elasticsearch.xpack.ml.job.persistence.JobResultsProvider; import org.elasticsearch.xpack.ml.job.persistence.MockClientBuilder; import org.elasticsearch.xpack.ml.job.process.autodetect.UpdateParams; @@ -103,6 +104,7 @@ public class JobManagerTests extends ESTestCase { private ClusterService clusterService; private ThreadPool threadPool; private JobResultsProvider jobResultsProvider; + private JobResultsPersister jobResultsPersister; private Auditor auditor; private UpdateJobProcessNotifier updateJobProcessNotifier; @@ -123,6 +125,7 @@ public void setup() throws Exception { givenClusterSettings(settings); jobResultsProvider = mock(JobResultsProvider.class); + jobResultsPersister = mock(JobResultsPersister.class); auditor = mock(Auditor.class); updateJobProcessNotifier = mock(UpdateJobProcessNotifier.class); @@ -593,8 +596,17 @@ private Job.Builder createJob() { } private JobManager createJobManager(Client client) { - return new JobManager(environment, environment.settings(), jobResultsProvider, clusterService, - auditor, threadPool, client, updateJobProcessNotifier, xContentRegistry()); + return new JobManager( + environment, + environment.settings(), + jobResultsProvider, + jobResultsPersister, + clusterService, + auditor, + threadPool, + client, + updateJobProcessNotifier, + xContentRegistry()); } private ClusterState createClusterState() { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java index 721e0d9cdca94..f1263f4057999 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java @@ -6,15 +6,18 @@ package org.elasticsearch.xpack.ml.job.persistence; import org.elasticsearch.action.ActionFuture; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.bulk.BulkItemResponse; import org.elasticsearch.action.bulk.BulkRequest; import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.client.Client; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.TimingStats; import org.elasticsearch.xpack.core.ml.job.results.AnomalyRecord; import org.elasticsearch.xpack.core.ml.job.results.Bucket; @@ -32,6 +35,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -227,6 +231,38 @@ public void testPersistTimingStats() { verifyNoMoreInteractions(client); } + public void testPersistDatafeedTimingStats() { + Client client = mockClient(ArgumentCaptor.forClass(BulkRequest.class)); + doAnswer( + invocationOnMock -> { + // Take the listener passed to client::index as 2nd argument + ActionListener listener = (ActionListener) invocationOnMock.getArguments()[1]; + // Handle the response on the listener + listener.onResponse(new IndexResponse()); + return null; + }) + .when(client).index(any(), any(ActionListener.class)); + + JobResultsPersister persister = new JobResultsPersister(client); + DatafeedTimingStats timingStats = new DatafeedTimingStats("foo", 666.0); + persister.persistDatafeedTimingStats(timingStats); + + ArgumentCaptor indexRequestCaptor = ArgumentCaptor.forClass(IndexRequest.class); + verify(client, times(1)).index(indexRequestCaptor.capture(), any(ActionListener.class)); + IndexRequest indexRequest = indexRequestCaptor.getValue(); + assertThat(indexRequest.index(), equalTo(".ml-anomalies-.write-foo")); + assertThat(indexRequest.id(), equalTo("foo_datafeed_timing_stats")); + assertThat( + indexRequest.sourceAsMap(), + equalTo( + Map.of( + "job_id", "foo", + "total_search_time_ms", 666.0))); + + verify(client, times(1)).threadPool(); + verifyNoMoreInteractions(client); + } + @SuppressWarnings({"unchecked"}) private Client mockClient(ArgumentCaptor captor) { Client client = mock(Client.class); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java index af821cf48fdc2..57b5a096c17d3 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java @@ -42,6 +42,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.action.util.QueryPage; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndexFields; @@ -63,6 +64,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -879,6 +881,83 @@ public void testTimingStats_NotFound() throws IOException { verifyNoMoreInteractions(client); } + public void testDatafeedTimingStats_MultipleDocumentsAtOnce() throws IOException { + String indexName = AnomalyDetectorsIndex.jobResultsIndexName(); + List> source = + Arrays.asList( + Map.of( + Job.ID.getPreferredName(), "foo", + DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), 666.0), + Map.of( + Job.ID.getPreferredName(), "bar", + DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), 777.0)); + SearchResponse response = createSearchResponse(source); + Client client = getMockedClient( + queryBuilder -> assertThat(queryBuilder.getName(), equalTo("ids")), + response); + + when(client.prepareSearch(indexName)).thenReturn(new SearchRequestBuilder(client, SearchAction.INSTANCE).setIndices(indexName)); + JobResultsProvider provider = createProvider(client); + provider.datafeedTimingStats( + Set.of("foo", "bar"), + statsByJobId -> + assertThat( + statsByJobId, + equalTo(Map.of("foo", new DatafeedTimingStats("foo", 666.0), "bar", new DatafeedTimingStats("bar", 777.0)))), + e -> { throw new AssertionError(); }); + + verify(client).prepareSearch(indexName); + verify(client).threadPool(); + verify(client).search(any(SearchRequest.class), any(ActionListener.class)); + verifyNoMoreInteractions(client); + } + + public void testDatafeedTimingStats_Ok() throws IOException { + String indexName = AnomalyDetectorsIndex.jobResultsAliasedName("foo"); + List> source = + Arrays.asList( + Map.of( + Job.ID.getPreferredName(), "foo", + DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), 666.0)); + SearchResponse response = createSearchResponse(source); + Client client = getMockedClient( + queryBuilder -> assertThat(queryBuilder.getName(), equalTo("ids")), + response); + + when(client.prepareSearch(indexName)).thenReturn(new SearchRequestBuilder(client, SearchAction.INSTANCE).setIndices(indexName)); + JobResultsProvider provider = createProvider(client); + provider.datafeedTimingStats( + "foo", + stats -> assertThat(stats, equalTo(new DatafeedTimingStats("foo", 666.0))), + e -> { throw new AssertionError(); }); + + verify(client).prepareSearch(indexName); + verify(client).threadPool(); + verify(client).search(any(SearchRequest.class), any(ActionListener.class)); + verifyNoMoreInteractions(client); + } + + public void testDatafeedTimingStats_NotFound() throws IOException { + String indexName = AnomalyDetectorsIndex.jobResultsAliasedName("foo"); + List> source = new ArrayList<>(); + SearchResponse response = createSearchResponse(source); + Client client = getMockedClient( + queryBuilder -> assertThat(queryBuilder.getName(), equalTo("ids")), + response); + + when(client.prepareSearch(indexName)).thenReturn(new SearchRequestBuilder(client, SearchAction.INSTANCE).setIndices(indexName)); + JobResultsProvider provider = createProvider(client); + provider.datafeedTimingStats( + "foo", + stats -> assertThat(stats, equalTo(new DatafeedTimingStats("foo"))), + e -> { throw new AssertionError(); }); + + verify(client).prepareSearch(indexName); + verify(client).threadPool(); + verify(client).search(any(SearchRequest.class), any(ActionListener.class)); + verifyNoMoreInteractions(client); + } + private Bucket createBucketAtEpochTime(long epoch) { return new Bucket("foo", new Date(epoch), 123); } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml index f9178dc6f71f5..d83f10d7f290b 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml @@ -126,16 +126,18 @@ setup: - do: ml.get_datafeed_stats: datafeed_id: datafeed-1 - - match: { datafeeds.0.datafeed_id: "datafeed-1"} - - match: { datafeeds.0.state: "stopped"} + - match: { datafeeds.0.datafeed_id: "datafeed-1"} + - match: { datafeeds.0.state: "stopped"} - is_false: datafeeds.0.node + - match: { datafeeds.0.timing_stats.job_id: "get-datafeed-stats-1" } - do: ml.get_datafeed_stats: datafeed_id: datafeed-2 - - match: { datafeeds.0.datafeed_id: "datafeed-2"} - - match: { datafeeds.0.state: "stopped"} + - match: { datafeeds.0.datafeed_id: "datafeed-2"} + - match: { datafeeds.0.state: "stopped"} - is_false: datafeeds.0.node + - match: { datafeeds.0.timing_stats.job_id: "get-datafeed-stats-2" } --- "Test get stats for started datafeed": @@ -158,6 +160,40 @@ setup: - is_true: datafeeds.0.node.transport_address - match: { datafeeds.0.node.attributes.ml\.max_open_jobs: "20"} +--- +"Test get stats for started datafeed contains timing stats": + + - do: + ml.open_job: + job_id: get-datafeed-stats-1 + + - do: + ml.start_datafeed: + "datafeed_id": "datafeed-1" + "start": 0 + + - do: + ml.get_datafeed_stats: + datafeed_id: datafeed-1 + - match: { datafeeds.0.datafeed_id: "datafeed-1"} + - match: { datafeeds.0.state: "started"} + - match: { datafeeds.0.timing_stats.job_id: "get-datafeed-stats-1"} + # Datafeed did not perform any search yet, hence 0.0 + - match: { datafeeds.0.timing_stats.total_search_time_ms: 0.0} + + - do: + ml.stop_datafeed: + "datafeed_id": "datafeed-1" + + - do: + ml.get_datafeed_stats: + datafeed_id: datafeed-1 + - match: { datafeeds.0.datafeed_id: "datafeed-1"} + - match: { datafeeds.0.state: "stopped"} + - match: { datafeeds.0.timing_stats.job_id: "get-datafeed-stats-1"} + # Datafeed did perform at least one search + - gt: { datafeeds.0.timing_stats.total_search_time_ms: 0.0} + --- "Test implicit get all datafeed stats given started datafeeds": From 3dda61a27ab134a00e77434c1465ab45d185ee51 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Mon, 24 Jun 2019 11:23:52 +0200 Subject: [PATCH 02/16] Move differSignificantly method from DatafeedTimingStats to DatafeedTimingStatsReporter where it logically belongs --- .../core/ml/datafeed/DatafeedTimingStats.java | 21 ----------------- .../ml/datafeed/DatafeedTimingStatsTests.java | 13 ----------- .../datafeed/DatafeedTimingStatsReporter.java | 23 ++++++++++++++++++- .../DatafeedTimingStatsReporterTests.java | 16 +++++++++++++ 4 files changed, 38 insertions(+), 35 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java index e80733448ebf2..8bcedc7a7dbfa 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java @@ -114,25 +114,4 @@ public int hashCode() { public String toString() { return Strings.toString(this); } - - /** - * Returns true if given stats objects differ from each other by more than 10% for at least one of the statistics. - */ - public static boolean differSignificantly(DatafeedTimingStats stats1, DatafeedTimingStats stats2) { - return differSignificantly(stats1.totalSearchTimeMs, stats2.totalSearchTimeMs); - } - - /** - * Returns {@code true} if one of the ratios { value1 / value2, value2 / value1 } is smaller than MIN_VALID_RATIO. - * This can be interpreted as values { value1, value2 } differing significantly from each other. - */ - private static boolean differSignificantly(double value1, double value2) { - return (value2 / value1 < MIN_VALID_RATIO) || (value1 / value2 < MIN_VALID_RATIO); - } - - /** - * Minimum ratio of values that is interpreted as values being similar. - * If the values ratio is less than MIN_VALID_RATIO, the values are interpreted as significantly different. - */ - private static final double MIN_VALID_RATIO = 0.9; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java index 5c7b9414d9c8c..cb139e06ee1a5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java @@ -12,7 +12,6 @@ import java.io.IOException; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; public class DatafeedTimingStatsTests extends AbstractSerializingTestCase { @@ -93,16 +92,4 @@ public void testIncrementTotalSearchTimeMs() { public void testDocumentId() { assertThat(DatafeedTimingStats.documentId("my-job-id"), equalTo("my-job-id_datafeed_timing_stats")); } - - public void testTimingStatsDifferSignificantly() { - assertThat( - DatafeedTimingStats.differSignificantly(new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1000.0)), - is(false)); - assertThat( - DatafeedTimingStats.differSignificantly(new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1100.0)), - is(false)); - assertThat( - DatafeedTimingStats.differSignificantly(new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1120.0)), - is(true)); - } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java index 4ebf90ad4a587..f679c2ae7d7b4 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java @@ -48,10 +48,31 @@ public R executeWithReporting(Function function, T argument) { private void reportSearchDuration(Duration searchDuration) { double searchDurationMs = searchDuration.toMillis(); currentTimingStats.incrementTotalSearchTimeMs(searchDurationMs); - if (DatafeedTimingStats.differSignificantly(currentTimingStats, persistedTimingStats)) { + if (differSignificantly(currentTimingStats, persistedTimingStats)) { jobResultsPersister.persistDatafeedTimingStats(currentTimingStats); persistedTimingStats = currentTimingStats; currentTimingStats = new DatafeedTimingStats(persistedTimingStats); } } + + /** + * Returns true if given stats objects differ from each other by more than 10% for at least one of the statistics. + */ + public static boolean differSignificantly(DatafeedTimingStats stats1, DatafeedTimingStats stats2) { + return differSignificantly(stats1.getTotalSearchTimeMs(), stats2.getTotalSearchTimeMs()); + } + + /** + * Returns {@code true} if one of the ratios { value1 / value2, value2 / value1 } is smaller than MIN_VALID_RATIO. + * This can be interpreted as values { value1, value2 } differing significantly from each other. + */ + private static boolean differSignificantly(double value1, double value2) { + return (value2 / value1 < MIN_VALID_RATIO) || (value1 / value2 < MIN_VALID_RATIO); + } + + /** + * Minimum ratio of values that is interpreted as values being similar. + * If the values ratio is less than MIN_VALID_RATIO, the values are interpreted as significantly different. + */ + private static final double MIN_VALID_RATIO = 0.9; } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java index 94870ef6d531e..0b78077d02ec1 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java @@ -12,6 +12,7 @@ import org.junit.Before; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -58,4 +59,19 @@ private Void advanceTime(int seconds) { clock.fastForwardSeconds(seconds); return null; } + + public void testTimingStatsDifferSignificantly() { + assertThat( + DatafeedTimingStatsReporter.differSignificantly( + new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1000.0)), + is(false)); + assertThat( + DatafeedTimingStatsReporter.differSignificantly( + new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1100.0)), + is(false)); + assertThat( + DatafeedTimingStatsReporter.differSignificantly( + new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1120.0)), + is(true)); + } } From 609f4d79ce0c16ee55c073cd8e67484713c0d797 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Mon, 24 Jun 2019 12:31:25 +0200 Subject: [PATCH 03/16] Pass jobId instead of the whole jobIdByDatafeedId map --- .../ml/action/TransportGetDatafeedsStatsAction.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java index bd0432f890abc..2d26731680a4b 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java @@ -77,8 +77,13 @@ protected void masterOperation(Task task, GetDatafeedsStatsAction.Request reques timingStatsByJobId -> { PersistentTasksCustomMetaData tasksInProgress = state.getMetaData().custom(PersistentTasksCustomMetaData.TYPE); List results = datafeedBuilders.stream() - .map(datafeedBuilder -> getDatafeedStats( - datafeedBuilder.getId(), state, tasksInProgress, jobIdByDatafeedId, timingStatsByJobId)) + .map( + datafeedBuilder -> getDatafeedStats( + datafeedBuilder.getId(), + state, + tasksInProgress, + jobIdByDatafeedId.get(datafeedBuilder.getId()), + timingStatsByJobId)) .collect(Collectors.toList()); QueryPage statsPage = new QueryPage<>(results, results.size(), DatafeedConfig.RESULTS_FIELD); @@ -93,7 +98,7 @@ protected void masterOperation(Task task, GetDatafeedsStatsAction.Request reques private static GetDatafeedsStatsAction.Response.DatafeedStats getDatafeedStats(String datafeedId, ClusterState state, PersistentTasksCustomMetaData tasks, - Map jobIdByDatafeedId, + String jobId, Map timingStatsByJobId) { PersistentTasksCustomMetaData.PersistentTask task = MlTasks.getDatafeedTask(datafeedId, tasks); DatafeedState datafeedState = MlTasks.getDatafeedState(datafeedId, tasks); @@ -104,7 +109,6 @@ private static GetDatafeedsStatsAction.Response.DatafeedStats getDatafeedStats(S explanation = task.getAssignment().getExplanation(); } DatafeedTimingStats timingStats = null; - String jobId = jobIdByDatafeedId.get(datafeedId); if (jobId != null) { timingStats = timingStatsByJobId.get(jobId); if (timingStats == null) { From e11e021780c7d2a2090c25642c650316315c226d Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Wed, 26 Jun 2019 12:00:29 +0200 Subject: [PATCH 04/16] Apply review comments --- .../persistence/AnomalyDetectorsIndex.java | 8 -- .../TransportGetDatafeedsStatsAction.java | 41 +++++---- .../datafeed/DatafeedTimingStatsReporter.java | 10 ++- .../job/persistence/JobResultsProvider.java | 87 +++++++++++++------ .../DatafeedTimingStatsReporterTests.java | 62 ++++++++++--- .../persistence/JobResultsProviderTests.java | 57 +++++++++--- 6 files changed, 186 insertions(+), 79 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/AnomalyDetectorsIndex.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/AnomalyDetectorsIndex.java index 1c774f2c43577..7e61d42705a90 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/AnomalyDetectorsIndex.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/AnomalyDetectorsIndex.java @@ -37,14 +37,6 @@ public static String jobResultsIndexPrefix() { return AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX; } - /** - * The name of the alias pointing to the indices where the jobs' results are stored - * @return The read alias - */ - public static String jobResultsIndexName() { - return AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT; - } - /** * The name of the alias pointing to the indices where the job's results are stored * @param jobId Job Id diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java index 2d26731680a4b..a759757c7d80d 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetDatafeedsStatsAction.java @@ -29,7 +29,6 @@ import org.elasticsearch.xpack.ml.job.persistence.JobResultsProvider; import java.util.List; -import java.util.Map; import java.util.stream.Collectors; public class TransportGetDatafeedsStatsAction extends TransportMasterNodeReadAction { - Map jobIdByDatafeedId = datafeedBuilders.stream() - .map(DatafeedConfig.Builder::build) - .collect(Collectors.toUnmodifiableMap(DatafeedConfig::getId, DatafeedConfig::getJobId)); + List jobIds = + datafeedBuilders.stream() + .map(DatafeedConfig.Builder::build) + .map(DatafeedConfig::getJobId) + .collect(Collectors.toList()); jobResultsProvider.datafeedTimingStats( - jobIdByDatafeedId.values(), + jobIds, timingStatsByJobId -> { PersistentTasksCustomMetaData tasksInProgress = state.getMetaData().custom(PersistentTasksCustomMetaData.TYPE); - List results = datafeedBuilders.stream() - .map( - datafeedBuilder -> getDatafeedStats( - datafeedBuilder.getId(), - state, - tasksInProgress, - jobIdByDatafeedId.get(datafeedBuilder.getId()), - timingStatsByJobId)) - .collect(Collectors.toList()); + List results = + datafeedBuilders.stream() + .map(DatafeedConfig.Builder::build) + .map( + datafeed -> getDatafeedStats( + datafeed.getId(), + state, + tasksInProgress, + datafeed.getJobId(), + timingStatsByJobId.get(datafeed.getJobId()))) + .collect(Collectors.toList()); QueryPage statsPage = new QueryPage<>(results, results.size(), DatafeedConfig.RESULTS_FIELD); listener.onResponse(new GetDatafeedsStatsAction.Response(statsPage)); @@ -99,7 +102,7 @@ private static GetDatafeedsStatsAction.Response.DatafeedStats getDatafeedStats(S ClusterState state, PersistentTasksCustomMetaData tasks, String jobId, - Map timingStatsByJobId) { + DatafeedTimingStats timingStats) { PersistentTasksCustomMetaData.PersistentTask task = MlTasks.getDatafeedTask(datafeedId, tasks); DatafeedState datafeedState = MlTasks.getDatafeedState(datafeedId, tasks); DiscoveryNode node = null; @@ -108,12 +111,8 @@ private static GetDatafeedsStatsAction.Response.DatafeedStats getDatafeedStats(S node = state.nodes().get(task.getExecutorNode()); explanation = task.getAssignment().getExplanation(); } - DatafeedTimingStats timingStats = null; - if (jobId != null) { - timingStats = timingStatsByJobId.get(jobId); - if (timingStats == null) { - timingStats = new DatafeedTimingStats(jobId); - } + if (timingStats == null) { + timingStats = new DatafeedTimingStats(jobId); } return new GetDatafeedsStatsAction.Response.DatafeedStats(datafeedId, datafeedState, node, explanation, timingStats); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java index f679c2ae7d7b4..1d9728cf232dd 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java @@ -67,7 +67,9 @@ public static boolean differSignificantly(DatafeedTimingStats stats1, DatafeedTi * This can be interpreted as values { value1, value2 } differing significantly from each other. */ private static boolean differSignificantly(double value1, double value2) { - return (value2 / value1 < MIN_VALID_RATIO) || (value1 / value2 < MIN_VALID_RATIO); + return (value2 / value1 < MIN_VALID_RATIO) + || (value1 / value2 < MIN_VALID_RATIO) + || Math.abs(value1 - value2) > MAX_VALID_ABS_DIFFERENCE_MS; } /** @@ -75,4 +77,10 @@ private static boolean differSignificantly(double value1, double value2) { * If the values ratio is less than MIN_VALID_RATIO, the values are interpreted as significantly different. */ private static final double MIN_VALID_RATIO = 0.9; + + /** + * Maximum absolute difference of values that is interpreted as values being similar. + * If the values absolute difference is greater than MAX_VALID_ABS_DIFFERENCE, the values are interpreted as significantly different. + */ + private static final double MAX_VALID_ABS_DIFFERENCE_MS = 10000.0; } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java index 0b03f86c8a780..daaf6654bc5ca 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java @@ -24,6 +24,7 @@ import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.get.GetResponse; +import org.elasticsearch.action.search.MultiSearchRequest; import org.elasticsearch.action.search.MultiSearchRequestBuilder; import org.elasticsearch.action.search.MultiSearchResponse; import org.elasticsearch.action.search.SearchRequest; @@ -117,6 +118,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -125,7 +127,6 @@ import java.util.Set; import java.util.function.BiFunction; import java.util.function.Consumer; -import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -430,44 +431,77 @@ public void timingStats(String jobId, Consumer handler, Consumer handler.accept(result.result), errorHandler, () -> new TimingStats(jobId)); } - private SearchRequestBuilder createTimingStatsSearch(String indexName, String jobId) { + private SearchRequestBuilder createLatestTimingStatsSearch(String indexName, String jobId) { return client.prepareSearch(indexName) .setSize(1) .setIndicesOptions(IndicesOptions.lenientExpandOpen()) - .setQuery(QueryBuilders.idsQuery().addIds(TimingStats.documentId(jobId))); + .setQuery(QueryBuilders.idsQuery().addIds(TimingStats.documentId(jobId))) + .addSort(SortBuilders.fieldSort(TimingStats.BUCKET_COUNT.getPreferredName()).order(SortOrder.DESC)); } - public void datafeedTimingStats(Collection jobIds, Consumer> handler, + public void datafeedTimingStats(List jobIds, Consumer> handler, Consumer errorHandler) { - String indexName = AnomalyDetectorsIndex.jobResultsIndexName(); - SearchRequest searchRequest = - client.prepareSearch(indexName) - .setSize(jobIds.size()) - .setIndicesOptions(IndicesOptions.lenientExpandOpen()) - .setQuery(QueryBuilders.idsQuery().addIds(jobIds.stream().map(DatafeedTimingStats::documentId).toArray(String[]::new))) - .request(); + MultiSearchRequestBuilder msearchRequestBuilder = client.prepareMultiSearch(); + for (String jobId : jobIds) { + String indexName = AnomalyDetectorsIndex.jobResultsAliasedName(jobId); + msearchRequestBuilder.add(createLatestDatafeedTimingStatsSearch(indexName, jobId)); + } + MultiSearchRequest msearchRequest = msearchRequestBuilder.request(); + executeAsyncWithOrigin( client.threadPool().getThreadContext(), ML_ORIGIN, - searchRequest, - ActionListener.wrap( - searchResponse -> { - Map timingStatsByJobId = - Arrays.stream(searchResponse.getHits().getHits()) - .map(hit -> parseSearchHit(hit, DatafeedTimingStats.PARSER, errorHandler)) - .collect(Collectors.toUnmodifiableMap(DatafeedTimingStats::getJobId, Function.identity())); + msearchRequest, + ActionListener.wrap( + msearchResponse -> { + Map timingStatsByJobId = new HashMap<>(); + for (int i = 0; i < msearchResponse.getResponses().length; i++) { + String jobId = jobIds.get(i); + MultiSearchResponse.Item itemResponse = msearchResponse.getResponses()[i]; + if (itemResponse.isFailure()) { + errorHandler.accept(itemResponse.getFailure()); + } else { + SearchResponse searchResponse = itemResponse.getResponse(); + ShardSearchFailure[] shardFailures = searchResponse.getShardFailures(); + int unavailableShards = searchResponse.getTotalShards() - searchResponse.getSuccessfulShards(); + if (shardFailures != null && shardFailures.length > 0) { + LOGGER.error("[{}] Search request returned shard failures: {}", jobId, Arrays.toString(shardFailures)); + errorHandler.accept( + new ElasticsearchException(ExceptionsHelper.shardFailuresToErrorMsg(jobId, shardFailures))); + } else if (unavailableShards > 0) { + errorHandler.accept( + new ElasticsearchException( + "[" + jobId + "] Search request encountered [" + unavailableShards + "] unavailable shards")); + } else { + SearchHits hits = searchResponse.getHits(); + long hitsCount = hits.getHits().length; + if (hitsCount == 0) { + SearchRequest searchRequest = msearchRequest.requests().get(i); + LOGGER.debug("Found 0 hits for [{}]", new Object[]{searchRequest.indices()}); + } else if (hitsCount > 1) { + SearchRequest searchRequest = msearchRequest.requests().get(i); + LOGGER.debug("Found multiple hits for [{}]", new Object[]{searchRequest.indices()}); + } else { + assert hitsCount == 1; + SearchHit hit = hits.getHits()[0]; + DatafeedTimingStats timingStats = parseSearchHit(hit, DatafeedTimingStats.PARSER, errorHandler); + timingStatsByJobId.put(jobId, timingStats); + } + } + } + } handler.accept(timingStatsByJobId); }, errorHandler ), - client::search); + client::multiSearch); } public void datafeedTimingStats(String jobId, Consumer handler, Consumer errorHandler) { @@ -475,18 +509,19 @@ public void datafeedTimingStats(String jobId, Consumer hand searchSingleResult( jobId, DatafeedTimingStats.TYPE.getPreferredName(), - createDatafeedTimingStatsSearch(indexName, jobId), + createLatestDatafeedTimingStatsSearch(indexName, jobId), DatafeedTimingStats.PARSER, result -> handler.accept(result.result), errorHandler, () -> new DatafeedTimingStats(jobId)); } - private SearchRequestBuilder createDatafeedTimingStatsSearch(String indexName, String jobId) { + private SearchRequestBuilder createLatestDatafeedTimingStatsSearch(String indexName, String jobId) { return client.prepareSearch(indexName) .setSize(1) .setIndicesOptions(IndicesOptions.lenientExpandOpen()) - .setQuery(QueryBuilders.idsQuery().addIds(DatafeedTimingStats.documentId(jobId))); + .setQuery(QueryBuilders.idsQuery().addIds(DatafeedTimingStats.documentId(jobId))) + .addSort(SortBuilders.fieldSort(DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName()).order(SortOrder.DESC)); } public void getAutodetectParams(Job job, Consumer consumer, Consumer errorHandler) { @@ -515,7 +550,7 @@ public void getAutodetectParams(Job job, Consumer consumer, Co MultiSearchRequestBuilder msearch = client.prepareMultiSearch() .add(createLatestDataCountsSearch(resultsIndex, jobId)) .add(createLatestModelSizeStatsSearch(resultsIndex)) - .add(createTimingStatsSearch(resultsIndex, jobId)) + .add(createLatestTimingStatsSearch(resultsIndex, jobId)) // These next two document IDs never need to be the legacy ones due to the rule // that you cannot open a 5.4 job in a subsequent version of the product .add(createDocIdSearch(resultsIndex, ModelSnapshot.documentId(jobId, job.getModelSnapshotId()))) @@ -572,7 +607,7 @@ private SearchRequestBuilder createDocIdSearch(String index, String id) { .setRouting(id); } - private void parseAutodetectParamSearchHit(String jobId, AutodetectParams.Builder paramsBuilder, SearchHit hit, + private static void parseAutodetectParamSearchHit(String jobId, AutodetectParams.Builder paramsBuilder, SearchHit hit, Consumer errorHandler) { String hitId = hit.getId(); if (DataCounts.documentId(jobId).equals(hitId)) { @@ -594,7 +629,7 @@ private void parseAutodetectParamSearchHit(String jobId, AutodetectParams.Builde } } - private T parseSearchHit(SearchHit hit, BiFunction objectParser, + private static T parseSearchHit(SearchHit hit, BiFunction objectParser, Consumer errorHandler) { BytesReference source = hit.getSourceRef(); try (InputStream stream = source.streamInput(); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java index 0b78077d02ec1..869ebaec38eb8 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java @@ -7,10 +7,14 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; -import org.elasticsearch.xpack.core.watcher.watch.ClockMock; import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.junit.Before; +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.time.ZoneId; + import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; @@ -21,13 +25,14 @@ public class DatafeedTimingStatsReporterTests extends ESTestCase { private static final String JOB_ID = "my-job-id"; + private static final Duration ONE_SECOND = Duration.ofSeconds(1); - private ClockMock clock; + private FakeClock clock; private JobResultsPersister jobResultsPersister; @Before public void setUpTests() { - clock = new ClockMock(); + clock = new FakeClock(); jobResultsPersister = mock(JobResultsPersister.class); } @@ -36,28 +41,49 @@ public void testExecuteWithReporting() { DatafeedTimingStatsReporter timingStatsReporter = new DatafeedTimingStatsReporter(timingStats, clock, jobResultsPersister); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 10000.0))); - timingStatsReporter.executeWithReporting(this::advanceTime, 1); + timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 11000.0))); verifyZeroInteractions(jobResultsPersister); - timingStatsReporter.executeWithReporting(this::advanceTime, 1); + timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 12000.0))); verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 12000.0)); verifyNoMoreInteractions(jobResultsPersister); - timingStatsReporter.executeWithReporting(this::advanceTime, 1); + timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 13000.0))); verifyZeroInteractions(jobResultsPersister); - timingStatsReporter.executeWithReporting(this::advanceTime, 1); + timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 14000.0))); verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 14000.0)); verifyNoMoreInteractions(jobResultsPersister); } - private Void advanceTime(int seconds) { - clock.fastForwardSeconds(seconds); - return null; + /** Mutable clock that allows advancing current time. */ + private static final class FakeClock extends Clock { + + private Instant instant = Instant.EPOCH; + + @Override + public Instant instant() { + return instant; + } + + Void advanceTime(Duration duration) { + instant = instant.plus(duration); + return null; + } + + @Override + public ZoneId getZone() { + throw new UnsupportedOperationException(); + } + + @Override + public Clock withZone(ZoneId zone) { + throw new UnsupportedOperationException(); + } } public void testTimingStatsDifferSignificantly() { @@ -73,5 +99,21 @@ public void testTimingStatsDifferSignificantly() { DatafeedTimingStatsReporter.differSignificantly( new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1120.0)), is(true)); + assertThat( + DatafeedTimingStatsReporter.differSignificantly( + new DatafeedTimingStats(JOB_ID, 10000.0), new DatafeedTimingStats(JOB_ID, 11000.0)), + is(false)); + assertThat( + DatafeedTimingStatsReporter.differSignificantly( + new DatafeedTimingStats(JOB_ID, 10000.0), new DatafeedTimingStats(JOB_ID, 11200.0)), + is(true)); + assertThat( + DatafeedTimingStatsReporter.differSignificantly( + new DatafeedTimingStats(JOB_ID, 100000.0), new DatafeedTimingStats(JOB_ID, 110000.0)), + is(false)); + assertThat( + DatafeedTimingStatsReporter.differSignificantly( + new DatafeedTimingStats(JOB_ID, 100000.0), new DatafeedTimingStats(JOB_ID, 110001.0)), + is(true)); } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java index 57b5a096c17d3..778aae3304b49 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java @@ -11,7 +11,9 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; +import org.elasticsearch.action.search.MultiSearchAction; import org.elasticsearch.action.search.MultiSearchRequest; +import org.elasticsearch.action.search.MultiSearchRequestBuilder; import org.elasticsearch.action.search.MultiSearchResponse; import org.elasticsearch.action.search.SearchAction; import org.elasticsearch.action.search.SearchRequest; @@ -64,12 +66,12 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import static org.elasticsearch.xpack.core.ml.job.config.JobTests.buildJobBuilder; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; @@ -882,33 +884,57 @@ public void testTimingStats_NotFound() throws IOException { } public void testDatafeedTimingStats_MultipleDocumentsAtOnce() throws IOException { - String indexName = AnomalyDetectorsIndex.jobResultsIndexName(); - List> source = + List> sourceFoo = Arrays.asList( Map.of( Job.ID.getPreferredName(), "foo", - DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), 666.0), + DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), 666.0)); + List> sourceBar = + Arrays.asList( Map.of( Job.ID.getPreferredName(), "bar", DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), 777.0)); - SearchResponse response = createSearchResponse(source); - Client client = getMockedClient( - queryBuilder -> assertThat(queryBuilder.getName(), equalTo("ids")), - response); + SearchResponse responseFoo = createSearchResponse(sourceFoo); + SearchResponse responseBar = createSearchResponse(sourceBar); + MultiSearchResponse multiSearchResponse = new MultiSearchResponse( + new MultiSearchResponse.Item[]{ + new MultiSearchResponse.Item(responseFoo, null), + new MultiSearchResponse.Item(responseBar, null)}, + randomNonNegativeLong()); + + Client client = getBasicMockedClient(); + when(client.prepareMultiSearch()).thenReturn(new MultiSearchRequestBuilder(client, MultiSearchAction.INSTANCE)); + doAnswer(invocationOnMock -> { + MultiSearchRequest multiSearchRequest = (MultiSearchRequest) invocationOnMock.getArguments()[0]; + assertThat(multiSearchRequest.requests(), hasSize(2)); + assertThat(multiSearchRequest.requests().get(0).source().query().getName(), equalTo("ids")); + assertThat(multiSearchRequest.requests().get(1).source().query().getName(), equalTo("ids")); + @SuppressWarnings("unchecked") + ActionListener actionListener = (ActionListener) invocationOnMock.getArguments()[1]; + actionListener.onResponse(multiSearchResponse); + return null; + }).when(client).multiSearch(any(), any()); + when(client.prepareSearch(AnomalyDetectorsIndex.jobResultsAliasedName("foo"))) + .thenReturn( + new SearchRequestBuilder(client, SearchAction.INSTANCE).setIndices(AnomalyDetectorsIndex.jobResultsAliasedName("foo"))); + when(client.prepareSearch(AnomalyDetectorsIndex.jobResultsAliasedName("bar"))) + .thenReturn( + new SearchRequestBuilder(client, SearchAction.INSTANCE).setIndices(AnomalyDetectorsIndex.jobResultsAliasedName("bar"))); - when(client.prepareSearch(indexName)).thenReturn(new SearchRequestBuilder(client, SearchAction.INSTANCE).setIndices(indexName)); JobResultsProvider provider = createProvider(client); provider.datafeedTimingStats( - Set.of("foo", "bar"), + List.of("foo", "bar"), statsByJobId -> assertThat( statsByJobId, equalTo(Map.of("foo", new DatafeedTimingStats("foo", 666.0), "bar", new DatafeedTimingStats("bar", 777.0)))), e -> { throw new AssertionError(); }); - verify(client).prepareSearch(indexName); verify(client).threadPool(); - verify(client).search(any(SearchRequest.class), any(ActionListener.class)); + verify(client).prepareMultiSearch(); + verify(client).multiSearch(any(MultiSearchRequest.class), any(ActionListener.class)); + verify(client).prepareSearch(AnomalyDetectorsIndex.jobResultsAliasedName("foo")); + verify(client).prepareSearch(AnomalyDetectorsIndex.jobResultsAliasedName("bar")); verifyNoMoreInteractions(client); } @@ -988,11 +1014,16 @@ private static SearchResponse createSearchResponse(List> sou return response; } - private Client getMockedClient(Consumer queryBuilderConsumer, SearchResponse response) { + private Client getBasicMockedClient() { Client client = mock(Client.class); ThreadPool threadPool = mock(ThreadPool.class); when(client.threadPool()).thenReturn(threadPool); when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); + return client; + } + + private Client getMockedClient(Consumer queryBuilderConsumer, SearchResponse response) { + Client client = getBasicMockedClient(); doAnswer(invocationOnMock -> { MultiSearchRequest multiSearchRequest = (MultiSearchRequest) invocationOnMock.getArguments()[0]; queryBuilderConsumer.accept(multiSearchRequest.requests().get(0).source().query()); From 3e5ded9db991a6425ffa700d3c462fe55325640b Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Wed, 26 Jun 2019 12:39:43 +0200 Subject: [PATCH 05/16] Add missing mappings --- .../ml/job/persistence/ElasticsearchMappings.java | 14 ++++++++++++++ .../core/ml/job/results/ReservedFieldNames.java | 3 +++ .../persistence/ElasticsearchMappingsTests.java | 2 ++ 3 files changed, 19 insertions(+) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java index 0fc7770758ad3..92dc4677cc13b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java @@ -25,6 +25,7 @@ import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.xpack.core.ml.datafeed.ChunkingConfig; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.datafeed.DelayedDataCheckConfig; import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsConfig; import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsDest; @@ -510,6 +511,7 @@ public static XContentBuilder resultsMapping(String mappingType, Collection expected = collectResultsDocFieldNames(); expected.removeAll(overridden); From b7b4d5df0485cc42cd9ff2ef1ca681ce69d1d199 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Wed, 26 Jun 2019 13:17:53 +0200 Subject: [PATCH 06/16] Make datafeedTimingStats method behave correctly in the presence of empty job list --- .../ml/job/persistence/JobResultsProvider.java | 4 ++++ .../ml/job/persistence/JobResultsProviderTests.java | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java index daaf6654bc5ca..ea0657a914f32 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java @@ -448,6 +448,10 @@ private SearchRequestBuilder createLatestTimingStatsSearch(String indexName, Str public void datafeedTimingStats(List jobIds, Consumer> handler, Consumer errorHandler) { + if (jobIds.isEmpty()) { + handler.accept(Map.of()); + return; + } MultiSearchRequestBuilder msearchRequestBuilder = client.prepareMultiSearch(); for (String jobId : jobIds) { String indexName = AnomalyDetectorsIndex.jobResultsAliasedName(jobId); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java index 778aae3304b49..500662b48b393 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java @@ -79,6 +79,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; public class JobResultsProviderTests extends ESTestCase { @@ -883,6 +884,18 @@ public void testTimingStats_NotFound() throws IOException { verifyNoMoreInteractions(client); } + public void testDatafeedTimingStats_EmptyJobList() throws IOException { + Client client = getBasicMockedClient(); + + JobResultsProvider provider = createProvider(client); + provider.datafeedTimingStats( + List.of(), + statsByJobId -> assertThat(statsByJobId, equalTo(Map.of())), + e -> { throw new AssertionError(); }); + + verifyZeroInteractions(client); + } + public void testDatafeedTimingStats_MultipleDocumentsAtOnce() throws IOException { List> sourceFoo = Arrays.asList( From 9488f83aa1a4f53cce3d3d65eede96bbb0668cf0 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Wed, 26 Jun 2019 16:30:48 +0200 Subject: [PATCH 07/16] Small cleanup --- .../persistence/JobResultsProviderTests.java | 5 +++-- .../test/ml/get_datafeed_stats.yml | 18 +++++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java index 500662b48b393..fd01720160280 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java @@ -70,6 +70,7 @@ import java.util.function.Consumer; import static org.elasticsearch.xpack.core.ml.job.config.JobTests.buildJobBuilder; +import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.mockito.Matchers.any; @@ -884,13 +885,13 @@ public void testTimingStats_NotFound() throws IOException { verifyNoMoreInteractions(client); } - public void testDatafeedTimingStats_EmptyJobList() throws IOException { + public void testDatafeedTimingStats_EmptyJobList() { Client client = getBasicMockedClient(); JobResultsProvider provider = createProvider(client); provider.datafeedTimingStats( List.of(), - statsByJobId -> assertThat(statsByJobId, equalTo(Map.of())), + statsByJobId -> assertThat(statsByJobId, anEmptyMap()), e -> { throw new AssertionError(); }); verifyZeroInteractions(client); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml index d83f10d7f290b..b3ac8b92370cd 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml @@ -148,8 +148,8 @@ setup: - do: ml.start_datafeed: - "datafeed_id": "datafeed-1" - "start": 0 + datafeed_id: "datafeed-1" + start: 0 - do: ml.get_datafeed_stats: @@ -169,8 +169,8 @@ setup: - do: ml.start_datafeed: - "datafeed_id": "datafeed-1" - "start": 0 + datafeed_id: "datafeed-1" + start: 0 - do: ml.get_datafeed_stats: @@ -183,7 +183,7 @@ setup: - do: ml.stop_datafeed: - "datafeed_id": "datafeed-1" + datafeed_id: "datafeed-1" - do: ml.get_datafeed_stats: @@ -203,8 +203,8 @@ setup: - do: ml.start_datafeed: - "datafeed_id": "datafeed-1" - "start": 0 + datafeed_id: "datafeed-1" + start: 0 - do: ml.open_job: @@ -212,8 +212,8 @@ setup: - do: ml.start_datafeed: - "datafeed_id": "datafeed-2" - "start": 0 + datafeed_id: "datafeed-2" + start: 0 - do: ml.get_datafeed_stats: {} From 7f54e89a19f8b7e12769f0e8b1f0d28338747b1d Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Thu, 27 Jun 2019 08:41:26 +0200 Subject: [PATCH 08/16] Apply review comments --- .../xpack/core/ml/datafeed/DatafeedTimingStats.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java index 8bcedc7a7dbfa..e606f7c59ec27 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java @@ -57,7 +57,7 @@ public DatafeedTimingStats(String jobId) { public DatafeedTimingStats(StreamInput in) throws IOException { jobId = in.readString(); - totalSearchTimeMs = in.readOptionalDouble(); + totalSearchTimeMs = in.readDouble(); } public DatafeedTimingStats(DatafeedTimingStats other) { @@ -79,7 +79,7 @@ public void incrementTotalSearchTimeMs(double totalSearchTimeMs) { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(jobId); - out.writeOptionalDouble(totalSearchTimeMs); + out.writeDouble(totalSearchTimeMs); } @Override From 5a306e0c59d6a1b8425bb1c1ca5f30f597c717c1 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Thu, 27 Jun 2019 20:55:59 +0200 Subject: [PATCH 09/16] Small improvements --- .../core/ml/datafeed/DatafeedTimingStats.java | 4 ++-- .../ml/datafeed/DatafeedTimingStatsReporter.java | 11 +++++++---- .../DatafeedTimingStatsReporterTests.java | 16 +++++++--------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java index e606f7c59ec27..3e934056bcc10 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java @@ -72,8 +72,8 @@ public double getTotalSearchTimeMs() { return totalSearchTimeMs; } - public void incrementTotalSearchTimeMs(double totalSearchTimeMs) { - this.totalSearchTimeMs += totalSearchTimeMs; + public void incrementTotalSearchTimeMs(double searchTimeMs) { + this.totalSearchTimeMs += searchTimeMs; } @Override diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java index 1d9728cf232dd..cdd731a83aa9a 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java @@ -49,12 +49,15 @@ private void reportSearchDuration(Duration searchDuration) { double searchDurationMs = searchDuration.toMillis(); currentTimingStats.incrementTotalSearchTimeMs(searchDurationMs); if (differSignificantly(currentTimingStats, persistedTimingStats)) { - jobResultsPersister.persistDatafeedTimingStats(currentTimingStats); - persistedTimingStats = currentTimingStats; - currentTimingStats = new DatafeedTimingStats(persistedTimingStats); + flush(); } } + private void flush() { + persistedTimingStats = new DatafeedTimingStats(currentTimingStats); + jobResultsPersister.persistDatafeedTimingStats(persistedTimingStats); + } + /** * Returns true if given stats objects differ from each other by more than 10% for at least one of the statistics. */ @@ -82,5 +85,5 @@ private static boolean differSignificantly(double value1, double value2) { * Maximum absolute difference of values that is interpreted as values being similar. * If the values absolute difference is greater than MAX_VALID_ABS_DIFFERENCE, the values are interpreted as significantly different. */ - private static final double MAX_VALID_ABS_DIFFERENCE_MS = 10000.0; + private static final double MAX_VALID_ABS_DIFFERENCE_MS = 10000.0; // 10s } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java index 869ebaec38eb8..457f5ea5e5c0e 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java @@ -9,6 +9,7 @@ import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.junit.Before; +import org.mockito.InOrder; import java.time.Clock; import java.time.Duration; @@ -17,10 +18,8 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; public class DatafeedTimingStatsReporterTests extends ESTestCase { @@ -43,21 +42,20 @@ public void testExecuteWithReporting() { timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 11000.0))); - verifyZeroInteractions(jobResultsPersister); timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 12000.0))); - verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 12000.0)); - verifyNoMoreInteractions(jobResultsPersister); timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 13000.0))); - verifyZeroInteractions(jobResultsPersister); timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 14000.0))); - verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 14000.0)); - verifyNoMoreInteractions(jobResultsPersister); + + InOrder inOrder = inOrder(jobResultsPersister); + inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 12000.0)); + inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 14000.0)); + inOrder.verifyNoMoreInteractions(); } /** Mutable clock that allows advancing current time. */ From 8c54d93f9ae58947e543721c053ed5184ba6d1ef Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Fri, 28 Jun 2019 09:53:51 +0200 Subject: [PATCH 10/16] Report actual (server-side) search time rather than client-side wall time --- .../TransportPreviewDatafeedAction.java | 3 +- .../action/TransportStartDatafeedAction.java | 3 +- .../xpack/ml/datafeed/DatafeedJobBuilder.java | 3 +- .../datafeed/DatafeedTimingStatsReporter.java | 28 ++++------- .../AbstractAggregationDataExtractor.java | 7 +-- .../chunked/ChunkedDataExtractor.java | 14 +++--- .../extractor/scroll/ScrollDataExtractor.java | 16 ++----- .../DatafeedTimingStatsReporterTests.java | 48 ++++--------------- .../AggregationDataExtractorTests.java | 7 +-- .../chunked/ChunkedDataExtractorTests.java | 4 +- .../scroll/ScrollDataExtractorTests.java | 6 +-- 11 files changed, 40 insertions(+), 99 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPreviewDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPreviewDatafeedAction.java index d2d2ffd4c07ce..7e76fc0c7cecb 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPreviewDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPreviewDatafeedAction.java @@ -31,7 +31,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; -import java.time.Clock; import java.util.Map; import java.util.Optional; import java.util.function.Supplier; @@ -86,7 +85,7 @@ protected void doExecute(Task task, PreviewDatafeedAction.Request request, Actio previewDatafeed.build(), jobBuilder.build(), xContentRegistry, - new DatafeedTimingStatsReporter(timingStats, Clock.systemUTC(), jobResultsPersister), + new DatafeedTimingStatsReporter(timingStats, jobResultsPersister), new ActionListener<>() { @Override public void onResponse(DataExtractorFactory dataExtractorFactory) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java index e79c2eecc47b9..fceabfa9e99e7 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java @@ -60,7 +60,6 @@ import org.elasticsearch.xpack.ml.notifications.Auditor; import java.io.IOException; -import java.time.Clock; import java.util.ArrayList; import java.util.List; import java.util.Locale; @@ -258,7 +257,7 @@ private void createDataExtractor(Job job, DatafeedConfig datafeed, StartDatafeed job, xContentRegistry, // Creating fake {@link TimingStatsReporter} so that search API call is not needed. - new DatafeedTimingStatsReporter(new DatafeedTimingStats(job.getId()), Clock.systemUTC(), jobResultsPersister), + new DatafeedTimingStatsReporter(new DatafeedTimingStats(job.getId()), jobResultsPersister), ActionListener.wrap( unused -> persistentTasksService.sendStartRequest( diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilder.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilder.java index 66687a270ca3d..0689b9774b56a 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilder.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobBuilder.java @@ -29,7 +29,6 @@ import org.elasticsearch.xpack.ml.job.persistence.JobResultsProvider; import org.elasticsearch.xpack.ml.notifications.Auditor; -import java.time.Clock; import java.util.Collections; import java.util.Objects; import java.util.concurrent.atomic.AtomicReference; @@ -98,7 +97,7 @@ void build(String datafeedId, ActionListener listener) { datafeedConfigHolder.get(), jobHolder.get(), xContentRegistry, - new DatafeedTimingStatsReporter(timingStats, Clock.systemUTC(), jobResultsPersister), + new DatafeedTimingStatsReporter(timingStats, jobResultsPersister), dataExtractorFactoryHandler); }; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java index cdd731a83aa9a..7eb25b04b8667 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java @@ -5,27 +5,22 @@ */ package org.elasticsearch.xpack.ml.datafeed; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; -import java.time.Clock; -import java.time.Duration; -import java.time.Instant; import java.util.Objects; -import java.util.function.Function; public class DatafeedTimingStatsReporter { - private final Clock clock; private final JobResultsPersister jobResultsPersister; private DatafeedTimingStats persistedTimingStats; private volatile DatafeedTimingStats currentTimingStats; - public DatafeedTimingStatsReporter(DatafeedTimingStats timingStats, Clock clock, JobResultsPersister jobResultsPersister) { + public DatafeedTimingStatsReporter(DatafeedTimingStats timingStats, JobResultsPersister jobResultsPersister) { Objects.requireNonNull(timingStats); this.persistedTimingStats = new DatafeedTimingStats(timingStats); this.currentTimingStats = new DatafeedTimingStats(timingStats); - this.clock = Objects.requireNonNull(clock); this.jobResultsPersister = Objects.requireNonNull(jobResultsPersister); } @@ -34,20 +29,13 @@ public DatafeedTimingStats getCurrentTimingStats() { } /** - * Executes the given function and reports how much time did the execution take. + * Reports how much time did the search request execution take. */ - public R executeWithReporting(Function function, T argument) { - Instant before = clock.instant(); - R result = function.apply(argument); - Instant after = clock.instant(); - Duration duration = Duration.between(before, after); - reportSearchDuration(duration); - return result; - } - - private void reportSearchDuration(Duration searchDuration) { - double searchDurationMs = searchDuration.toMillis(); - currentTimingStats.incrementTotalSearchTimeMs(searchDurationMs); + public void reportSearchDuration(TimeValue searchDuration) { + if (searchDuration == null) { + return; + } + currentTimingStats.incrementTotalSearchTimeMs(searchDuration.millis()); if (differSignificantly(currentTimingStats, persistedTimingStats)) { flush(); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AbstractAggregationDataExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AbstractAggregationDataExtractor.java index af94b1f8f4f43..3559f3a7c50a2 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AbstractAggregationDataExtractor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AbstractAggregationDataExtractor.java @@ -109,8 +109,9 @@ public Optional next() throws IOException { private Aggregations search() throws IOException { LOGGER.debug("[{}] Executing aggregated search", context.jobId); - SearchResponse searchResponse = executeSearchRequestWithReporting(buildSearchRequest(buildBaseSearchSource())); + SearchResponse searchResponse = executeSearchRequest(buildSearchRequest(buildBaseSearchSource())); LOGGER.debug("[{}] Search response was obtained", context.jobId); + timingStatsReporter.reportSearchDuration(searchResponse.getTook()); ExtractorUtils.checkSearchWasSuccessful(context.jobId, searchResponse); return validateAggs(searchResponse.getAggregations()); } @@ -121,10 +122,6 @@ private void initAggregationProcessor(Aggregations aggs) throws IOException { aggregationToJsonProcessor.process(aggs); } - private SearchResponse executeSearchRequestWithReporting(T searchRequestBuilder) { - return timingStatsReporter.executeWithReporting(this::executeSearchRequest, searchRequestBuilder); - } - protected SearchResponse executeSearchRequest(T searchRequestBuilder) { return ClientHelper.executeWithHeaders(context.headers, ClientHelper.ML_ORIGIN, client, searchRequestBuilder::get); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java index ad3d6c90e385e..c64aae1916b5a 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java @@ -129,10 +129,6 @@ private void setUpChunkedSearch() throws IOException { } } - private SearchResponse executeSearchRequestWithReporting(ActionRequestBuilder searchRequestBuilder) { - return timingStatsReporter.executeWithReporting(this::executeSearchRequest, searchRequestBuilder); - } - protected SearchResponse executeSearchRequest(ActionRequestBuilder searchRequestBuilder) { return ClientHelper.executeWithHeaders(context.headers, ClientHelper.ML_ORIGIN, client, searchRequestBuilder::get); } @@ -209,8 +205,9 @@ private DataSummary buildDataSummary() throws IOException { private DataSummary newScrolledDataSummary() throws IOException { SearchRequestBuilder searchRequestBuilder = rangeSearchRequest(); - SearchResponse searchResponse = executeSearchRequestWithReporting(searchRequestBuilder); + SearchResponse searchResponse = executeSearchRequest(searchRequestBuilder); LOGGER.debug("[{}] Scrolling Data summary response was obtained", context.jobId); + timingStatsReporter.reportSearchDuration(searchResponse.getTook()); ExtractorUtils.checkSearchWasSuccessful(context.jobId, searchResponse); @@ -231,12 +228,13 @@ private DataSummary newAggregatedDataSummary() throws IOException { // TODO: once RollupSearchAction is changed from indices:admin* to indices:data/read/* this branch is not needed ActionRequestBuilder searchRequestBuilder = dataExtractorFactory instanceof RollupDataExtractorFactory ? rollupRangeSearchRequest() : rangeSearchRequest(); - SearchResponse response = executeSearchRequestWithReporting(searchRequestBuilder); + SearchResponse searchResponse = executeSearchRequest(searchRequestBuilder); LOGGER.debug("[{}] Aggregating Data summary response was obtained", context.jobId); + timingStatsReporter.reportSearchDuration(searchResponse.getTook()); - ExtractorUtils.checkSearchWasSuccessful(context.jobId, response); + ExtractorUtils.checkSearchWasSuccessful(context.jobId, searchResponse); - Aggregations aggregations = response.getAggregations(); + Aggregations aggregations = searchResponse.getAggregations(); Min min = aggregations.get(EARLIEST_TIME); Max max = aggregations.get(LATEST_TIME); return new AggregatedDataSummary(min.getValue(), max.getValue(), context.histogramInterval); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractor.java index 26ca38913221c..ea3bf44469862 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractor.java @@ -110,15 +110,12 @@ private Optional tryNextStream() throws IOException { protected InputStream initScroll(long startTimestamp) throws IOException { LOGGER.debug("[{}] Initializing scroll", context.jobId); - SearchResponse searchResponse = executeSearchRequestWithReporting(buildSearchRequest(startTimestamp)); + SearchResponse searchResponse = executeSearchRequest(buildSearchRequest(startTimestamp)); LOGGER.debug("[{}] Search response was obtained", context.jobId); + timingStatsReporter.reportSearchDuration(searchResponse.getTook()); return processSearchResponse(searchResponse); } - private SearchResponse executeSearchRequestWithReporting(SearchRequestBuilder searchRequestBuilder) { - return timingStatsReporter.executeWithReporting(this::executeSearchRequest, searchRequestBuilder); - } - protected SearchResponse executeSearchRequest(SearchRequestBuilder searchRequestBuilder) { return ClientHelper.executeWithHeaders(context.headers, ClientHelper.ML_ORIGIN, client, searchRequestBuilder::get); } @@ -190,18 +187,19 @@ private InputStream continueScroll() throws IOException { LOGGER.debug("[{}] Continuing scroll with id [{}]", context.jobId, scrollId); SearchResponse searchResponse; try { - searchResponse = executeSearchScrollRequestWithReporting(scrollId); + searchResponse = executeSearchScrollRequest(scrollId); } catch (SearchPhaseExecutionException searchExecutionException) { if (searchHasShardFailure == false) { LOGGER.debug("[{}] Reinitializing scroll due to SearchPhaseExecutionException", context.jobId); markScrollAsErrored(); searchResponse = - executeSearchRequestWithReporting(buildSearchRequest(lastTimestamp == null ? context.start : lastTimestamp)); + executeSearchRequest(buildSearchRequest(lastTimestamp == null ? context.start : lastTimestamp)); } else { throw searchExecutionException; } } LOGGER.debug("[{}] Search response was obtained", context.jobId); + timingStatsReporter.reportSearchDuration(searchResponse.getTook()); return processSearchResponse(searchResponse); } @@ -215,10 +213,6 @@ private void markScrollAsErrored() { searchHasShardFailure = true; } - private SearchResponse executeSearchScrollRequestWithReporting(String scrollId) { - return timingStatsReporter.executeWithReporting(this::executeSearchScrollRequest, scrollId); - } - protected SearchResponse executeSearchScrollRequest(String scrollId) { return ClientHelper.executeWithHeaders(context.headers, ClientHelper.ML_ORIGIN, client, () -> new SearchScrollRequestBuilder(client, SearchScrollAction.INSTANCE) diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java index 457f5ea5e5c0e..1ed6400bc4b7d 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java @@ -5,17 +5,13 @@ */ package org.elasticsearch.xpack.ml.datafeed; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; import org.junit.Before; import org.mockito.InOrder; -import java.time.Clock; -import java.time.Duration; -import java.time.Instant; -import java.time.ZoneId; - import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.inOrder; @@ -24,32 +20,30 @@ public class DatafeedTimingStatsReporterTests extends ESTestCase { private static final String JOB_ID = "my-job-id"; - private static final Duration ONE_SECOND = Duration.ofSeconds(1); + private static final TimeValue ONE_SECOND = TimeValue.timeValueSeconds(1); - private FakeClock clock; private JobResultsPersister jobResultsPersister; @Before public void setUpTests() { - clock = new FakeClock(); jobResultsPersister = mock(JobResultsPersister.class); } - public void testExecuteWithReporting() { + public void testReportSearchDuration() { DatafeedTimingStats timingStats = new DatafeedTimingStats(JOB_ID, 10000.0); - DatafeedTimingStatsReporter timingStatsReporter = new DatafeedTimingStatsReporter(timingStats, clock, jobResultsPersister); + DatafeedTimingStatsReporter timingStatsReporter = new DatafeedTimingStatsReporter(timingStats, jobResultsPersister); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 10000.0))); - timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); + timingStatsReporter.reportSearchDuration(ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 11000.0))); - timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); + timingStatsReporter.reportSearchDuration(ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 12000.0))); - timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); + timingStatsReporter.reportSearchDuration(ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 13000.0))); - timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND); + timingStatsReporter.reportSearchDuration(ONE_SECOND); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 14000.0))); InOrder inOrder = inOrder(jobResultsPersister); @@ -58,32 +52,6 @@ public void testExecuteWithReporting() { inOrder.verifyNoMoreInteractions(); } - /** Mutable clock that allows advancing current time. */ - private static final class FakeClock extends Clock { - - private Instant instant = Instant.EPOCH; - - @Override - public Instant instant() { - return instant; - } - - Void advanceTime(Duration duration) { - instant = instant.plus(duration); - return null; - } - - @Override - public ZoneId getZone() { - throw new UnsupportedOperationException(); - } - - @Override - public Clock withZone(ZoneId zone) { - throw new UnsupportedOperationException(); - } - } - public void testTimingStatsDifferSignificantly() { assertThat( DatafeedTimingStatsReporter.differSignificantly( diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorTests.java index 33456ca742ce7..6d9db043755ad 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/aggregation/AggregationDataExtractorTests.java @@ -9,6 +9,7 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.ShardSearchFailure; import org.elasticsearch.client.Client; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.rest.RestStatus; @@ -28,7 +29,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; -import java.time.Clock; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -93,8 +93,7 @@ public void setUpTests() { .addAggregator(AggregationBuilders.histogram("time").field("time").interval(1000).subAggregation( AggregationBuilders.terms("airline").field("airline").subAggregation( AggregationBuilders.avg("responsetime").field("responsetime")))); - timingStatsReporter = - new DatafeedTimingStatsReporter(new DatafeedTimingStats(jobId), Clock.systemUTC(), mock(JobResultsPersister.class)); + timingStatsReporter = new DatafeedTimingStatsReporter(new DatafeedTimingStats(jobId), mock(JobResultsPersister.class)); } public void testExtraction() throws IOException { @@ -291,6 +290,7 @@ private SearchResponse createSearchResponse(Aggregations aggregations) { when(searchResponse.status()).thenReturn(RestStatus.OK); when(searchResponse.getScrollId()).thenReturn(randomAlphaOfLength(1000)); when(searchResponse.getAggregations()).thenReturn(aggregations); + when(searchResponse.getTook()).thenReturn(TimeValue.timeValueMillis(randomNonNegativeLong())); return searchResponse; } @@ -313,6 +313,7 @@ private SearchResponse createResponseWithUnavailableShards(int unavailableShards when(searchResponse.status()).thenReturn(RestStatus.OK); when(searchResponse.getSuccessfulShards()).thenReturn(3); when(searchResponse.getTotalShards()).thenReturn(3 + unavailableShards); + when(searchResponse.getTook()).thenReturn(TimeValue.timeValueMillis(randomNonNegativeLong())); return searchResponse; } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorTests.java index 378bb59ad08c9..77ebc9651ddd7 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorTests.java @@ -32,7 +32,6 @@ import java.io.IOException; import java.io.InputStream; -import java.time.Clock; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -94,8 +93,7 @@ public void setUpTests() { scrollSize = 1000; chunkSpan = null; dataExtractorFactory = mock(DataExtractorFactory.class); - timingStatsReporter = - new DatafeedTimingStatsReporter(new DatafeedTimingStats(jobId), Clock.systemUTC(), mock(JobResultsPersister.class)); + timingStatsReporter = new DatafeedTimingStatsReporter(new DatafeedTimingStats(jobId), mock(JobResultsPersister.class)); } public void testExtractionGivenNoData() throws IOException { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java index e56c241e2e4f0..eb1542ae814df 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.client.Client; import org.elasticsearch.common.document.DocumentField; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; @@ -41,7 +42,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; -import java.time.Clock; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -145,8 +145,7 @@ public void setUpTests() { clearScrollFuture = mock(ActionFuture.class); capturedClearScrollRequests = ArgumentCaptor.forClass(ClearScrollRequest.class); when(client.execute(same(ClearScrollAction.INSTANCE), capturedClearScrollRequests.capture())).thenReturn(clearScrollFuture); - timingStatsReporter = - new DatafeedTimingStatsReporter(new DatafeedTimingStats(jobId), Clock.systemUTC(), mock(JobResultsPersister.class)); + timingStatsReporter = new DatafeedTimingStatsReporter(new DatafeedTimingStats(jobId), mock(JobResultsPersister.class)); } public void testSinglePageExtraction() throws IOException { @@ -513,6 +512,7 @@ private SearchResponse createSearchResponse(List timestamps, List SearchHits searchHits = new SearchHits(hits.toArray(new SearchHit[0]), new TotalHits(hits.size(), TotalHits.Relation.EQUAL_TO), 1); when(searchResponse.getHits()).thenReturn(searchHits); + when(searchResponse.getTook()).thenReturn(TimeValue.timeValueMillis(randomNonNegativeLong())); return searchResponse; } From 7e012c912f8b45e2d44808080eb53cbef7c35009 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Fri, 28 Jun 2019 14:31:32 +0200 Subject: [PATCH 11/16] Delete DatafeedTimingStats document on Datafeed deletion --- .../core/ml/datafeed/DatafeedConfig.java | 2 +- .../core/ml/datafeed/DatafeedTimingStats.java | 37 +++++++-- .../GetDatafeedStatsActionResponseTests.java | 3 +- .../ml/datafeed/DatafeedTimingStatsTests.java | 42 +++++++--- .../xpack/ml/integration/DatafeedJobsIT.java | 80 ++++++++++++++++++- .../action/TransportDeleteDatafeedAction.java | 55 ++++++++++++- .../datafeed/DatafeedTimingStatsReporter.java | 11 ++- .../job/persistence/TimingStatsReporter.java | 4 +- .../DatafeedTimingStatsReporterTests.java | 30 +++---- .../persistence/JobResultsPersisterTests.java | 3 +- .../persistence/JobResultsProviderTests.java | 7 +- .../test/ml/get_datafeed_stats.yml | 10 ++- 12 files changed, 234 insertions(+), 50 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java index 9285256c76819..9f45456feeee1 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java @@ -682,7 +682,7 @@ public DatafeedConfig build() { if (!MlStrings.isValidId(id)) { throw ExceptionsHelper.badRequestException(Messages.getMessage(Messages.INVALID_ID, ID.getPreferredName(), id)); } - if (indices == null || indices.isEmpty() || indices.contains(null) || indices.contains("")) { + if (indices == null || indices.isEmpty() || indices.contains("")) { throw invalidOptionValue(INDICES.getPreferredName(), indices); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java index 3e934056bcc10..443cb84ecdc0a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.ml.datafeed; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -24,6 +25,7 @@ public class DatafeedTimingStats implements ToXContentObject, Writeable { public static final ParseField JOB_ID = new ParseField("job_id"); + public static final ParseField SEARCH_COUNT = new ParseField("search_count"); public static final ParseField TOTAL_SEARCH_TIME_MS = new ParseField("total_search_time_ms"); public static final ParseField TYPE = new ParseField("datafeed_timing_stats"); @@ -33,8 +35,16 @@ public class DatafeedTimingStats implements ToXContentObject, Writeable { private static ConstructingObjectParser createParser() { ConstructingObjectParser parser = new ConstructingObjectParser<>( - "datafeed_timing_stats", true, args -> new DatafeedTimingStats((String) args[0], (double) args[1])); + "datafeed_timing_stats", + true, + args -> { + String jobId = (String) args[0]; + Long searchCount = (Long) args[1]; + Double totalSearchTimeMs = (Double) args[2]; + return new DatafeedTimingStats(jobId, getOrDefault(searchCount, 0L), getOrDefault(totalSearchTimeMs, 0.0)); + }); parser.declareString(constructorArg(), JOB_ID); + parser.declareLong(optionalConstructorArg(), SEARCH_COUNT); parser.declareDouble(optionalConstructorArg(), TOTAL_SEARCH_TIME_MS); return parser; } @@ -44,41 +54,50 @@ public static String documentId(String jobId) { } private final String jobId; + private long searchCount; private double totalSearchTimeMs; - public DatafeedTimingStats(String jobId, double totalSearchTimeMs) { + public DatafeedTimingStats(String jobId, long searchCount, double totalSearchTimeMs) { this.jobId = Objects.requireNonNull(jobId); + this.searchCount = searchCount; this.totalSearchTimeMs = totalSearchTimeMs; } public DatafeedTimingStats(String jobId) { - this(jobId, 0); + this(jobId, 0, 0); } public DatafeedTimingStats(StreamInput in) throws IOException { jobId = in.readString(); + searchCount = in.readLong(); totalSearchTimeMs = in.readDouble(); } public DatafeedTimingStats(DatafeedTimingStats other) { - this(other.jobId, other.totalSearchTimeMs); + this(other.jobId, other.searchCount, other.totalSearchTimeMs); } public String getJobId() { return jobId; } + public long getSearchCount() { + return searchCount; + } + public double getTotalSearchTimeMs() { return totalSearchTimeMs; } public void incrementTotalSearchTimeMs(double searchTimeMs) { + this.searchCount++; this.totalSearchTimeMs += searchTimeMs; } @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(jobId); + out.writeLong(searchCount); out.writeDouble(totalSearchTimeMs); } @@ -86,6 +105,7 @@ public void writeTo(StreamOutput out) throws IOException { public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { builder.startObject(); builder.field(JOB_ID.getPreferredName(), jobId); + builder.field(SEARCH_COUNT.getPreferredName(), searchCount); builder.field(TOTAL_SEARCH_TIME_MS.getPreferredName(), totalSearchTimeMs); builder.endObject(); return builder; @@ -102,16 +122,21 @@ public boolean equals(Object obj) { DatafeedTimingStats other = (DatafeedTimingStats) obj; return Objects.equals(this.jobId, other.jobId) - && Objects.equals(this.totalSearchTimeMs, other.totalSearchTimeMs); + && this.searchCount == other.searchCount + && this.totalSearchTimeMs == other.totalSearchTimeMs; } @Override public int hashCode() { - return Objects.hash(jobId, totalSearchTimeMs); + return Objects.hash(jobId, searchCount, totalSearchTimeMs); } @Override public String toString() { return Strings.toString(this); } + + private static T getOrDefault(@Nullable T value, T defaultValue) { + return value != null ? value : defaultValue; + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java index 1f56c4c387cc7..d7cfef1a135b7 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java @@ -77,7 +77,7 @@ public void testDatafeedStatsToXContent() throws IOException { Set.of(), Version.CURRENT); - DatafeedTimingStats timingStats = new DatafeedTimingStats("my-job-id", 123.456); + DatafeedTimingStats timingStats = new DatafeedTimingStats("my-job-id", 5, 123.456); Response.DatafeedStats stats = new Response.DatafeedStats("df-id", DatafeedState.STARTED, node, null, timingStats); @@ -110,6 +110,7 @@ public void testDatafeedStatsToXContent() throws IOException { Map timingStatsMap = (Map) dfStatsMap.get("timing_stats"); assertThat(timingStatsMap.size(), is(equalTo(2))); assertThat(timingStatsMap, hasEntry("job_id", "my-job-id")); + assertThat(timingStatsMap, hasEntry("search_count", 5)); assertThat(timingStatsMap, hasEntry("total_search_time_ms", 123.456)); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java index cb139e06ee1a5..9ecff4974a751 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStatsTests.java @@ -6,7 +6,10 @@ package org.elasticsearch.xpack.core.ml.datafeed; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.DeprecationHandler; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.AbstractSerializingTestCase; import java.io.IOException; @@ -18,7 +21,7 @@ public class DatafeedTimingStatsTests extends AbstractSerializingTestCase assertEquals(getJobStats(jobId).get(0).getState(), JobState.OPENED)); + + String datafeedId = jobId + "-datafeed"; + DatafeedConfig datafeedConfig = createDatafeed(datafeedId, jobId, List.of("data-1", "data-2")); + + registerDatafeed(datafeedConfig); + putDatafeed(datafeedConfig); + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, equalTo(0L)); + + startDatafeed(datafeedId, 0L, now.toEpochMilli()); + assertBusy(() -> { + DataCounts dataCounts = getDataCounts(jobId); + assertThat(dataCounts.getProcessedRecordCount(), equalTo(numDocs + numDocs2)); + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, greaterThan(0L)); + }, 60, TimeUnit.SECONDS); + + deleteDatafeed(datafeedId); + + registerDatafeed(datafeedConfig); + putDatafeed(datafeedConfig); + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, equalTo(0L)); + + openJob(jobId); + startDatafeed(datafeedId, 0L, now.toEpochMilli()); + assertBusy(() -> { + DataCounts dataCounts = getDataCounts(jobId); + assertThat(dataCounts.getProcessedRecordCount(), equalTo(numDocs + numDocs2)); + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, greaterThan(0L)); + }, 60, TimeUnit.SECONDS); + + waitUntilJobIsClosed(jobId); + } + + private void assertDatafeedStats(String datafeedId, DatafeedState state, String jobId, Matcher searchCountMatcher) { + GetDatafeedsStatsAction.Request request = new GetDatafeedsStatsAction.Request(datafeedId); + GetDatafeedsStatsAction.Response response = client().execute(GetDatafeedsStatsAction.INSTANCE, request).actionGet(); + assertThat(response.getResponse().results(), hasSize(1)); + GetDatafeedsStatsAction.Response.DatafeedStats stats = response.getResponse().results().get(0); + assertThat(stats.getDatafeedState(), equalTo(state)); + assertThat(stats.getTimingStats().getJobId(), equalTo(jobId)); + assertThat(stats.getTimingStats().getSearchCount(), searchCountMatcher); + } + public void testRealtime() throws Exception { String jobId = "realtime-job"; String datafeedId = jobId + "-datafeed"; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java index d731f1b3b08a2..09efa85d7cddb 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java @@ -7,7 +7,12 @@ import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.delete.DeleteAction; +import org.elasticsearch.action.delete.DeleteRequest; +import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.TransportMasterNodeAction; import org.elasticsearch.client.Client; @@ -29,7 +34,9 @@ import org.elasticsearch.xpack.core.ml.action.DeleteDatafeedAction; import org.elasticsearch.xpack.core.ml.action.IsolateDatafeedAction; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.messages.Messages; +import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.ml.MlConfigMigrationEligibilityCheck; import org.elasticsearch.xpack.ml.datafeed.persistence.DatafeedConfigProvider; @@ -144,10 +151,50 @@ private void deleteDatafeedConfig(DeleteDatafeedAction.Request request, ActionLi return; } - datafeedConfigProvider.deleteDatafeedConfig(request.getDatafeedId(), ActionListener.wrap( - deleteResponse -> listener.onResponse(new AcknowledgedResponse(true)), - listener::onFailure - )); + // Get datafeed config document + datafeedConfigProvider.getDatafeedConfig( + request.getDatafeedId(), + ActionListener.wrap( + datafeedConfigBuilder -> { + // Delete datafeed timing stats document + deleteDatafeedTimingStats( + datafeedConfigBuilder.build().getJobId(), + ActionListener.wrap( + unused1 -> { + // Delete datafeed config document + datafeedConfigProvider.deleteDatafeedConfig( + request.getDatafeedId(), + ActionListener.wrap( + unused2 -> listener.onResponse(new AcknowledgedResponse(true)), + listener::onFailure)); + }, + listener::onFailure)); + }, + listener::onFailure)); + } + + /** + * Delete the datafeed config document + * + * @param jobId The job id + * @param actionListener Deleted datafeed listener + */ + private void deleteDatafeedTimingStats(String jobId, ActionListener actionListener) { + DeleteRequest request = + new DeleteRequest(AnomalyDetectorsIndex.jobResultsAliasedName(jobId), DatafeedTimingStats.documentId(jobId)) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); + executeAsyncWithOrigin(client, ML_ORIGIN, DeleteAction.INSTANCE, request, new ActionListener<>() { + @Override + public void onResponse(DeleteResponse deleteResponse) { + assert deleteResponse.getResult() == DocWriteResponse.Result.DELETED + || deleteResponse.getResult() == DocWriteResponse.Result.NOT_FOUND; + actionListener.onResponse(deleteResponse); + } + @Override + public void onFailure(Exception e) { + actionListener.onFailure(e); + } + }); } @Override diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java index 7eb25b04b8667..a44ff568a67e9 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java @@ -11,11 +11,20 @@ import java.util.Objects; +/** + * {@link DatafeedTimingStatsReporter} class handles the logic of persisting {@link DatafeedTimingStats} if they changed significantly + * since the last time they were persisted. + * + * This class is not thread-safe. + */ public class DatafeedTimingStatsReporter { - private final JobResultsPersister jobResultsPersister; + /** Persisted timing stats. May be stale. */ private DatafeedTimingStats persistedTimingStats; + /** Current timing stats. */ private volatile DatafeedTimingStats currentTimingStats; + /** Object used to persist current timing stats. */ + private final JobResultsPersister jobResultsPersister; public DatafeedTimingStatsReporter(DatafeedTimingStats timingStats, JobResultsPersister jobResultsPersister) { Objects.requireNonNull(timingStats); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporter.java index d30335a5f06e9..12d1689497643 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporter.java @@ -20,9 +20,9 @@ public class TimingStatsReporter { /** Persisted timing stats. May be stale. */ private TimingStats persistedTimingStats; /** Current timing stats. */ - private TimingStats currentTimingStats; + private volatile TimingStats currentTimingStats; /** Object used to persist current timing stats. */ - private JobResultsPersister.Builder bulkResultsPersister; + private final JobResultsPersister.Builder bulkResultsPersister; public TimingStatsReporter(TimingStats timingStats, JobResultsPersister.Builder jobResultsPersister) { Objects.requireNonNull(timingStats); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java index 1ed6400bc4b7d..68572b3266363 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java @@ -30,56 +30,56 @@ public void setUpTests() { } public void testReportSearchDuration() { - DatafeedTimingStats timingStats = new DatafeedTimingStats(JOB_ID, 10000.0); + DatafeedTimingStats timingStats = new DatafeedTimingStats(JOB_ID, 3, 10000.0); DatafeedTimingStatsReporter timingStatsReporter = new DatafeedTimingStatsReporter(timingStats, jobResultsPersister); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 10000.0))); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 3, 10000.0))); timingStatsReporter.reportSearchDuration(ONE_SECOND); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 11000.0))); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 4, 11000.0))); timingStatsReporter.reportSearchDuration(ONE_SECOND); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 12000.0))); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 5, 12000.0))); timingStatsReporter.reportSearchDuration(ONE_SECOND); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 13000.0))); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 6, 13000.0))); timingStatsReporter.reportSearchDuration(ONE_SECOND); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 14000.0))); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 7, 14000.0))); InOrder inOrder = inOrder(jobResultsPersister); - inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 12000.0)); - inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 14000.0)); + inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 5, 12000.0)); + inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 7, 14000.0)); inOrder.verifyNoMoreInteractions(); } public void testTimingStatsDifferSignificantly() { assertThat( DatafeedTimingStatsReporter.differSignificantly( - new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1000.0)), + new DatafeedTimingStats(JOB_ID, 5, 1000.0), new DatafeedTimingStats(JOB_ID, 5, 1000.0)), is(false)); assertThat( DatafeedTimingStatsReporter.differSignificantly( - new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1100.0)), + new DatafeedTimingStats(JOB_ID, 5, 1000.0), new DatafeedTimingStats(JOB_ID, 5, 1100.0)), is(false)); assertThat( DatafeedTimingStatsReporter.differSignificantly( - new DatafeedTimingStats(JOB_ID, 1000.0), new DatafeedTimingStats(JOB_ID, 1120.0)), + new DatafeedTimingStats(JOB_ID, 5, 1000.0), new DatafeedTimingStats(JOB_ID, 5, 1120.0)), is(true)); assertThat( DatafeedTimingStatsReporter.differSignificantly( - new DatafeedTimingStats(JOB_ID, 10000.0), new DatafeedTimingStats(JOB_ID, 11000.0)), + new DatafeedTimingStats(JOB_ID, 5, 10000.0), new DatafeedTimingStats(JOB_ID, 5, 11000.0)), is(false)); assertThat( DatafeedTimingStatsReporter.differSignificantly( - new DatafeedTimingStats(JOB_ID, 10000.0), new DatafeedTimingStats(JOB_ID, 11200.0)), + new DatafeedTimingStats(JOB_ID, 5, 10000.0), new DatafeedTimingStats(JOB_ID, 5, 11200.0)), is(true)); assertThat( DatafeedTimingStatsReporter.differSignificantly( - new DatafeedTimingStats(JOB_ID, 100000.0), new DatafeedTimingStats(JOB_ID, 110000.0)), + new DatafeedTimingStats(JOB_ID, 5, 100000.0), new DatafeedTimingStats(JOB_ID, 5, 110000.0)), is(false)); assertThat( DatafeedTimingStatsReporter.differSignificantly( - new DatafeedTimingStats(JOB_ID, 100000.0), new DatafeedTimingStats(JOB_ID, 110001.0)), + new DatafeedTimingStats(JOB_ID, 5, 100000.0), new DatafeedTimingStats(JOB_ID, 5, 110001.0)), is(true)); } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java index f1263f4057999..ef866a2d91fde 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java @@ -244,7 +244,7 @@ public void testPersistDatafeedTimingStats() { .when(client).index(any(), any(ActionListener.class)); JobResultsPersister persister = new JobResultsPersister(client); - DatafeedTimingStats timingStats = new DatafeedTimingStats("foo", 666.0); + DatafeedTimingStats timingStats = new DatafeedTimingStats("foo", 6, 666.0); persister.persistDatafeedTimingStats(timingStats); ArgumentCaptor indexRequestCaptor = ArgumentCaptor.forClass(IndexRequest.class); @@ -257,6 +257,7 @@ public void testPersistDatafeedTimingStats() { equalTo( Map.of( "job_id", "foo", + "search_count", 6, "total_search_time_ms", 666.0))); verify(client, times(1)).threadPool(); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java index fd01720160280..aec8b72e8cb72 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProviderTests.java @@ -902,11 +902,13 @@ public void testDatafeedTimingStats_MultipleDocumentsAtOnce() throws IOException Arrays.asList( Map.of( Job.ID.getPreferredName(), "foo", + DatafeedTimingStats.SEARCH_COUNT.getPreferredName(), 6, DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), 666.0)); List> sourceBar = Arrays.asList( Map.of( Job.ID.getPreferredName(), "bar", + DatafeedTimingStats.SEARCH_COUNT.getPreferredName(), 7, DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), 777.0)); SearchResponse responseFoo = createSearchResponse(sourceFoo); SearchResponse responseBar = createSearchResponse(sourceBar); @@ -941,7 +943,7 @@ public void testDatafeedTimingStats_MultipleDocumentsAtOnce() throws IOException statsByJobId -> assertThat( statsByJobId, - equalTo(Map.of("foo", new DatafeedTimingStats("foo", 666.0), "bar", new DatafeedTimingStats("bar", 777.0)))), + equalTo(Map.of("foo", new DatafeedTimingStats("foo", 6, 666.0), "bar", new DatafeedTimingStats("bar", 7, 777.0)))), e -> { throw new AssertionError(); }); verify(client).threadPool(); @@ -958,6 +960,7 @@ public void testDatafeedTimingStats_Ok() throws IOException { Arrays.asList( Map.of( Job.ID.getPreferredName(), "foo", + DatafeedTimingStats.SEARCH_COUNT.getPreferredName(), 6, DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), 666.0)); SearchResponse response = createSearchResponse(source); Client client = getMockedClient( @@ -968,7 +971,7 @@ public void testDatafeedTimingStats_Ok() throws IOException { JobResultsProvider provider = createProvider(client); provider.datafeedTimingStats( "foo", - stats -> assertThat(stats, equalTo(new DatafeedTimingStats("foo", 666.0))), + stats -> assertThat(stats, equalTo(new DatafeedTimingStats("foo", 6, 666.0))), e -> { throw new AssertionError(); }); verify(client).prepareSearch(indexName); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml index b3ac8b92370cd..9cc81d9094131 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml @@ -130,6 +130,8 @@ setup: - match: { datafeeds.0.state: "stopped"} - is_false: datafeeds.0.node - match: { datafeeds.0.timing_stats.job_id: "get-datafeed-stats-1" } + - match: { datafeeds.0.timing_stats.search_count: 0 } + - match: { datafeeds.0.timing_stats.total_search_time_ms: 0.0} - do: ml.get_datafeed_stats: @@ -138,6 +140,8 @@ setup: - match: { datafeeds.0.state: "stopped"} - is_false: datafeeds.0.node - match: { datafeeds.0.timing_stats.job_id: "get-datafeed-stats-2" } + - match: { datafeeds.0.timing_stats.search_count: 0 } + - match: { datafeeds.0.timing_stats.total_search_time_ms: 0.0} --- "Test get stats for started datafeed": @@ -178,7 +182,7 @@ setup: - match: { datafeeds.0.datafeed_id: "datafeed-1"} - match: { datafeeds.0.state: "started"} - match: { datafeeds.0.timing_stats.job_id: "get-datafeed-stats-1"} - # Datafeed did not perform any search yet, hence 0.0 + - match: { datafeeds.0.timing_stats.search_count: 0} - match: { datafeeds.0.timing_stats.total_search_time_ms: 0.0} - do: @@ -191,8 +195,8 @@ setup: - match: { datafeeds.0.datafeed_id: "datafeed-1"} - match: { datafeeds.0.state: "stopped"} - match: { datafeeds.0.timing_stats.job_id: "get-datafeed-stats-1"} - # Datafeed did perform at least one search - - gt: { datafeeds.0.timing_stats.total_search_time_ms: 0.0} + - match: { datafeeds.0.timing_stats.search_count: 0} + - match: { datafeeds.0.timing_stats.total_search_time_ms: 0.0} --- "Test implicit get all datafeed stats given started datafeeds": From 4d5ff2a74957901dd2fefcd5003c458659f2fb65 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Fri, 28 Jun 2019 17:11:41 +0200 Subject: [PATCH 12/16] Delete DatafeedTimingStats document on Datafeed update --- .../core/ml/datafeed/DatafeedUpdate.java | 36 ++++++---- .../xpack/ml/integration/DatafeedJobsIT.java | 16 +++++ .../MlNativeAutodetectIntegTestCase.java | 7 ++ .../action/TransportDeleteDatafeedAction.java | 9 ++- .../action/TransportUpdateDatafeedAction.java | 70 ++++++++++++++++--- 5 files changed, 111 insertions(+), 27 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java index 9f38d0323f244..1a848f86138fc 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java @@ -420,57 +420,69 @@ public Builder(DatafeedUpdate config) { this.delayedDataCheckConfig = config.delayedDataCheckConfig; } - public void setId(String datafeedId) { + public Builder setId(String datafeedId) { id = ExceptionsHelper.requireNonNull(datafeedId, DatafeedConfig.ID.getPreferredName()); + return this; } - public void setJobId(String jobId) { + public Builder setJobId(String jobId) { this.jobId = jobId; + return this; } - public void setIndices(List indices) { + public Builder setIndices(List indices) { this.indices = indices; + return this; } - public void setQueryDelay(TimeValue queryDelay) { + public Builder setQueryDelay(TimeValue queryDelay) { this.queryDelay = queryDelay; + return this; } - public void setFrequency(TimeValue frequency) { + public Builder setFrequency(TimeValue frequency) { this.frequency = frequency; + return this; } - public void setQuery(QueryProvider queryProvider) { + public Builder setQuery(QueryProvider queryProvider) { this.queryProvider = queryProvider; + return this; } - private void setAggregationsSafe(AggProvider aggProvider) { + private Builder setAggregationsSafe(AggProvider aggProvider) { if (this.aggProvider != null) { throw ExceptionsHelper.badRequestException("Found two aggregation definitions: [aggs] and [aggregations]"); } setAggregations(aggProvider); + return this; } - public void setAggregations(AggProvider aggProvider) { + public Builder setAggregations(AggProvider aggProvider) { this.aggProvider = aggProvider; + return this; } - public void setScriptFields(List scriptFields) { + public Builder setScriptFields(List scriptFields) { List sorted = new ArrayList<>(scriptFields); sorted.sort(Comparator.comparing(SearchSourceBuilder.ScriptField::fieldName)); this.scriptFields = sorted; + return this; } - public void setDelayedDataCheckConfig(DelayedDataCheckConfig delayedDataCheckConfig) { + public Builder setDelayedDataCheckConfig(DelayedDataCheckConfig delayedDataCheckConfig) { this.delayedDataCheckConfig = delayedDataCheckConfig; + return this; } - public void setScrollSize(int scrollSize) { + public Builder setScrollSize(int scrollSize) { this.scrollSize = scrollSize; + return this; } - public void setChunkingConfig(ChunkingConfig chunkingConfig) { + public Builder setChunkingConfig(ChunkingConfig chunkingConfig) { this.chunkingConfig = chunkingConfig; + return this; } public DatafeedUpdate build() { diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java index 4fd11cb9b9bc5..e59c1f909a886 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java @@ -21,6 +21,7 @@ import org.elasticsearch.xpack.core.ml.datafeed.ChunkingConfig; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedUpdate; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.JobState; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.DataCounts; @@ -152,6 +153,21 @@ public void testDatafeedTimingStats() throws Exception { }, 60, TimeUnit.SECONDS); waitUntilJobIsClosed(jobId); + + String otherJobId = "other-lookback-job"; + Job.Builder otherJob = createScheduledJob(otherJobId); + + registerJob(otherJob); + PutJobAction.Response putOtherJobResponse = putJob(otherJob); + assertThat(putOtherJobResponse.getResponse().getJobVersion(), equalTo(Version.CURRENT)); + + updateDatafeed(new DatafeedUpdate.Builder(datafeedId).setJobId(otherJobId).build()); + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, otherJobId, equalTo(0L)); + + updateDatafeed(new DatafeedUpdate.Builder(datafeedId).setJobId(jobId).build()); + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, equalTo(0L)); + + waitUntilJobIsClosed(otherJobId); } private void assertDatafeedStats(String datafeedId, DatafeedState state, String jobId, Matcher searchCountMatcher) { diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java index 9e3c6de561249..fad76f8b23f80 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java @@ -43,11 +43,13 @@ import org.elasticsearch.xpack.core.ml.action.RevertModelSnapshotAction; import org.elasticsearch.xpack.core.ml.action.StartDatafeedAction; import org.elasticsearch.xpack.core.ml.action.StopDatafeedAction; +import org.elasticsearch.xpack.core.ml.action.UpdateDatafeedAction; import org.elasticsearch.xpack.core.ml.action.UpdateJobAction; import org.elasticsearch.xpack.core.action.util.PageParams; import org.elasticsearch.xpack.core.ml.calendars.Calendar; import org.elasticsearch.xpack.core.ml.calendars.ScheduledEvent; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedUpdate; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.JobState; import org.elasticsearch.xpack.core.ml.job.config.JobUpdate; @@ -175,6 +177,11 @@ protected StopDatafeedAction.Response stopDatafeed(String datafeedId) { return client().execute(StopDatafeedAction.INSTANCE, request).actionGet(); } + protected PutDatafeedAction.Response updateDatafeed(DatafeedUpdate update) { + UpdateDatafeedAction.Request request = new UpdateDatafeedAction.Request(update); + return client().execute(UpdateDatafeedAction.INSTANCE, request).actionGet(); + } + protected AcknowledgedResponse deleteDatafeed(String datafeedId) { DeleteDatafeedAction.Request request = new DeleteDatafeedAction.Request(datafeedId); return client().execute(DeleteDatafeedAction.INSTANCE, request).actionGet(); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java index 09efa85d7cddb..ba4a5b07886ed 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java @@ -151,19 +151,18 @@ private void deleteDatafeedConfig(DeleteDatafeedAction.Request request, ActionLi return; } - // Get datafeed config document + String datafeedId = request.getDatafeedId(); + datafeedConfigProvider.getDatafeedConfig( - request.getDatafeedId(), + datafeedId, ActionListener.wrap( datafeedConfigBuilder -> { - // Delete datafeed timing stats document deleteDatafeedTimingStats( datafeedConfigBuilder.build().getJobId(), ActionListener.wrap( unused1 -> { - // Delete datafeed config document datafeedConfigProvider.deleteDatafeedConfig( - request.getDatafeedId(), + datafeedId, ActionListener.wrap( unused2 -> listener.onResponse(new AcknowledgedResponse(true)), listener::onFailure)); 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 164c297251bbc..6875c63dda6f4 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 @@ -6,7 +6,12 @@ package org.elasticsearch.xpack.ml.action; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.delete.DeleteAction; +import org.elasticsearch.action.delete.DeleteRequest; +import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.action.support.master.TransportMasterNodeAction; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; @@ -26,7 +31,9 @@ import org.elasticsearch.xpack.core.ml.action.PutDatafeedAction; import org.elasticsearch.xpack.core.ml.action.UpdateDatafeedAction; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.messages.Messages; +import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.ml.MlConfigMigrationEligibilityCheck; import org.elasticsearch.xpack.ml.datafeed.persistence.DatafeedConfigProvider; @@ -35,8 +42,12 @@ import java.util.Collections; import java.util.Map; +import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN; +import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin; + public class TransportUpdateDatafeedAction extends TransportMasterNodeAction { + private final Client client; private final DatafeedConfigProvider datafeedConfigProvider; private final JobConfigProvider jobConfigProvider; private final MlConfigMigrationEligibilityCheck migrationEligibilityCheck; @@ -49,9 +60,10 @@ public TransportUpdateDatafeedAction(Settings settings, TransportService transpo super(UpdateDatafeedAction.NAME, transportService, clusterService, threadPool, actionFilters, indexNameExpressionResolver, UpdateDatafeedAction.Request::new); - datafeedConfigProvider = new DatafeedConfigProvider(client, xContentRegistry); - jobConfigProvider = new JobConfigProvider(client, xContentRegistry); - migrationEligibilityCheck = new MlConfigMigrationEligibilityCheck(settings, clusterService); + this.client = client; + this.datafeedConfigProvider = new DatafeedConfigProvider(client, xContentRegistry); + this.jobConfigProvider = new JobConfigProvider(client, xContentRegistry); + this.migrationEligibilityCheck = new MlConfigMigrationEligibilityCheck(settings, clusterService); } @Override @@ -86,13 +98,27 @@ protected void masterOperation(Task task, UpdateDatafeedAction.Request request, String datafeedId = request.getUpdate().getId(); - CheckedConsumer updateConsumer = ok -> { - datafeedConfigProvider.updateDatefeedConfig(request.getUpdate().getId(), request.getUpdate(), headers, - jobConfigProvider::validateDatafeedJob, - ActionListener.wrap( - updatedConfig -> listener.onResponse(new PutDatafeedAction.Response(updatedConfig)), - listener::onFailure - )); + CheckedConsumer updateConsumer = unused1 -> { + datafeedConfigProvider.getDatafeedConfig( + datafeedId, + ActionListener.wrap( + datafeedConfigBuilder -> { + deleteDatafeedTimingStats( + datafeedConfigBuilder.build().getJobId(), + ActionListener.wrap( + unused2 -> { + datafeedConfigProvider.updateDatefeedConfig( + datafeedId, + request.getUpdate(), + headers, + jobConfigProvider::validateDatafeedJob, + ActionListener.wrap( + updatedConfig -> listener.onResponse(new PutDatafeedAction.Response(updatedConfig)), + listener::onFailure)); + }, + listener::onFailure)); + }, + listener::onFailure)); }; @@ -104,6 +130,30 @@ protected void masterOperation(Task task, UpdateDatafeedAction.Request request, } } + /** + * Delete the datafeed config document + * + * @param jobId The job id + * @param actionListener Deleted datafeed listener + */ + private void deleteDatafeedTimingStats(String jobId, ActionListener actionListener) { + DeleteRequest request = + new DeleteRequest(AnomalyDetectorsIndex.jobResultsAliasedName(jobId), DatafeedTimingStats.documentId(jobId)) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); + executeAsyncWithOrigin(client, ML_ORIGIN, DeleteAction.INSTANCE, request, new ActionListener<>() { + @Override + public void onResponse(DeleteResponse deleteResponse) { + assert deleteResponse.getResult() == DocWriteResponse.Result.DELETED + || deleteResponse.getResult() == DocWriteResponse.Result.NOT_FOUND; + actionListener.onResponse(deleteResponse); + } + @Override + public void onFailure(Exception e) { + actionListener.onFailure(e); + } + }); + } + /* * This is a check against changing the datafeed's jobId and that job * already having a datafeed. From aa64029d2d55b17bcc7d851cb001396cad4652d1 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Mon, 8 Jul 2019 11:37:48 +0200 Subject: [PATCH 13/16] Fix tests --- .../xpack/core/ml/datafeed/DatafeedConfig.java | 2 +- .../action/GetDatafeedStatsActionResponseTests.java | 2 +- .../extractor/DataExtractorFactoryTests.java | 12 ++++++++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java index 9f45456feeee1..9285256c76819 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java @@ -682,7 +682,7 @@ public DatafeedConfig build() { if (!MlStrings.isValidId(id)) { throw ExceptionsHelper.badRequestException(Messages.getMessage(Messages.INVALID_ID, ID.getPreferredName(), id)); } - if (indices == null || indices.isEmpty() || indices.contains("")) { + if (indices == null || indices.isEmpty() || indices.contains(null) || indices.contains("")) { throw invalidOptionValue(INDICES.getPreferredName(), indices); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java index d7cfef1a135b7..eef987509bfa8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java @@ -108,7 +108,7 @@ public void testDatafeedStatsToXContent() throws IOException { assertThat(nodeAttributes, hasEntry("ml.max_open_jobs", "5")); Map timingStatsMap = (Map) dfStatsMap.get("timing_stats"); - assertThat(timingStatsMap.size(), is(equalTo(2))); + assertThat(timingStatsMap.size(), is(equalTo(3))); assertThat(timingStatsMap, hasEntry("job_id", "my-job-id")); assertThat(timingStatsMap, hasEntry("search_count", 5)); assertThat(timingStatsMap, hasEntry("total_search_time_ms", 123.456)); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactoryTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactoryTests.java index 308a42242304b..67e48c11fd1c4 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactoryTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/DataExtractorFactoryTests.java @@ -256,7 +256,8 @@ public void testCreateDataExtractorFactoryGivenRollupAndRemoteIndex() { }, e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); // Test with remote index, aggregation, and chunking datafeedConfig.setChunkingConfig(ChunkingConfig.newAuto()); @@ -264,7 +265,8 @@ public void testCreateDataExtractorFactoryGivenRollupAndRemoteIndex() { dataExtractorFactory -> assertThat(dataExtractorFactory, instanceOf(ChunkedDataExtractorFactory.class)), e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); // Test with remote index, no aggregation, and no chunking datafeedConfig = DatafeedManagerTests.createDatafeedConfig("datafeed1", "foo"); @@ -276,7 +278,8 @@ public void testCreateDataExtractorFactoryGivenRollupAndRemoteIndex() { e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); // Test with remote index, no aggregation, and chunking datafeedConfig.setChunkingConfig(ChunkingConfig.newAuto()); @@ -284,7 +287,8 @@ public void testCreateDataExtractorFactoryGivenRollupAndRemoteIndex() { dataExtractorFactory -> assertThat(dataExtractorFactory, instanceOf(ChunkedDataExtractorFactory.class)), e -> fail() ); - DataExtractorFactory.create(client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), listener); + DataExtractorFactory.create( + client, datafeedConfig.build(), jobBuilder.build(new Date()), xContentRegistry(), timingStatsReporter, listener); } public void testCreateDataExtractorFactoryGivenRollupAndValidAggregationAndAutoChunk() { From 5bfc3ea690f588561b6dbcf4d9cf754e994e8d90 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Mon, 8 Jul 2019 16:11:24 +0200 Subject: [PATCH 14/16] * Move DatafeedTimingStats deleting logic to JobDataDeleter class * Use DeleteByQueryAction to delete DatafeedTimingStats --- .../xpack/ml/integration/DatafeedJobsIT.java | 37 +++--------- .../action/TransportDeleteDatafeedAction.java | 37 ++---------- .../action/TransportUpdateDatafeedAction.java | 40 ++----------- .../ml/job/persistence/JobDataDeleter.java | 16 ++++++ .../job/persistence/JobDataDeleterTests.java | 57 +++++++++++++++++++ .../test/ml/get_datafeed_stats.yml | 4 +- 6 files changed, 91 insertions(+), 100 deletions(-) create mode 100644 x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleterTests.java diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java index e59c1f909a886..6f330ab32245f 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java @@ -31,6 +31,7 @@ import java.time.Duration; import java.time.Instant; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; @@ -103,63 +104,41 @@ public void testDatafeedTimingStats() throws Exception { .get(); long numDocs = randomIntBetween(32, 2048); Instant now = Instant.now(); - Instant oneWeekAgo = now.minus(Duration.ofDays(7)); - Instant twoWeeksAgo = oneWeekAgo.minus(Duration.ofDays(7)); - indexDocs(logger, "data-1", numDocs, twoWeeksAgo.toEpochMilli(), oneWeekAgo.toEpochMilli()); - - client().admin().indices().prepareCreate("data-2") - .addMapping("type", "time", "type=date") - .get(); - client().admin().cluster().prepareHealth("data-1", "data-2").setWaitForYellowStatus().get(); - long numDocs2 = randomIntBetween(32, 2048); - indexDocs(logger, "data-2", numDocs2, oneWeekAgo.toEpochMilli(), now.toEpochMilli()); + indexDocs(logger, "data-1", numDocs, now.minus(Duration.ofDays(14)).toEpochMilli(), now.toEpochMilli()); String jobId = "lookback-job"; Job.Builder job = createScheduledJob(jobId); - registerJob(job); - PutJobAction.Response putJobResponse = putJob(job); - assertThat(putJobResponse.getResponse().getJobVersion(), equalTo(Version.CURRENT)); - + putJob(job); openJob(jobId); assertBusy(() -> assertEquals(getJobStats(jobId).get(0).getState(), JobState.OPENED)); String datafeedId = jobId + "-datafeed"; - DatafeedConfig datafeedConfig = createDatafeed(datafeedId, jobId, List.of("data-1", "data-2")); - + DatafeedConfig datafeedConfig = createDatafeed(datafeedId, jobId, Arrays.asList("data-1")); registerDatafeed(datafeedConfig); putDatafeed(datafeedConfig); assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, equalTo(0L)); startDatafeed(datafeedId, 0L, now.toEpochMilli()); assertBusy(() -> { - DataCounts dataCounts = getDataCounts(jobId); - assertThat(dataCounts.getProcessedRecordCount(), equalTo(numDocs + numDocs2)); + assertThat(getDataCounts(jobId).getProcessedRecordCount(), equalTo(numDocs)); assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, greaterThan(0L)); }, 60, TimeUnit.SECONDS); + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, greaterThan(0L)); + deleteDatafeed(datafeedId); registerDatafeed(datafeedConfig); putDatafeed(datafeedConfig); assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, equalTo(0L)); - openJob(jobId); - startDatafeed(datafeedId, 0L, now.toEpochMilli()); - assertBusy(() -> { - DataCounts dataCounts = getDataCounts(jobId); - assertThat(dataCounts.getProcessedRecordCount(), equalTo(numDocs + numDocs2)); - assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, greaterThan(0L)); - }, 60, TimeUnit.SECONDS); - waitUntilJobIsClosed(jobId); String otherJobId = "other-lookback-job"; Job.Builder otherJob = createScheduledJob(otherJobId); - registerJob(otherJob); - PutJobAction.Response putOtherJobResponse = putJob(otherJob); - assertThat(putOtherJobResponse.getResponse().getJobVersion(), equalTo(Version.CURRENT)); + putJob(otherJob); updateDatafeed(new DatafeedUpdate.Builder(datafeedId).setJobId(otherJobId).build()); assertDatafeedStats(datafeedId, DatafeedState.STOPPED, otherJobId, equalTo(0L)); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java index ba4a5b07886ed..56c54bbe9547c 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java @@ -7,12 +7,7 @@ import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.DocWriteResponse; -import org.elasticsearch.action.delete.DeleteAction; -import org.elasticsearch.action.delete.DeleteRequest; -import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.TransportMasterNodeAction; import org.elasticsearch.client.Client; @@ -34,12 +29,11 @@ import org.elasticsearch.xpack.core.ml.action.DeleteDatafeedAction; import org.elasticsearch.xpack.core.ml.action.IsolateDatafeedAction; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; -import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.messages.Messages; -import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.ml.MlConfigMigrationEligibilityCheck; import org.elasticsearch.xpack.ml.datafeed.persistence.DatafeedConfigProvider; +import org.elasticsearch.xpack.ml.job.persistence.JobDataDeleter; import java.io.IOException; @@ -157,8 +151,9 @@ private void deleteDatafeedConfig(DeleteDatafeedAction.Request request, ActionLi datafeedId, ActionListener.wrap( datafeedConfigBuilder -> { - deleteDatafeedTimingStats( - datafeedConfigBuilder.build().getJobId(), + String jobId = datafeedConfigBuilder.build().getJobId(); + JobDataDeleter jobDataDeleter = new JobDataDeleter(client, jobId); + jobDataDeleter.deleteDatafeedTimingStats( ActionListener.wrap( unused1 -> { datafeedConfigProvider.deleteDatafeedConfig( @@ -172,30 +167,6 @@ private void deleteDatafeedConfig(DeleteDatafeedAction.Request request, ActionLi listener::onFailure)); } - /** - * Delete the datafeed config document - * - * @param jobId The job id - * @param actionListener Deleted datafeed listener - */ - private void deleteDatafeedTimingStats(String jobId, ActionListener actionListener) { - DeleteRequest request = - new DeleteRequest(AnomalyDetectorsIndex.jobResultsAliasedName(jobId), DatafeedTimingStats.documentId(jobId)) - .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); - executeAsyncWithOrigin(client, ML_ORIGIN, DeleteAction.INSTANCE, request, new ActionListener<>() { - @Override - public void onResponse(DeleteResponse deleteResponse) { - assert deleteResponse.getResult() == DocWriteResponse.Result.DELETED - || deleteResponse.getResult() == DocWriteResponse.Result.NOT_FOUND; - actionListener.onResponse(deleteResponse); - } - @Override - public void onFailure(Exception e) { - actionListener.onFailure(e); - } - }); - } - @Override protected ClusterBlockException checkBlock(DeleteDatafeedAction.Request request, ClusterState state) { return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE); 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 6875c63dda6f4..d203be2cb7679 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 @@ -6,12 +6,7 @@ package org.elasticsearch.xpack.ml.action; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.DocWriteResponse; -import org.elasticsearch.action.delete.DeleteAction; -import org.elasticsearch.action.delete.DeleteRequest; -import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.action.support.master.TransportMasterNodeAction; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; @@ -31,20 +26,16 @@ import org.elasticsearch.xpack.core.ml.action.PutDatafeedAction; import org.elasticsearch.xpack.core.ml.action.UpdateDatafeedAction; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; -import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.messages.Messages; -import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.ml.MlConfigMigrationEligibilityCheck; import org.elasticsearch.xpack.ml.datafeed.persistence.DatafeedConfigProvider; import org.elasticsearch.xpack.ml.job.persistence.JobConfigProvider; +import org.elasticsearch.xpack.ml.job.persistence.JobDataDeleter; import java.util.Collections; import java.util.Map; -import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN; -import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin; - public class TransportUpdateDatafeedAction extends TransportMasterNodeAction { private final Client client; @@ -103,8 +94,9 @@ protected void masterOperation(Task task, UpdateDatafeedAction.Request request, datafeedId, ActionListener.wrap( datafeedConfigBuilder -> { - deleteDatafeedTimingStats( - datafeedConfigBuilder.build().getJobId(), + String jobId = datafeedConfigBuilder.build().getJobId(); + JobDataDeleter jobDataDeleter = new JobDataDeleter(client, jobId); + jobDataDeleter.deleteDatafeedTimingStats( ActionListener.wrap( unused2 -> { datafeedConfigProvider.updateDatefeedConfig( @@ -130,30 +122,6 @@ protected void masterOperation(Task task, UpdateDatafeedAction.Request request, } } - /** - * Delete the datafeed config document - * - * @param jobId The job id - * @param actionListener Deleted datafeed listener - */ - private void deleteDatafeedTimingStats(String jobId, ActionListener actionListener) { - DeleteRequest request = - new DeleteRequest(AnomalyDetectorsIndex.jobResultsAliasedName(jobId), DatafeedTimingStats.documentId(jobId)) - .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); - executeAsyncWithOrigin(client, ML_ORIGIN, DeleteAction.INSTANCE, request, new ActionListener<>() { - @Override - public void onResponse(DeleteResponse deleteResponse) { - assert deleteResponse.getResult() == DocWriteResponse.Result.DELETED - || deleteResponse.getResult() == DocWriteResponse.Result.NOT_FOUND; - actionListener.onResponse(deleteResponse); - } - @Override - public void onFailure(Exception e) { - actionListener.onFailure(e); - } - }); - } - /* * This is a check against changing the datafeed's jobId and that job * already having a datafeed. diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java index f48dcb977e59c..6e8a046d8edcc 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java @@ -21,6 +21,7 @@ import org.elasticsearch.index.reindex.BulkByScrollTask; import org.elasticsearch.index.reindex.DeleteByQueryAction; import org.elasticsearch.index.reindex.DeleteByQueryRequest; +import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.ModelSnapshot; import org.elasticsearch.xpack.core.ml.job.results.Result; @@ -122,6 +123,21 @@ public void deleteInterimResults() { } } + + /** + * Delete the datafeed timing stats document from all the job results indices + * + * @param listener Response listener + */ + public void deleteDatafeedTimingStats(ActionListener listener) { + DeleteByQueryRequest deleteByQueryRequest = new DeleteByQueryRequest(AnomalyDetectorsIndex.jobResultsAliasedName(jobId)) + .setRefresh(true) + .setIndicesOptions(IndicesOptions.lenientExpandOpen()) + .setQuery(new IdsQueryBuilder().addIds(DatafeedTimingStats.documentId(jobId))); + + executeAsyncWithOrigin(client, ML_ORIGIN, DeleteByQueryAction.INSTANCE, deleteByQueryRequest, listener); + } + // Wrapper to ensure safety private static class DeleteByQueryHolder { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleterTests.java new file mode 100644 index 0000000000000..f02bc5bf9f1e7 --- /dev/null +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleterTests.java @@ -0,0 +1,57 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.ml.job.persistence; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.client.Client; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.index.reindex.DeleteByQueryAction; +import org.elasticsearch.index.reindex.DeleteByQueryRequest; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; +import org.junit.Before; +import org.mockito.ArgumentCaptor; + +import static org.hamcrest.Matchers.arrayContaining; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +public class JobDataDeleterTests extends ESTestCase { + + private static final String JOB_ID = "my-job-id"; + + private Client client; + + @Before + public void setUpTests() { + client = mock(Client.class); + ThreadPool threadPool = mock(ThreadPool.class); + when(client.threadPool()).thenReturn(threadPool); + when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); + } + + public void testDeleteDatafeedTimingStats() { + JobDataDeleter jobDataDeleter = new JobDataDeleter(client, JOB_ID); + jobDataDeleter.deleteDatafeedTimingStats(ActionListener.wrap( + deleteResponse -> {}, + e -> fail(e.toString()) + )); + + ArgumentCaptor deleteRequestCaptor = ArgumentCaptor.forClass(DeleteByQueryRequest.class); + verify(client).threadPool(); + verify(client).execute(eq(DeleteByQueryAction.INSTANCE), deleteRequestCaptor.capture(), any(ActionListener.class)); + verifyNoMoreInteractions(client); + + DeleteByQueryRequest deleteRequest = deleteRequestCaptor.getValue(); + assertThat(deleteRequest.indices(), arrayContaining(AnomalyDetectorsIndex.jobResultsAliasedName(JOB_ID))); + } +} diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml index 9cc81d9094131..0496de1db477b 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/get_datafeed_stats.yml @@ -195,8 +195,8 @@ setup: - match: { datafeeds.0.datafeed_id: "datafeed-1"} - match: { datafeeds.0.state: "stopped"} - match: { datafeeds.0.timing_stats.job_id: "get-datafeed-stats-1"} - - match: { datafeeds.0.timing_stats.search_count: 0} - - match: { datafeeds.0.timing_stats.total_search_time_ms: 0.0} + - match: { datafeeds.0.timing_stats.search_count: 1} + - gte: { datafeeds.0.timing_stats.total_search_time_ms: 0.0} --- "Test implicit get all datafeed stats given started datafeeds": From 691bb694ad1f5b3a3f6f28ac5f4adb5423a1845c Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Tue, 9 Jul 2019 10:45:09 +0200 Subject: [PATCH 15/16] * Add search_count field to HLRC * Refine the new tests in DatafeedJobsIT test suite --- .../ml/datafeed/DatafeedTimingStats.java | 31 ++++++- .../ml/datafeed/DatafeedTimingStatsTests.java | 48 +++++++++- .../ml/apis/datafeedresource.asciidoc | 1 + .../ml/apis/get-datafeed-stats.asciidoc | 1 + .../ml/action/GetDatafeedsStatsAction.java | 6 +- .../persistence/ElasticsearchMappings.java | 3 + .../ml/job/results/ReservedFieldNames.java | 1 + .../GetDatafeedStatsActionResponseTests.java | 1 + .../xpack/ml/integration/DatafeedJobsIT.java | 90 +++++++++++-------- .../ml/job/persistence/JobDataDeleter.java | 3 +- 10 files changed, 140 insertions(+), 45 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStats.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStats.java index 8428ffe01c9c1..17617f556cf7b 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStats.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStats.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.client.ml.datafeed; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.ConstructingObjectParser; @@ -34,6 +35,7 @@ public class DatafeedTimingStats implements ToXContentObject { public static final ParseField JOB_ID = new ParseField("job_id"); + public static final ParseField SEARCH_COUNT = new ParseField("search_count"); public static final ParseField TOTAL_SEARCH_TIME_MS = new ParseField("total_search_time_ms"); public static final ParseField TYPE = new ParseField("datafeed_timing_stats"); @@ -42,17 +44,28 @@ public class DatafeedTimingStats implements ToXContentObject { private static ConstructingObjectParser createParser() { ConstructingObjectParser parser = - new ConstructingObjectParser<>("datafeed_timing_stats", true, a -> new DatafeedTimingStats((String) a[0], (double) a[1])); + new ConstructingObjectParser<>( + "datafeed_timing_stats", + true, + args -> { + String jobId = (String) args[0]; + Long searchCount = (Long) args[1]; + Double totalSearchTimeMs = (Double) args[2]; + return new DatafeedTimingStats(jobId, getOrDefault(searchCount, 0L), getOrDefault(totalSearchTimeMs, 0.0)); + }); parser.declareString(constructorArg(), JOB_ID); + parser.declareLong(optionalConstructorArg(), SEARCH_COUNT); parser.declareDouble(optionalConstructorArg(), TOTAL_SEARCH_TIME_MS); return parser; } private final String jobId; + private long searchCount; private double totalSearchTimeMs; - public DatafeedTimingStats(String jobId, double totalSearchTimeMs) { + public DatafeedTimingStats(String jobId, long searchCount, double totalSearchTimeMs) { this.jobId = Objects.requireNonNull(jobId); + this.searchCount = searchCount; this.totalSearchTimeMs = totalSearchTimeMs; } @@ -60,6 +73,10 @@ public String getJobId() { return jobId; } + public long getSearchCount() { + return searchCount; + } + public double getTotalSearchTimeMs() { return totalSearchTimeMs; } @@ -68,6 +85,7 @@ public double getTotalSearchTimeMs() { public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException { builder.startObject(); builder.field(JOB_ID.getPreferredName(), jobId); + builder.field(SEARCH_COUNT.getPreferredName(), searchCount); builder.field(TOTAL_SEARCH_TIME_MS.getPreferredName(), totalSearchTimeMs); builder.endObject(); return builder; @@ -84,16 +102,21 @@ public boolean equals(Object obj) { DatafeedTimingStats other = (DatafeedTimingStats) obj; return Objects.equals(this.jobId, other.jobId) - && Objects.equals(this.totalSearchTimeMs, other.totalSearchTimeMs); + && this.searchCount == other.searchCount + && this.totalSearchTimeMs == other.totalSearchTimeMs; } @Override public int hashCode() { - return Objects.hash(jobId, totalSearchTimeMs); + return Objects.hash(jobId, searchCount, totalSearchTimeMs); } @Override public String toString() { return Strings.toString(this); } + + private static T getOrDefault(@Nullable T value, T defaultValue) { + return value != null ? value : defaultValue; + } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStatsTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStatsTests.java index 6bdbceb942265..0a5134606da45 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStatsTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStatsTests.java @@ -18,15 +18,22 @@ */ package org.elasticsearch.client.ml.datafeed; +import org.elasticsearch.common.xcontent.DeprecationHandler; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.AbstractXContentTestCase; import java.io.IOException; +import static org.hamcrest.Matchers.equalTo; + public class DatafeedTimingStatsTests extends AbstractXContentTestCase { + private static final String JOB_ID = "my-job-id"; + public static DatafeedTimingStats createRandomInstance() { - return new DatafeedTimingStats(randomAlphaOfLength(10), randomDouble()); + return new DatafeedTimingStats(randomAlphaOfLength(10), randomLong(), randomDouble()); } @Override @@ -43,4 +50,43 @@ protected DatafeedTimingStats doParseInstance(XContentParser parser) throws IOEx protected boolean supportsUnknownFields() { return true; } + + public void testParse_OptionalFieldsAbsent() throws IOException { + String json = "{\"job_id\": \"my-job-id\"}"; + try (XContentParser parser = + XContentFactory.xContent(XContentType.JSON).createParser( + xContentRegistry(), DeprecationHandler.THROW_UNSUPPORTED_OPERATION, json)) { + DatafeedTimingStats stats = DatafeedTimingStats.PARSER.apply(parser, null); + assertThat(stats.getJobId(), equalTo(JOB_ID)); + assertThat(stats.getSearchCount(), equalTo(0L)); + assertThat(stats.getTotalSearchTimeMs(), equalTo(0.0)); + } + } + + public void testEquals() { + DatafeedTimingStats stats1 = new DatafeedTimingStats(JOB_ID, 5, 100.0); + DatafeedTimingStats stats2 = new DatafeedTimingStats(JOB_ID, 5, 100.0); + DatafeedTimingStats stats3 = new DatafeedTimingStats(JOB_ID, 5, 200.0); + + assertTrue(stats1.equals(stats1)); + assertTrue(stats1.equals(stats2)); + assertFalse(stats2.equals(stats3)); + } + + public void testHashCode() { + DatafeedTimingStats stats1 = new DatafeedTimingStats(JOB_ID, 5, 100.0); + DatafeedTimingStats stats2 = new DatafeedTimingStats(JOB_ID, 5, 100.0); + DatafeedTimingStats stats3 = new DatafeedTimingStats(JOB_ID, 5, 200.0); + + assertEquals(stats1.hashCode(), stats1.hashCode()); + assertEquals(stats1.hashCode(), stats2.hashCode()); + assertNotEquals(stats2.hashCode(), stats3.hashCode()); + } + + public void testConstructorAndGetters() { + DatafeedTimingStats stats = new DatafeedTimingStats(JOB_ID, 5, 123.456); + assertThat(stats.getJobId(), equalTo(JOB_ID)); + assertThat(stats.getSearchCount(), equalTo(5L)); + assertThat(stats.getTotalSearchTimeMs(), equalTo(123.456)); + } } diff --git a/docs/reference/ml/apis/datafeedresource.asciidoc b/docs/reference/ml/apis/datafeedresource.asciidoc index 87b19b7433455..32e1237415d9f 100644 --- a/docs/reference/ml/apis/datafeedresource.asciidoc +++ b/docs/reference/ml/apis/datafeedresource.asciidoc @@ -147,5 +147,6 @@ update their values: `timing_stats`:: (object) An object that provides statistical information about timing aspect of this datafeed. + `job_id`::: A numerical character string that uniquely identifies the job. + `search_count`::: Number of searches performed by this datafeed. `total_search_time_ms`::: Total time the datafeed spent searching in milliseconds. diff --git a/docs/reference/ml/apis/get-datafeed-stats.asciidoc b/docs/reference/ml/apis/get-datafeed-stats.asciidoc index db041a235779a..8a71351e76d67 100644 --- a/docs/reference/ml/apis/get-datafeed-stats.asciidoc +++ b/docs/reference/ml/apis/get-datafeed-stats.asciidoc @@ -93,6 +93,7 @@ The API returns the following results: "assignment_explanation": "", "timing_stats": { "job_id": "job-total-requests", + "search_count": 20, "total_search_time_ms": 120.5 } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java index f619c76f52f89..e07ad3ba31dc5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java @@ -29,7 +29,7 @@ import java.util.Map; import java.util.Objects; -import static org.elasticsearch.Version.V_7_3_0; +import static org.elasticsearch.Version.V_7_4_0; public class GetDatafeedsStatsAction extends StreamableResponseActionType { @@ -148,7 +148,7 @@ public DatafeedStats(String datafeedId, DatafeedState datafeedState, @Nullable D datafeedState = DatafeedState.fromStream(in); node = in.readOptionalWriteable(DiscoveryNode::new); assignmentExplanation = in.readOptionalString(); - if (in.getVersion().onOrAfter(V_7_3_0)) { + if (in.getVersion().onOrAfter(V_7_4_0)) { timingStats = in.readOptionalWriteable(DatafeedTimingStats::new); } else { timingStats = null; @@ -212,7 +212,7 @@ public void writeTo(StreamOutput out) throws IOException { datafeedState.writeTo(out); out.writeOptionalWriteable(node); out.writeOptionalString(assignmentExplanation); - if (out.getVersion().onOrAfter(V_7_3_0)) { + if (out.getVersion().onOrAfter(V_7_4_0)) { out.writeOptionalWriteable(timingStats); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java index 92dc4677cc13b..77073a23491e6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java @@ -937,6 +937,9 @@ private static void addTimingStatsExceptBucketCountMapping(XContentBuilder build */ private static void addDatafeedTimingStats(XContentBuilder builder) throws IOException { builder + .startObject(DatafeedTimingStats.SEARCH_COUNT.getPreferredName()) + .field(TYPE, LONG) + .endObject() .startObject(DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName()) .field(TYPE, DOUBLE) .endObject(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java index 30ba8def9747c..ddd13e7fc31ba 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java @@ -186,6 +186,7 @@ public final class ReservedFieldNames { TimingStats.AVG_BUCKET_PROCESSING_TIME_MS.getPreferredName(), TimingStats.EXPONENTIAL_AVG_BUCKET_PROCESSING_TIME_MS.getPreferredName(), + DatafeedTimingStats.SEARCH_COUNT.getPreferredName(), DatafeedTimingStats.TOTAL_SEARCH_TIME_MS.getPreferredName(), GetResult._ID, diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java index eef987509bfa8..0fbd8a3386eb9 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedStatsActionResponseTests.java @@ -94,6 +94,7 @@ public void testDatafeedStatsToXContent() throws IOException { assertThat(dfStatsMap, hasEntry("datafeed_id", "df-id")); assertThat(dfStatsMap, hasEntry("state", "started")); assertThat(dfStatsMap, hasKey("node")); + assertThat(dfStatsMap, hasKey("timing_stats")); Map nodeMap = (Map) dfStatsMap.get("node"); assertThat(nodeMap, hasEntry("id", "df-node-id")); diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java index 6f330ab32245f..a3b57fb81ee64 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java @@ -98,55 +98,75 @@ public void testLookbackOnly() throws Exception { waitUntilJobIsClosed(job.getId()); } - public void testDatafeedTimingStats() throws Exception { - client().admin().indices().prepareCreate("data-1") + public void testDatafeedTimingStats_DatafeedRecreated() throws Exception { + client().admin().indices().prepareCreate("data") .addMapping("type", "time", "type=date") .get(); long numDocs = randomIntBetween(32, 2048); Instant now = Instant.now(); - indexDocs(logger, "data-1", numDocs, now.minus(Duration.ofDays(14)).toEpochMilli(), now.toEpochMilli()); + indexDocs(logger, "data", numDocs, now.minus(Duration.ofDays(14)).toEpochMilli(), now.toEpochMilli()); + + Job.Builder job = createScheduledJob("lookback-job"); + + String datafeedId = "lookback-datafeed"; + DatafeedConfig datafeedConfig = createDatafeed(datafeedId, job.getId(), Arrays.asList("data")); - String jobId = "lookback-job"; - Job.Builder job = createScheduledJob(jobId); registerJob(job); putJob(job); - openJob(jobId); - assertBusy(() -> assertEquals(getJobStats(jobId).get(0).getState(), JobState.OPENED)); - String datafeedId = jobId + "-datafeed"; - DatafeedConfig datafeedConfig = createDatafeed(datafeedId, jobId, Arrays.asList("data-1")); - registerDatafeed(datafeedConfig); - putDatafeed(datafeedConfig); - assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, equalTo(0L)); - - startDatafeed(datafeedId, 0L, now.toEpochMilli()); - assertBusy(() -> { - assertThat(getDataCounts(jobId).getProcessedRecordCount(), equalTo(numDocs)); - assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, greaterThan(0L)); - }, 60, TimeUnit.SECONDS); + for (int i = 0; i < 2; ++i) { + openJob(job.getId()); + assertBusy(() -> assertEquals(getJobStats(job.getId()).get(0).getState(), JobState.OPENED)); + registerDatafeed(datafeedConfig); + putDatafeed(datafeedConfig); + // Datafeed did not do anything yet, hence search_count is equal to 0. + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, job.getId(), equalTo(0L)); + startDatafeed(datafeedId, 0L, now.toEpochMilli()); + assertBusy(() -> { + assertThat(getDataCounts(job.getId()).getProcessedRecordCount(), equalTo(numDocs)); + // Datafeed processed numDocs documents so search_count must be greater than 0. + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, job.getId(), greaterThan(0L)); + }, 60, TimeUnit.SECONDS); + deleteDatafeed(datafeedId); + waitUntilJobIsClosed(job.getId()); + } + } - assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, greaterThan(0L)); + public void testDatafeedTimingStats_DatafeedJobIdUpdated() throws Exception { + client().admin().indices().prepareCreate("data") + .addMapping("type", "time", "type=date") + .get(); + long numDocs = randomIntBetween(32, 2048); + Instant now = Instant.now(); + indexDocs(logger, "data", numDocs, now.minus(Duration.ofDays(14)).toEpochMilli(), now.toEpochMilli()); - deleteDatafeed(datafeedId); + Job.Builder jobA = createScheduledJob("lookback-job"); + Job.Builder jobB = createScheduledJob("other-lookback-job"); + for (Job.Builder job : Arrays.asList(jobA, jobB)) { + registerJob(job); + putJob(job); + } + String datafeedId = "lookback-datafeed"; + DatafeedConfig datafeedConfig = createDatafeed(datafeedId, jobA.getId(), Arrays.asList("data")); registerDatafeed(datafeedConfig); putDatafeed(datafeedConfig); - assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, equalTo(0L)); - - waitUntilJobIsClosed(jobId); - - String otherJobId = "other-lookback-job"; - Job.Builder otherJob = createScheduledJob(otherJobId); - registerJob(otherJob); - putJob(otherJob); - updateDatafeed(new DatafeedUpdate.Builder(datafeedId).setJobId(otherJobId).build()); - assertDatafeedStats(datafeedId, DatafeedState.STOPPED, otherJobId, equalTo(0L)); - - updateDatafeed(new DatafeedUpdate.Builder(datafeedId).setJobId(jobId).build()); - assertDatafeedStats(datafeedId, DatafeedState.STOPPED, jobId, equalTo(0L)); - - waitUntilJobIsClosed(otherJobId); + for (Job.Builder job : Arrays.asList(jobA, jobB, jobA)) { + openJob(job.getId()); + assertBusy(() -> assertEquals(getJobStats(job.getId()).get(0).getState(), JobState.OPENED)); + // Bind datafeedId to the current job on the list, timing stats are wiped out. + updateDatafeed(new DatafeedUpdate.Builder(datafeedId).setJobId(job.getId()).build()); + // Datafeed did not do anything yet, hence search_count is equal to 0. + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, job.getId(), equalTo(0L)); + startDatafeed(datafeedId, 0L, now.toEpochMilli()); + assertBusy(() -> { + assertThat(getDataCounts(job.getId()).getProcessedRecordCount(), equalTo(numDocs)); + // Datafeed processed numDocs documents so search_count must be greater than 0. + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, job.getId(), greaterThan(0L)); + }, 60, TimeUnit.SECONDS); + waitUntilJobIsClosed(job.getId()); + } } private void assertDatafeedStats(String datafeedId, DatafeedState state, String jobId, Matcher searchCountMatcher) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java index 6e8a046d8edcc..07c75f75f7e93 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java @@ -122,8 +122,7 @@ public void deleteInterimResults() { LOGGER.error("[" + jobId + "] An error occurred while deleting interim results", e); } } - - + /** * Delete the datafeed timing stats document from all the job results indices * From aa04e157e4bb0c59482eee4f50c4dd0d2a3b0d55 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Tue, 9 Jul 2019 14:08:16 +0200 Subject: [PATCH 16/16] Apply review comments --- .../xpack/ml/integration/DatafeedJobsIT.java | 35 +++++++++++ .../action/TransportUpdateDatafeedAction.java | 59 +++++++++++-------- .../datafeed/DatafeedTimingStatsReporter.java | 8 ++- .../job/persistence/JobResultsPersister.java | 4 +- .../job/persistence/TimingStatsReporter.java | 2 +- .../DatafeedTimingStatsReporterTests.java | 14 +++-- .../persistence/JobResultsPersisterTests.java | 4 +- .../persistence/TimingStatsReporterTests.java | 4 +- 8 files changed, 91 insertions(+), 39 deletions(-) diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java index a3b57fb81ee64..9b6523eb73cc2 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsIT.java @@ -169,6 +169,41 @@ public void testDatafeedTimingStats_DatafeedJobIdUpdated() throws Exception { } } + public void testDatafeedTimingStats_QueryDelayUpdated_TimingStatsNotReset() throws Exception { + client().admin().indices().prepareCreate("data") + .addMapping("type", "time", "type=date") + .get(); + long numDocs = randomIntBetween(32, 2048); + Instant now = Instant.now(); + indexDocs(logger, "data", numDocs, now.minus(Duration.ofDays(14)).toEpochMilli(), now.toEpochMilli()); + + Job.Builder job = createScheduledJob("lookback-job"); + registerJob(job); + putJob(job); + + String datafeedId = "lookback-datafeed"; + DatafeedConfig datafeedConfig = createDatafeed(datafeedId, job.getId(), Arrays.asList("data")); + registerDatafeed(datafeedConfig); + putDatafeed(datafeedConfig); + + openJob(job.getId()); + assertBusy(() -> assertEquals(getJobStats(job.getId()).get(0).getState(), JobState.OPENED)); + // Datafeed did not do anything yet, hence search_count is equal to 0. + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, job.getId(), equalTo(0L)); + startDatafeed(datafeedId, 0L, now.toEpochMilli()); + assertBusy(() -> { + assertThat(getDataCounts(job.getId()).getProcessedRecordCount(), equalTo(numDocs)); + // Datafeed processed numDocs documents so search_count must be greater than 0. + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, job.getId(), greaterThan(0L)); + }, 60, TimeUnit.SECONDS); + waitUntilJobIsClosed(job.getId()); + + // Change something different than jobId, here: queryDelay. + updateDatafeed(new DatafeedUpdate.Builder(datafeedId).setQueryDelay(TimeValue.timeValueSeconds(777)).build()); + // Search_count is still greater than 0 (i.e. has not been reset by datafeed update) + assertDatafeedStats(datafeedId, DatafeedState.STOPPED, job.getId(), greaterThan(0L)); + } + private void assertDatafeedStats(String datafeedId, DatafeedState state, String jobId, Matcher searchCountMatcher) { GetDatafeedsStatsAction.Request request = new GetDatafeedsStatsAction.Request(datafeedId); GetDatafeedsStatsAction.Response response = client().execute(GetDatafeedsStatsAction.INSTANCE, request).actionGet(); 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 d203be2cb7679..a8fdc43642014 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 @@ -18,6 +18,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.index.reindex.BulkByScrollResponse; import org.elasticsearch.persistent.PersistentTasksCustomMetaData; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; @@ -89,36 +90,42 @@ protected void masterOperation(Task task, UpdateDatafeedAction.Request request, String datafeedId = request.getUpdate().getId(); - CheckedConsumer updateConsumer = unused1 -> { - datafeedConfigProvider.getDatafeedConfig( - datafeedId, - ActionListener.wrap( - datafeedConfigBuilder -> { - String jobId = datafeedConfigBuilder.build().getJobId(); - JobDataDeleter jobDataDeleter = new JobDataDeleter(client, jobId); - jobDataDeleter.deleteDatafeedTimingStats( - ActionListener.wrap( - unused2 -> { - datafeedConfigProvider.updateDatefeedConfig( - datafeedId, - request.getUpdate(), - headers, - jobConfigProvider::validateDatafeedJob, - ActionListener.wrap( - updatedConfig -> listener.onResponse(new PutDatafeedAction.Response(updatedConfig)), - listener::onFailure)); - }, - listener::onFailure)); - }, - listener::onFailure)); - }; + CheckedConsumer updateConsumer = + unused -> { + datafeedConfigProvider.updateDatefeedConfig( + datafeedId, + request.getUpdate(), + headers, + jobConfigProvider::validateDatafeedJob, + ActionListener.wrap( + updatedConfig -> listener.onResponse(new PutDatafeedAction.Response(updatedConfig)), + listener::onFailure)); + }; + + CheckedConsumer deleteTimingStatsAndUpdateConsumer = + unused -> { + datafeedConfigProvider.getDatafeedConfig( + datafeedId, + ActionListener.wrap( + datafeedConfigBuilder -> { + String jobId = datafeedConfigBuilder.build().getJobId(); + if (jobId.equals(request.getUpdate().getJobId())) { + // Datafeed's jobId didn't change, no point in deleting datafeed timing stats. + updateConsumer.accept(null); + } else { + JobDataDeleter jobDataDeleter = new JobDataDeleter(client, jobId); + jobDataDeleter.deleteDatafeedTimingStats(ActionListener.wrap(updateConsumer, listener::onFailure)); + } + }, + listener::onFailure)); + }; if (request.getUpdate().getJobId() != null) { - checkJobDoesNotHaveADifferentDatafeed(request.getUpdate().getJobId(), datafeedId, - ActionListener.wrap(updateConsumer, listener::onFailure)); + checkJobDoesNotHaveADifferentDatafeed( + request.getUpdate().getJobId(), datafeedId, ActionListener.wrap(deleteTimingStatsAndUpdateConsumer, listener::onFailure)); } else { - updateConsumer.accept(Boolean.TRUE); + updateConsumer.accept(null); } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java index a44ff568a67e9..260db421e0446 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.ml.datafeed; +import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; @@ -46,13 +47,14 @@ public void reportSearchDuration(TimeValue searchDuration) { } currentTimingStats.incrementTotalSearchTimeMs(searchDuration.millis()); if (differSignificantly(currentTimingStats, persistedTimingStats)) { - flush(); + // TODO: Consider changing refresh policy to NONE here and only do IMMEDIATE on datafeed _stop action + flush(WriteRequest.RefreshPolicy.IMMEDIATE); } } - private void flush() { + private void flush(WriteRequest.RefreshPolicy refreshPolicy) { persistedTimingStats = new DatafeedTimingStats(currentTimingStats); - jobResultsPersister.persistDatafeedTimingStats(persistedTimingStats); + jobResultsPersister.persistDatafeedTimingStats(persistedTimingStats, refreshPolicy); } /** diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java index f54964c120bbb..1d960c5741836 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java @@ -330,11 +330,13 @@ public void commitStateWrites(String jobId) { * Persist datafeed timing stats * * @param timingStats datafeed timing stats to persist + * @param refreshPolicy refresh policy to apply */ - public IndexResponse persistDatafeedTimingStats(DatafeedTimingStats timingStats) { + public IndexResponse persistDatafeedTimingStats(DatafeedTimingStats timingStats, WriteRequest.RefreshPolicy refreshPolicy) { String jobId = timingStats.getJobId(); logger.trace("[{}] Persisting datafeed timing stats", jobId); Persistable persistable = new Persistable(jobId, timingStats, DatafeedTimingStats.documentId(timingStats.getJobId())); + persistable.setRefreshPolicy(refreshPolicy); return persistable.persist(AnomalyDetectorsIndex.resultsWriteAlias(jobId)).actionGet(); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporter.java index 12d1689497643..0da4046edb49e 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporter.java @@ -50,7 +50,7 @@ public void finishReporting() { flush(); } - public void flush() { + private void flush() { persistedTimingStats = new TimingStats(currentTimingStats); bulkResultsPersister.persistTimingStats(persistedTimingStats); } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java index 68572b3266363..9c86f05f2076e 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.ml.datafeed; +import org.elasticsearch.action.support.WriteRequest.RefreshPolicy; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats; @@ -16,6 +17,7 @@ import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoMoreInteractions; public class DatafeedTimingStatsReporterTests extends ESTestCase { @@ -30,8 +32,8 @@ public void setUpTests() { } public void testReportSearchDuration() { - DatafeedTimingStats timingStats = new DatafeedTimingStats(JOB_ID, 3, 10000.0); - DatafeedTimingStatsReporter timingStatsReporter = new DatafeedTimingStatsReporter(timingStats, jobResultsPersister); + DatafeedTimingStatsReporter timingStatsReporter = + new DatafeedTimingStatsReporter(new DatafeedTimingStats(JOB_ID, 3, 10000.0), jobResultsPersister); assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 3, 10000.0))); timingStatsReporter.reportSearchDuration(ONE_SECOND); @@ -47,9 +49,11 @@ public void testReportSearchDuration() { assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 7, 14000.0))); InOrder inOrder = inOrder(jobResultsPersister); - inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 5, 12000.0)); - inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 7, 14000.0)); - inOrder.verifyNoMoreInteractions(); + inOrder.verify(jobResultsPersister).persistDatafeedTimingStats( + new DatafeedTimingStats(JOB_ID, 5, 12000.0), RefreshPolicy.IMMEDIATE); + inOrder.verify(jobResultsPersister).persistDatafeedTimingStats( + new DatafeedTimingStats(JOB_ID, 7, 14000.0), RefreshPolicy.IMMEDIATE); + verifyNoMoreInteractions(jobResultsPersister); } public void testTimingStatsDifferSignificantly() { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java index ef866a2d91fde..dea47ef1b9d9b 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersisterTests.java @@ -12,6 +12,7 @@ import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.index.IndexResponse; +import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.client.Client; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -245,13 +246,14 @@ public void testPersistDatafeedTimingStats() { JobResultsPersister persister = new JobResultsPersister(client); DatafeedTimingStats timingStats = new DatafeedTimingStats("foo", 6, 666.0); - persister.persistDatafeedTimingStats(timingStats); + persister.persistDatafeedTimingStats(timingStats, WriteRequest.RefreshPolicy.IMMEDIATE); ArgumentCaptor indexRequestCaptor = ArgumentCaptor.forClass(IndexRequest.class); verify(client, times(1)).index(indexRequestCaptor.capture(), any(ActionListener.class)); IndexRequest indexRequest = indexRequestCaptor.getValue(); assertThat(indexRequest.index(), equalTo(".ml-anomalies-.write-foo")); assertThat(indexRequest.id(), equalTo("foo_datafeed_timing_stats")); + assertThat(indexRequest.getRefreshPolicy(), equalTo(WriteRequest.RefreshPolicy.IMMEDIATE)); assertThat( indexRequest.sourceAsMap(), equalTo( diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporterTests.java index f2314e6de3ed8..4e5a97f860d9d 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/TimingStatsReporterTests.java @@ -55,7 +55,7 @@ public void testReporting() { inOrder.verifyNoMoreInteractions(); } - public void testFlush() { + public void testFinishReporting() { TimingStatsReporter reporter = new TimingStatsReporter(new TimingStats(JOB_ID), bulkResultsPersister); assertThat(reporter.getCurrentTimingStats(), equalTo(new TimingStats(JOB_ID))); @@ -68,7 +68,7 @@ public void testFlush() { reporter.reportBucketProcessingTime(10); assertThat(reporter.getCurrentTimingStats(), equalTo(new TimingStats(JOB_ID, 3, 10.0, 10.0, 10.0, 10.0))); - reporter.flush(); + reporter.finishReporting(); assertThat(reporter.getCurrentTimingStats(), equalTo(new TimingStats(JOB_ID, 3, 10.0, 10.0, 10.0, 10.0))); InOrder inOrder = inOrder(bulkResultsPersister);