From 99e3508267d02e0eecd60683d67c9f025ea71947 Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Thu, 23 Mar 2017 18:45:53 +0000 Subject: [PATCH] [ML] Change chunking_config.time_span into a TimeValue (elastic/x-pack-elasticsearch#808) Original commit: elastic/x-pack-elasticsearch@42d7b06e3fcd05339cc9eff7b86f111e15bf6de9 --- .../xpack/ml/action/GetJobsStatsAction.java | 2 +- .../xpack/ml/datafeed/ChunkingConfig.java | 26 ++++++++++++------- .../chunked/ChunkedDataExtractor.java | 2 +- .../chunked/ChunkedDataExtractorContext.java | 5 ++-- .../ml/datafeed/ChunkingConfigTests.java | 14 +++++----- .../ml/datafeed/DatafeedUpdateTests.java | 4 +-- .../chunked/ChunkedDataExtractorTests.java | 15 ++++++----- .../rest-api-spec/test/ml/datafeeds_crud.yaml | 4 +-- 8 files changed, 39 insertions(+), 33 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetJobsStatsAction.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetJobsStatsAction.java index 5145a2f9897..9b279ec2215 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetJobsStatsAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetJobsStatsAction.java @@ -244,7 +244,7 @@ public class GetJobsStatsAction extends Action PARSER = new ConstructingObjectParser<>( - "chunking_config", a -> new ChunkingConfig((Mode) a[0], (Long) a[1])); + "chunking_config", a -> new ChunkingConfig((Mode) a[0], (TimeValue) a[1])); static { PARSER.declareField(ConstructingObjectParser.constructorArg(), p -> { @@ -39,31 +40,36 @@ public class ChunkingConfig extends ToXContentToBytes implements Writeable { } throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); }, MODE_FIELD, ValueType.STRING); - PARSER.declareLong(ConstructingObjectParser.optionalConstructorArg(), TIME_SPAN_FIELD); + PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(), p -> { + if (p.currentToken() == XContentParser.Token.VALUE_STRING) { + return TimeValue.parseTimeValue(p.text(), TIME_SPAN_FIELD.getPreferredName()); + } + throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); + }, TIME_SPAN_FIELD, ValueType.STRING); } private final Mode mode; - private final Long timeSpan; + private final TimeValue timeSpan; public ChunkingConfig(StreamInput in) throws IOException { mode = Mode.readFromStream(in); - timeSpan = in.readOptionalLong(); + timeSpan = in.readOptionalWriteable(TimeValue::new); } @Override public void writeTo(StreamOutput out) throws IOException { mode.writeTo(out); - out.writeOptionalLong(timeSpan); + out.writeOptionalWriteable(timeSpan); } - ChunkingConfig(Mode mode, @Nullable Long timeSpan) { + ChunkingConfig(Mode mode, @Nullable TimeValue timeSpan) { this.mode = ExceptionsHelper.requireNonNull(mode, MODE_FIELD.getPreferredName()); this.timeSpan = timeSpan; if (mode == Mode.MANUAL) { if (timeSpan == null) { throw new IllegalArgumentException("when chunk mode is manual time_span is required"); } - if (timeSpan <= 0) { + if (timeSpan.getMillis() <= 0) { throw new IllegalArgumentException("chunk time_span has to be positive"); } } else { @@ -74,7 +80,7 @@ public class ChunkingConfig extends ToXContentToBytes implements Writeable { } @Nullable - public Long getTimeSpan() { + public TimeValue getTimeSpan() { return timeSpan; } @@ -87,7 +93,7 @@ public class ChunkingConfig extends ToXContentToBytes implements Writeable { builder.startObject(); builder.field(MODE_FIELD.getPreferredName(), mode); if (timeSpan != null) { - builder.field(TIME_SPAN_FIELD.getPreferredName(), timeSpan); + builder.field(TIME_SPAN_FIELD.getPreferredName(), timeSpan.getStringRep()); } builder.endObject(); return builder; @@ -124,7 +130,7 @@ public class ChunkingConfig extends ToXContentToBytes implements Writeable { return new ChunkingConfig(Mode.OFF, null); } - public static ChunkingConfig newManual(long timeSpan) { + public static ChunkingConfig newManual(TimeValue timeSpan) { return new ChunkingConfig(Mode.MANUAL, timeSpan); } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java index 7304e0eedbf..5dc54182ef9 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractor.java @@ -95,7 +95,7 @@ public class ChunkedDataExtractor implements DataExtractor { if (dataSummary.totalHits > 0) { currentStart = dataSummary.earliestTime; currentEnd = currentStart; - chunkSpan = context.chunkSpan == null ? dataSummary.estimateChunk() : context.chunkSpan; + chunkSpan = context.chunkSpan == null ? dataSummary.estimateChunk() : context.chunkSpan.getMillis(); LOGGER.info("Chunked search configured: totalHits = {}, dataTimeSpread = {} ms, chunk span = {} ms", dataSummary.totalHits, dataSummary.getDataTimeSpread(), chunkSpan); } else { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorContext.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorContext.java index 67d18300df3..61fc3ed0c6f 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorContext.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/chunked/ChunkedDataExtractorContext.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.ml.datafeed.extractor.chunked; import org.elasticsearch.common.inject.internal.Nullable; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.query.QueryBuilder; import java.util.List; @@ -21,10 +22,10 @@ class ChunkedDataExtractorContext { final int scrollSize; final long start; final long end; - final Long chunkSpan; + final TimeValue chunkSpan; ChunkedDataExtractorContext(String jobId, String timeField, List indexes, List types, - QueryBuilder query, int scrollSize, long start, long end, @Nullable Long chunkSpan) { + QueryBuilder query, int scrollSize, long start, long end, @Nullable TimeValue chunkSpan) { this.jobId = Objects.requireNonNull(jobId); this.timeField = Objects.requireNonNull(timeField); this.indexes = indexes.toArray(new String[indexes.size()]); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/datafeed/ChunkingConfigTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/datafeed/ChunkingConfigTests.java index b6456a9128d..a886d3bba76 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/datafeed/ChunkingConfigTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/datafeed/ChunkingConfigTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.ml.datafeed; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.ml.support.AbstractSerializingTestCase; @@ -29,11 +30,11 @@ public class ChunkingConfigTests extends AbstractSerializingTestCasenew ChunkingConfig(ChunkingConfig.Mode.AUTO, 1000L)); + expectThrows(IllegalArgumentException.class, () ->new ChunkingConfig(ChunkingConfig.Mode.AUTO, TimeValue.timeValueMillis(1000))); } public void testConstructorGivenOffAndTimeSpan() { - expectThrows(IllegalArgumentException.class, () ->new ChunkingConfig(ChunkingConfig.Mode.OFF, 1000L)); + expectThrows(IllegalArgumentException.class, () ->new ChunkingConfig(ChunkingConfig.Mode.OFF, TimeValue.timeValueMillis(1000))); } public void testConstructorGivenManualAndNoTimeSpan() { @@ -42,18 +43,15 @@ public class ChunkingConfigTests extends AbstractSerializingTestCase indexes; private QueryBuilder query; private int scrollSize; - private Long chunkSpan; + private TimeValue chunkSpan; private DataExtractorFactory dataExtractorFactory; private class TestDataExtractor extends ChunkedDataExtractor { @@ -93,7 +94,7 @@ public class ChunkedDataExtractorTests extends ESTestCase { } public void testExtractionGivenSpecifiedChunk() throws IOException { - chunkSpan = 1000L; + chunkSpan = TimeValue.timeValueSeconds(1); TestDataExtractor extractor = new TestDataExtractor(1000L, 2300L); extractor.setNextResponse(createSearchResponse(10L, 1000L, 2200L)); @@ -317,7 +318,7 @@ public class ChunkedDataExtractorTests extends ESTestCase { } public void testCancelGivenNextWasNeverCalled() throws IOException { - chunkSpan = 1000L; + chunkSpan = TimeValue.timeValueSeconds(1); TestDataExtractor extractor = new TestDataExtractor(1000L, 2300L); extractor.setNextResponse(createSearchResponse(10L, 1000L, 2200L)); @@ -336,7 +337,7 @@ public class ChunkedDataExtractorTests extends ESTestCase { } public void testCancelGivenCurrentSubExtractorHasMore() throws IOException { - chunkSpan = 1000L; + chunkSpan = TimeValue.timeValueSeconds(1); TestDataExtractor extractor = new TestDataExtractor(1000L, 2300L); extractor.setNextResponse(createSearchResponse(10L, 1000L, 2200L)); @@ -363,7 +364,7 @@ public class ChunkedDataExtractorTests extends ESTestCase { } public void testCancelGivenCurrentSubExtractorIsDone() throws IOException { - chunkSpan = 1000L; + chunkSpan = TimeValue.timeValueSeconds(1); TestDataExtractor extractor = new TestDataExtractor(1000L, 2300L); extractor.setNextResponse(createSearchResponse(10L, 1000L, 2200L)); @@ -387,7 +388,7 @@ public class ChunkedDataExtractorTests extends ESTestCase { } public void testDataSummaryRequestIsNotOk() { - chunkSpan = 2000L; + chunkSpan = TimeValue.timeValueSeconds(2); TestDataExtractor extractor = new TestDataExtractor(1000L, 2300L); extractor.setNextResponse(createErrorResponse()); @@ -396,7 +397,7 @@ public class ChunkedDataExtractorTests extends ESTestCase { } public void testDataSummaryRequestHasShardFailures() { - chunkSpan = 2000L; + chunkSpan = TimeValue.timeValueSeconds(2); TestDataExtractor extractor = new TestDataExtractor(1000L, 2300L); extractor.setNextResponse(createResponseWithShardFailures()); diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yaml b/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yaml index 43fdc5e7156..f721e14bdb1 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yaml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yaml @@ -271,11 +271,11 @@ setup: "job_id":"job-1", "indexes":["index-foo"], "types":["type-bar"], - "chunking_config": {"mode":"manual","time_span": 3600} + "chunking_config": {"mode":"manual","time_span": "1h"} } - match: { datafeed_id: "test-datafeed-1" } - match: { chunking_config.mode: "manual" } - - match: { chunking_config.time_span: 3600 } + - match: { chunking_config.time_span: "1h" } --- "Test delete datafeed":