Skip to content

Add DatafeedTimingStats.average_search_time_per_bucket_ms and TimingStats.total_bucket_processing_time_ms stats #44125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 16, 2019
Merged

Add DatafeedTimingStats.average_search_time_per_bucket_ms and TimingStats.total_bucket_processing_time_ms stats #44125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aaf7106
Add avg_bucket_search_time_ms field to DatafeedTimingStats
przemekwitek Jul 9, 2019
64d8d84
Add new fields to ElasticsearchMappings and ReservedFieldNames
przemekwitek Jul 12, 2019
e92a8b4
Do not emit average_search_time_per_bucket_ms field if it is null
przemekwitek Jul 12, 2019
1355f4f
Apply review comments
przemekwitek Jul 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 35 additions & 5 deletions ...st-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStats.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ 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 BUCKET_COUNT = new ParseField("bucket_count");
public static final ParseField TOTAL_SEARCH_TIME_MS = new ParseField("total_search_time_ms");
public static final ParseField AVG_SEARCH_TIME_PER_BUCKET_MS = new ParseField("average_search_time_per_bucket_ms");

public static final ParseField TYPE = new ParseField("datafeed_timing_stats");

Expand All @@ -50,23 +52,37 @@ private static ConstructingObjectParser<DatafeedTimingStats, Void> createParser(
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));
Long bucketCount = (Long) args[2];
Double totalSearchTimeMs = (Double) args[3];
Double avgSearchTimePerBucketMs = (Double) args[4];
return new DatafeedTimingStats(
jobId,
getOrDefault(searchCount, 0L),
getOrDefault(bucketCount, 0L),
getOrDefault(totalSearchTimeMs, 0.0),
avgSearchTimePerBucketMs);
});
parser.declareString(constructorArg(), JOB_ID);
parser.declareLong(optionalConstructorArg(), SEARCH_COUNT);
parser.declareLong(optionalConstructorArg(), BUCKET_COUNT);
parser.declareDouble(optionalConstructorArg(), TOTAL_SEARCH_TIME_MS);
parser.declareDouble(optionalConstructorArg(), AVG_SEARCH_TIME_PER_BUCKET_MS);
return parser;
}

private final String jobId;
private long searchCount;
private long bucketCount;
private double totalSearchTimeMs;
private Double avgSearchTimePerBucketMs;

public DatafeedTimingStats(String jobId, long searchCount, double totalSearchTimeMs) {
public DatafeedTimingStats(
String jobId, long searchCount, long bucketCount, double totalSearchTimeMs, @Nullable Double avgSearchTimePerBucketMs) {
this.jobId = Objects.requireNonNull(jobId);
this.searchCount = searchCount;
this.bucketCount = bucketCount;
this.totalSearchTimeMs = totalSearchTimeMs;
this.avgSearchTimePerBucketMs = avgSearchTimePerBucketMs;
}

public String getJobId() {
Expand All @@ -77,16 +93,28 @@ public long getSearchCount() {
return searchCount;
}

public long getBucketCount() {
return bucketCount;
}

public double getTotalSearchTimeMs() {
return totalSearchTimeMs;
}

public Double getAvgSearchTimePerBucketMs() {
return avgSearchTimePerBucketMs;
}

@Override
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(BUCKET_COUNT.getPreferredName(), bucketCount);
builder.field(TOTAL_SEARCH_TIME_MS.getPreferredName(), totalSearchTimeMs);
if (avgSearchTimePerBucketMs != null) {
builder.field(AVG_SEARCH_TIME_PER_BUCKET_MS.getPreferredName(), avgSearchTimePerBucketMs);
}
builder.endObject();
return builder;
}
Expand All @@ -103,12 +131,14 @@ public boolean equals(Object obj) {
DatafeedTimingStats other = (DatafeedTimingStats) obj;
return Objects.equals(this.jobId, other.jobId)
&& this.searchCount == other.searchCount
&& this.totalSearchTimeMs == other.totalSearchTimeMs;
&& this.bucketCount == other.bucketCount
&& this.totalSearchTimeMs == other.totalSearchTimeMs
&& Objects.equals(this.avgSearchTimePerBucketMs, other.avgSearchTimePerBucketMs);
}

@Override
public int hashCode() {
return Objects.hash(jobId, searchCount, totalSearchTimeMs);
return Objects.hash(jobId, searchCount, bucketCount, totalSearchTimeMs, avgSearchTimePerBucketMs);
}

@Override
Expand Down
37 changes: 34 additions & 3 deletions ...nt/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/process/TimingStats.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
public class TimingStats implements ToXContentObject {

public static final ParseField BUCKET_COUNT = new ParseField("bucket_count");
public static final ParseField TOTAL_BUCKET_PROCESSING_TIME_MS = new ParseField("total_bucket_processing_time_ms");
public static final ParseField MIN_BUCKET_PROCESSING_TIME_MS = new ParseField("minimum_bucket_processing_time_ms");
public static final ParseField MAX_BUCKET_PROCESSING_TIME_MS = new ParseField("maximum_bucket_processing_time_ms");
public static final ParseField AVG_BUCKET_PROCESSING_TIME_MS = new ParseField("average_bucket_processing_time_ms");
Expand All @@ -49,12 +50,28 @@ public class TimingStats implements ToXContentObject {
new ConstructingObjectParser<>(
"timing_stats",
true,
args ->
new TimingStats((String) args[0], (long) args[1], (Double) args[2], (Double) args[3], (Double) args[4], (Double) args[5]));
args -> {
String jobId = (String) args[0];
Long bucketCount = (Long) args[1];
Double totalBucketProcessingTimeMs = (Double) args[2];
Double minBucketProcessingTimeMs = (Double) args[3];
Double maxBucketProcessingTimeMs = (Double) args[4];
Double avgBucketProcessingTimeMs = (Double) args[5];
Double exponentialAvgBucketProcessingTimeMs = (Double) args[6];
return new TimingStats(
jobId,
getOrDefault(bucketCount, 0L),
getOrDefault(totalBucketProcessingTimeMs, 0.0),
minBucketProcessingTimeMs,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why we are NOT allowing bucketCount and totalBucketProcessingTimeMs to be null but we ARE allowing all the other stats to be null. It seems to be that we should do logical zero values across the board.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking was that we should provide non-null values wherever possible to simplify dealing with the resulting stats.
We can do it for bucketCount and totalBucketProcessingTimeMs because "0" is natural default value for them.
However, Min, Max and AVG need at least one data point to be calculated so there is no natural default value here.
WDYT?

maxBucketProcessingTimeMs,
avgBucketProcessingTimeMs,
exponentialAvgBucketProcessingTimeMs);
});

static {
PARSER.declareString(constructorArg(), Job.ID);
PARSER.declareLong(constructorArg(), BUCKET_COUNT);
PARSER.declareLong(optionalConstructorArg(), BUCKET_COUNT);
PARSER.declareDouble(optionalConstructorArg(), TOTAL_BUCKET_PROCESSING_TIME_MS);
PARSER.declareDouble(optionalConstructorArg(), MIN_BUCKET_PROCESSING_TIME_MS);
PARSER.declareDouble(optionalConstructorArg(), MAX_BUCKET_PROCESSING_TIME_MS);
PARSER.declareDouble(optionalConstructorArg(), AVG_BUCKET_PROCESSING_TIME_MS);
Expand All @@ -63,6 +80,7 @@ public class TimingStats implements ToXContentObject {

private final String jobId;
private long bucketCount;
private double totalBucketProcessingTimeMs;
private Double minBucketProcessingTimeMs;
private Double maxBucketProcessingTimeMs;
private Double avgBucketProcessingTimeMs;
Expand All @@ -71,12 +89,14 @@ public class TimingStats implements ToXContentObject {
public TimingStats(
String jobId,
long bucketCount,
double totalBucketProcessingTimeMs,
@Nullable Double minBucketProcessingTimeMs,
@Nullable Double maxBucketProcessingTimeMs,
@Nullable Double avgBucketProcessingTimeMs,
@Nullable Double exponentialAvgBucketProcessingTimeMs) {
this.jobId = jobId;
this.bucketCount = bucketCount;
this.totalBucketProcessingTimeMs = totalBucketProcessingTimeMs;
this.minBucketProcessingTimeMs = minBucketProcessingTimeMs;
this.maxBucketProcessingTimeMs = maxBucketProcessingTimeMs;
this.avgBucketProcessingTimeMs = avgBucketProcessingTimeMs;
Expand All @@ -91,6 +111,10 @@ public long getBucketCount() {
return bucketCount;
}

public double getTotalBucketProcessingTimeMs() {
return totalBucketProcessingTimeMs;
}

public Double getMinBucketProcessingTimeMs() {
return minBucketProcessingTimeMs;
}
Expand All @@ -112,6 +136,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
builder.startObject();
builder.field(Job.ID.getPreferredName(), jobId);
builder.field(BUCKET_COUNT.getPreferredName(), bucketCount);
builder.field(TOTAL_BUCKET_PROCESSING_TIME_MS.getPreferredName(), totalBucketProcessingTimeMs);
if (minBucketProcessingTimeMs != null) {
builder.field(MIN_BUCKET_PROCESSING_TIME_MS.getPreferredName(), minBucketProcessingTimeMs);
}
Expand All @@ -135,6 +160,7 @@ public boolean equals(Object o) {
TimingStats that = (TimingStats) o;
return Objects.equals(this.jobId, that.jobId)
&& this.bucketCount == that.bucketCount
&& this.totalBucketProcessingTimeMs == that.totalBucketProcessingTimeMs
&& Objects.equals(this.minBucketProcessingTimeMs, that.minBucketProcessingTimeMs)
&& Objects.equals(this.maxBucketProcessingTimeMs, that.maxBucketProcessingTimeMs)
&& Objects.equals(this.avgBucketProcessingTimeMs, that.avgBucketProcessingTimeMs)
Expand All @@ -146,6 +172,7 @@ public int hashCode() {
return Objects.hash(
jobId,
bucketCount,
totalBucketProcessingTimeMs,
minBucketProcessingTimeMs,
maxBucketProcessingTimeMs,
avgBucketProcessingTimeMs,
Expand All @@ -156,4 +183,8 @@ public int hashCode() {
public String toString() {
return Strings.toString(this);
}

private static <T> T getOrDefault(@Nullable T value, T defaultValue) {
return value != null ? value : defaultValue;
}
}
22 changes: 14 additions & 8 deletions ...gh-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedTimingStatsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@
import java.io.IOException;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;

public class DatafeedTimingStatsTests extends AbstractXContentTestCase<DatafeedTimingStats> {

private static final String JOB_ID = "my-job-id";

public static DatafeedTimingStats createRandomInstance() {
return new DatafeedTimingStats(randomAlphaOfLength(10), randomLong(), randomDouble());
return new DatafeedTimingStats(
randomAlphaOfLength(10), randomLong(), randomLong(), randomDouble(), randomBoolean() ? null : randomDouble());
}

@Override
Expand All @@ -59,34 +61,38 @@ public void testParse_OptionalFieldsAbsent() throws IOException {
DatafeedTimingStats stats = DatafeedTimingStats.PARSER.apply(parser, null);
assertThat(stats.getJobId(), equalTo(JOB_ID));
assertThat(stats.getSearchCount(), equalTo(0L));
assertThat(stats.getBucketCount(), equalTo(0L));
assertThat(stats.getTotalSearchTimeMs(), equalTo(0.0));
assertThat(stats.getAvgSearchTimePerBucketMs(), nullValue());
}
}

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);
DatafeedTimingStats stats1 = new DatafeedTimingStats(JOB_ID, 5, 10, 100.0, 20.0);
DatafeedTimingStats stats2 = new DatafeedTimingStats(JOB_ID, 5, 10, 100.0, 20.0);
DatafeedTimingStats stats3 = new DatafeedTimingStats(JOB_ID, 5, 10, 200.0, 20.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);
DatafeedTimingStats stats1 = new DatafeedTimingStats(JOB_ID, 5, 10, 100.0, 20.0);
DatafeedTimingStats stats2 = new DatafeedTimingStats(JOB_ID, 5, 10, 100.0, 20.0);
DatafeedTimingStats stats3 = new DatafeedTimingStats(JOB_ID, 5, 10, 200.0, 20.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);
DatafeedTimingStats stats = new DatafeedTimingStats(JOB_ID, 5, 10, 123.456, 78.9);
assertThat(stats.getJobId(), equalTo(JOB_ID));
assertThat(stats.getSearchCount(), equalTo(5L));
assertThat(stats.getBucketCount(), equalTo(10L));
assertThat(stats.getTotalSearchTimeMs(), equalTo(123.456));
assertThat(stats.getAvgSearchTimePerBucketMs(), equalTo(78.9));
}
}
40 changes: 32 additions & 8 deletions ...st-high-level/src/test/java/org/elasticsearch/client/ml/job/process/TimingStatsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@
*/
package org.elasticsearch.client.ml.job.process;

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;
import static org.hamcrest.Matchers.nullValue;

Expand All @@ -32,6 +37,7 @@ public static TimingStats createTestInstance(String jobId) {
return new TimingStats(
jobId,
randomLong(),
randomDouble(),
randomBoolean() ? null : randomDouble(),
randomBoolean() ? null : randomDouble(),
randomBoolean() ? null : randomDouble(),
Expand All @@ -54,41 +60,59 @@ protected boolean supportsUnknownFields() {
}

public void testConstructor() {
TimingStats stats = new TimingStats(JOB_ID, 7, 1.0, 2.0, 1.23, 7.89);
TimingStats stats = new TimingStats(JOB_ID, 7, 8.61, 1.0, 2.0, 1.23, 7.89);

assertThat(stats.getJobId(), equalTo(JOB_ID));
assertThat(stats.getBucketCount(), equalTo(7L));
assertThat(stats.getTotalBucketProcessingTimeMs(), equalTo(8.61));
assertThat(stats.getMinBucketProcessingTimeMs(), equalTo(1.0));
assertThat(stats.getMaxBucketProcessingTimeMs(), equalTo(2.0));
assertThat(stats.getAvgBucketProcessingTimeMs(), equalTo(1.23));
assertThat(stats.getExponentialAvgBucketProcessingTimeMs(), equalTo(7.89));
}

public void testConstructor_NullValues() {
TimingStats stats = new TimingStats(JOB_ID, 7, null, null, null, null);
TimingStats stats = new TimingStats(JOB_ID, 7, 8.61, null, null, null, null);

assertThat(stats.getJobId(), equalTo(JOB_ID));
assertThat(stats.getBucketCount(), equalTo(7L));
assertThat(stats.getTotalBucketProcessingTimeMs(), equalTo(8.61));
assertThat(stats.getMinBucketProcessingTimeMs(), nullValue());
assertThat(stats.getMaxBucketProcessingTimeMs(), nullValue());
assertThat(stats.getAvgBucketProcessingTimeMs(), nullValue());
assertThat(stats.getExponentialAvgBucketProcessingTimeMs(), nullValue());
}

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)) {
TimingStats stats = TimingStats.PARSER.apply(parser, null);
assertThat(stats.getJobId(), equalTo(JOB_ID));
assertThat(stats.getBucketCount(), equalTo(0L));
assertThat(stats.getTotalBucketProcessingTimeMs(), equalTo(0.0));
assertThat(stats.getMinBucketProcessingTimeMs(), nullValue());
assertThat(stats.getMaxBucketProcessingTimeMs(), nullValue());
assertThat(stats.getAvgBucketProcessingTimeMs(), nullValue());
assertThat(stats.getExponentialAvgBucketProcessingTimeMs(), nullValue());
}
}

public void testEquals() {
TimingStats stats1 = new TimingStats(JOB_ID, 7, 1.0, 2.0, 1.23, 7.89);
TimingStats stats2 = new TimingStats(JOB_ID, 7, 1.0, 2.0, 1.23, 7.89);
TimingStats stats3 = new TimingStats(JOB_ID, 7, 1.0, 3.0, 1.23, 7.89);
TimingStats stats1 = new TimingStats(JOB_ID, 7, 8.61, 1.0, 2.0, 1.23, 7.89);
TimingStats stats2 = new TimingStats(JOB_ID, 7, 8.61, 1.0, 2.0, 1.23, 7.89);
TimingStats stats3 = new TimingStats(JOB_ID, 7, 8.61, 1.0, 3.0, 1.23, 7.89);

assertTrue(stats1.equals(stats1));
assertTrue(stats1.equals(stats2));
assertFalse(stats2.equals(stats3));
}

public void testHashCode() {
TimingStats stats1 = new TimingStats(JOB_ID, 7, 1.0, 2.0, 1.23, 7.89);
TimingStats stats2 = new TimingStats(JOB_ID, 7, 1.0, 2.0, 1.23, 7.89);
TimingStats stats3 = new TimingStats(JOB_ID, 7, 1.0, 3.0, 1.23, 7.89);
TimingStats stats1 = new TimingStats(JOB_ID, 7, 8.61, 1.0, 2.0, 1.23, 7.89);
TimingStats stats2 = new TimingStats(JOB_ID, 7, 8.61, 1.0, 2.0, 1.23, 7.89);
TimingStats stats3 = new TimingStats(JOB_ID, 7, 8.61, 1.0, 3.0, 1.23, 7.89);

assertEquals(stats1.hashCode(), stats1.hashCode());
assertEquals(stats1.hashCode(), stats2.hashCode());
Expand Down
Loading