[ML] Refactoring scheduled event to store instant instead of zoned time zone (#39380)

The ScheduledEvent class has never preserved the time
zone so it makes more sense for it to store the start and
end time using Instant rather than ZonedDateTime.

Closes #38620
This commit is contained in:
Mehran Koushkebaghi 2019-02-27 09:18:03 +00:00 committed by David Roberts
parent a427a28318
commit 1d0097b5e8
7 changed files with 60 additions and 70 deletions

View File

@ -27,8 +27,6 @@ import org.elasticsearch.xpack.core.ml.utils.time.TimeUtils;
import java.io.IOException;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
@ -55,18 +53,18 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
parser.declareString(ScheduledEvent.Builder::description, DESCRIPTION);
parser.declareField(ScheduledEvent.Builder::startTime, p -> {
if (p.currentToken() == XContentParser.Token.VALUE_NUMBER) {
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(p.longValue()), ZoneOffset.UTC);
return Instant.ofEpochMilli(p.longValue());
} else if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(TimeUtils.dateStringToEpoch(p.text())), ZoneOffset.UTC);
return Instant.ofEpochMilli(TimeUtils.dateStringToEpoch(p.text()));
}
throw new IllegalArgumentException(
"unexpected token [" + p.currentToken() + "] for [" + START_TIME.getPreferredName() + "]");
}, START_TIME, ObjectParser.ValueType.VALUE);
parser.declareField(ScheduledEvent.Builder::endTime, p -> {
if (p.currentToken() == XContentParser.Token.VALUE_NUMBER) {
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(p.longValue()), ZoneOffset.UTC);
return Instant.ofEpochMilli(p.longValue());
} else if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(TimeUtils.dateStringToEpoch(p.text())), ZoneOffset.UTC);
return Instant.ofEpochMilli(TimeUtils.dateStringToEpoch(p.text()));
}
throw new IllegalArgumentException(
"unexpected token [" + p.currentToken() + "] for [" + END_TIME.getPreferredName() + "]");
@ -83,12 +81,12 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
}
private final String description;
private final ZonedDateTime startTime;
private final ZonedDateTime endTime;
private final Instant startTime;
private final Instant endTime;
private final String calendarId;
private final String eventId;
ScheduledEvent(String description, ZonedDateTime startTime, ZonedDateTime endTime, String calendarId, @Nullable String eventId) {
ScheduledEvent(String description, Instant startTime, Instant endTime, String calendarId, @Nullable String eventId) {
this.description = Objects.requireNonNull(description);
this.startTime = Objects.requireNonNull(startTime);
this.endTime = Objects.requireNonNull(endTime);
@ -98,8 +96,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
public ScheduledEvent(StreamInput in) throws IOException {
description = in.readString();
startTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(in.readVLong()), ZoneOffset.UTC);
endTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(in.readVLong()), ZoneOffset.UTC);
startTime = Instant.ofEpochMilli(in.readVLong());
endTime = Instant.ofEpochMilli(in.readVLong());
calendarId = in.readString();
eventId = in.readOptionalString();
}
@ -108,11 +106,11 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
return description;
}
public ZonedDateTime getStartTime() {
public Instant getStartTime() {
return startTime;
}
public ZonedDateTime getEndTime() {
public Instant getEndTime() {
return endTime;
}
@ -141,9 +139,9 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
long bucketSpanSecs = bucketSpan.getSeconds();
long bucketStartTime = Intervals.alignToFloor(getStartTime().toEpochSecond(), bucketSpanSecs);
long bucketStartTime = Intervals.alignToFloor(getStartTime().getEpochSecond(), bucketSpanSecs);
conditions.add(RuleCondition.createTime(Operator.GTE, bucketStartTime));
long bucketEndTime = Intervals.alignToCeil(getEndTime().toEpochSecond(), bucketSpanSecs);
long bucketEndTime = Intervals.alignToCeil(getEndTime().getEpochSecond(), bucketSpanSecs);
conditions.add(RuleCondition.createTime(Operator.LT, bucketEndTime));
DetectionRule.Builder builder = new DetectionRule.Builder(conditions);
@ -154,8 +152,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(description);
out.writeVLong(startTime.toInstant().toEpochMilli());
out.writeVLong(endTime.toInstant().toEpochMilli());
out.writeVLong(startTime.toEpochMilli());
out.writeVLong(endTime.toEpochMilli());
out.writeString(calendarId);
out.writeOptionalString(eventId);
}
@ -164,8 +162,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(DESCRIPTION.getPreferredName(), description);
builder.timeField(START_TIME.getPreferredName(), START_TIME.getPreferredName() + "_string", startTime.toInstant().toEpochMilli());
builder.timeField(END_TIME.getPreferredName(), END_TIME.getPreferredName() + "_string", endTime.toInstant().toEpochMilli());
builder.timeField(START_TIME.getPreferredName(), START_TIME.getPreferredName() + "_string", startTime.toEpochMilli());
builder.timeField(END_TIME.getPreferredName(), END_TIME.getPreferredName() + "_string", endTime.toEpochMilli());
builder.field(Calendar.ID.getPreferredName(), calendarId);
if (eventId != null) {
builder.field(EVENT_ID.getPreferredName(), eventId);
@ -197,8 +195,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
// Note ZonedDataTime.equals() fails because the time zone and date-time must be the same
// which isn't the case in tests where the time zone is randomised.
return description.equals(other.description)
&& Objects.equals(startTime.toInstant().getEpochSecond(), other.startTime.toInstant().getEpochSecond())
&& Objects.equals(endTime.toInstant().getEpochSecond(), other.endTime.toInstant().getEpochSecond())
&& Objects.equals(startTime.getEpochSecond(), other.startTime.getEpochSecond())
&& Objects.equals(endTime.getEpochSecond(), other.endTime.getEpochSecond())
&& calendarId.equals(other.calendarId);
}
@ -209,8 +207,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
public static class Builder {
private String description;
private ZonedDateTime startTime;
private ZonedDateTime endTime;
private Instant startTime;
private Instant endTime;
private String calendarId;
private String eventId;
@ -219,12 +217,12 @@ public class ScheduledEvent implements ToXContentObject, Writeable {
return this;
}
public Builder startTime(ZonedDateTime startTime) {
public Builder startTime(Instant startTime) {
this.startTime = startTime;
return this;
}
public Builder endTime(ZonedDateTime endTime) {
public Builder endTime(Instant endTime) {
this.endTime = endTime;
return this;
}

View File

@ -17,10 +17,7 @@ import org.elasticsearch.xpack.core.ml.job.config.RuleAction;
import org.elasticsearch.xpack.core.ml.job.config.RuleCondition;
import java.io.IOException;
import java.time.Clock;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.EnumSet;
import java.util.List;
@ -29,7 +26,7 @@ import static org.hamcrest.Matchers.containsString;
public class ScheduledEventTests extends AbstractSerializingTestCase<ScheduledEvent> {
public static ScheduledEvent createScheduledEvent(String calendarId) {
ZonedDateTime start = nowWithMillisResolution();
Instant start = Instant.ofEpochMilli(Instant.now().toEpochMilli());
return new ScheduledEvent(randomAlphaOfLength(10), start, start.plusSeconds(randomIntBetween(1, 10000)),
calendarId, null);
}
@ -72,7 +69,7 @@ public class ScheduledEventTests extends AbstractSerializingTestCase<ScheduledEv
long conditionEndTime = (long) conditions.get(1).getValue();
assertEquals(0, conditionEndTime % bucketSpanSecs);
long eventTime = event.getEndTime().toEpochSecond() - conditionStartTime;
long eventTime = event.getEndTime().getEpochSecond() - conditionStartTime;
long numbBucketsInEvent = (eventTime + bucketSpanSecs -1) / bucketSpanSecs;
assertEquals(bucketSpanSecs * (bucketCount + numbBucketsInEvent), conditionEndTime);
}
@ -85,11 +82,11 @@ public class ScheduledEventTests extends AbstractSerializingTestCase<ScheduledEv
builder.description("foo");
e = expectThrows(ElasticsearchStatusException.class, builder::build);
assertEquals("Field [start_time] cannot be null", e.getMessage());
ZonedDateTime now = ZonedDateTime.now();
Instant now = Instant.now();
builder.startTime(now);
e = expectThrows(ElasticsearchStatusException.class, builder::build);
assertEquals("Field [end_time] cannot be null", e.getMessage());
builder.endTime(now.plusHours(1));
builder.endTime(now.plusSeconds(1*60*60));
e = expectThrows(ElasticsearchStatusException.class, builder::build);
assertEquals("Field [calendar_id] cannot be null", e.getMessage());
builder.calendarId("foo");
@ -98,7 +95,7 @@ public class ScheduledEventTests extends AbstractSerializingTestCase<ScheduledEv
builder = new ScheduledEvent.Builder().description("f").calendarId("c");
builder.startTime(now);
builder.endTime(now.minusHours(2));
builder.endTime(now.minusSeconds(2*60*60));
e = expectThrows(ElasticsearchStatusException.class, builder::build);
assertThat(e.getMessage(), containsString("must come before end time"));
@ -120,8 +117,4 @@ public class ScheduledEventTests extends AbstractSerializingTestCase<ScheduledEv
ScheduledEvent.LENIENT_PARSER.apply(parser, null);
}
}
private static ZonedDateTime nowWithMillisResolution() {
return Instant.ofEpochMilli(Clock.systemUTC().millis()).atZone(ZoneOffset.UTC);
}
}

View File

@ -25,8 +25,6 @@ import org.junit.After;
import java.io.IOException;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@ -56,21 +54,21 @@ public class ScheduledEventsIT extends MlNativeAutodetectIntegTestCase {
long firstEventStartTime = 1514937600000L;
long firstEventEndTime = firstEventStartTime + 2 * 60 * 60 * 1000;
events.add(new ScheduledEvent.Builder().description("1st event (2hr)")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(firstEventStartTime), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(firstEventEndTime), ZoneOffset.UTC))
.startTime(Instant.ofEpochMilli(firstEventStartTime))
.endTime(Instant.ofEpochMilli(firstEventEndTime))
.calendarId(calendarId).build());
// add 10 min event smaller than the bucket
long secondEventStartTime = 1515067200000L;
long secondEventEndTime = secondEventStartTime + 10 * 60 * 1000;
events.add(new ScheduledEvent.Builder().description("2nd event with period smaller than bucketspan")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(secondEventStartTime), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(secondEventEndTime), ZoneOffset.UTC))
.startTime(Instant.ofEpochMilli(secondEventStartTime))
.endTime(Instant.ofEpochMilli(secondEventEndTime))
.calendarId(calendarId).build());
long thirdEventStartTime = 1515088800000L;
long thirdEventEndTime = thirdEventStartTime + 3 * 60 * 60 * 1000;
events.add(new ScheduledEvent.Builder().description("3rd event 3hr")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(thirdEventStartTime), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(thirdEventEndTime), ZoneOffset.UTC))
.startTime(Instant.ofEpochMilli(thirdEventStartTime))
.endTime(Instant.ofEpochMilli(thirdEventEndTime))
.calendarId(calendarId).build());
postScheduledEvents(calendarId, events);
@ -168,8 +166,8 @@ public class ScheduledEventsIT extends MlNativeAutodetectIntegTestCase {
long firstEventStartTime = startTime + bucketSpan.millis() * bucketCount;
long firstEventEndTime = firstEventStartTime + bucketSpan.millis() * 2;
events.add(new ScheduledEvent.Builder().description("1st event 2hr")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(firstEventStartTime), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(firstEventEndTime), ZoneOffset.UTC))
.startTime(Instant.ofEpochMilli(firstEventStartTime))
.endTime((Instant.ofEpochMilli(firstEventEndTime)))
.calendarId(calendarId).build());
postScheduledEvents(calendarId, events);
@ -217,8 +215,8 @@ public class ScheduledEventsIT extends MlNativeAutodetectIntegTestCase {
long eventStartTime = startTime + (bucketCount + 1) * bucketSpan.millis();
long eventEndTime = eventStartTime + (long)(1.5 * bucketSpan.millis());
events.add(new ScheduledEvent.Builder().description("Some Event")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(eventStartTime), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(eventEndTime), ZoneOffset.UTC))
.startTime((Instant.ofEpochMilli(eventStartTime)))
.endTime((Instant.ofEpochMilli(eventEndTime)))
.calendarId(calendarId).build());
postScheduledEvents(calendarId, events);
@ -287,8 +285,8 @@ public class ScheduledEventsIT extends MlNativeAutodetectIntegTestCase {
long eventStartTime = startTime + (bucketCount + 1) * bucketSpan.millis();
long eventEndTime = eventStartTime + (long)(1.5 * bucketSpan.millis());
events.add(new ScheduledEvent.Builder().description("Some Event")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(eventStartTime), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(eventEndTime), ZoneOffset.UTC))
.startTime((Instant.ofEpochMilli(eventStartTime)))
.endTime((Instant.ofEpochMilli(eventEndTime)))
.calendarId(calendarId).build());
postScheduledEvents(calendarId, events);

View File

@ -41,8 +41,8 @@ import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
import org.elasticsearch.xpack.ml.MlSingleNodeTestCase;
import org.elasticsearch.xpack.ml.job.persistence.CalendarQueryBuilder;
import org.elasticsearch.xpack.ml.job.persistence.JobDataCountsPersister;
import org.elasticsearch.xpack.ml.job.persistence.JobResultsProvider;
import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister;
import org.elasticsearch.xpack.ml.job.persistence.JobResultsProvider;
import org.elasticsearch.xpack.ml.job.persistence.ScheduledEventsQueryBuilder;
import org.elasticsearch.xpack.ml.job.process.autodetect.params.AutodetectParams;
import org.junit.Before;
@ -335,7 +335,12 @@ public class JobResultsProviderIT extends MlSingleNodeTestCase {
}
private ScheduledEvent buildScheduledEvent(String description, ZonedDateTime start, ZonedDateTime end, String calendarId) {
return new ScheduledEvent.Builder().description(description).startTime(start).endTime(end).calendarId(calendarId).build();
return new ScheduledEvent.Builder()
.description(description)
.startTime(start.toInstant())
.endTime(end.toInstant())
.calendarId(calendarId)
.build();
}
public void testGetAutodetectParams() throws Exception {

View File

@ -226,14 +226,14 @@ public class AutodetectControlMsgWriterTests extends ESTestCase {
ScheduledEvent.Builder event1 = new ScheduledEvent.Builder();
event1.calendarId("moon");
event1.description("new year");
event1.startTime(ZonedDateTime.parse("2018-01-01T00:00:00Z"));
event1.endTime(ZonedDateTime.parse("2018-01-02T00:00:00Z"));
event1.startTime(ZonedDateTime.parse("2018-01-01T00:00:00Z").toInstant());
event1.endTime(ZonedDateTime.parse("2018-01-02T00:00:00Z").toInstant());
ScheduledEvent.Builder event2 = new ScheduledEvent.Builder();
event2.calendarId("moon");
event2.description("Jan maintenance day");
event2.startTime(ZonedDateTime.parse("2018-01-06T00:00:00Z"));
event2.endTime(ZonedDateTime.parse("2018-01-07T00:00:00Z"));
event2.startTime(ZonedDateTime.parse("2018-01-06T00:00:00Z").toInstant());
event2.endTime(ZonedDateTime.parse("2018-01-07T00:00:00Z").toInstant());
writer.writeUpdateScheduledEventsMessage(Arrays.asList(event1.build(), event2.build()), TimeValue.timeValueHours(1));

View File

@ -30,8 +30,6 @@ import java.io.OutputStreamWriter;
import java.io.StringReader;
import java.nio.charset.StandardCharsets;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@ -239,12 +237,12 @@ public class FieldConfigWriterTests extends ESTestCase {
analysisConfig = builder.build();
scheduledEvents.add(new ScheduledEvent.Builder().description("The Ashes")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1511395200000L), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1515369600000L), ZoneOffset.UTC))
.startTime(Instant.ofEpochMilli(1511395200000L))
.endTime(Instant.ofEpochMilli(1515369600000L))
.calendarId("calendar_id").build());
scheduledEvents.add(new ScheduledEvent.Builder().description("elasticon")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1519603200000L), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1519862400000L), ZoneOffset.UTC))
.startTime(Instant.ofEpochMilli(1519603200000L))
.endTime(Instant.ofEpochMilli(1519862400000L))
.calendarId("calendar_id").build());
writer = mock(OutputStreamWriter.class);

View File

@ -11,8 +11,6 @@ import org.elasticsearch.xpack.core.ml.calendars.ScheduledEvent;
import java.io.IOException;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@ -32,12 +30,12 @@ public class ScheduledEventsWriterTests extends ESTestCase {
public void testWrite() throws IOException {
List<ScheduledEvent> events = new ArrayList<>();
events.add(new ScheduledEvent.Builder().description("Black Friday")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1511395200000L), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1515369600000L), ZoneOffset.UTC))
.startTime(Instant.ofEpochMilli(1511395200000L))
.endTime(Instant.ofEpochMilli(1515369600000L))
.calendarId("calendar_id").build());
events.add(new ScheduledEvent.Builder().description("Blue Monday")
.startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1519603200000L), ZoneOffset.UTC))
.endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1519862400000L), ZoneOffset.UTC))
.startTime(Instant.ofEpochMilli(1519603200000L))
.endTime(Instant.ofEpochMilli(1519862400000L))
.calendarId("calendar_id").build());
StringBuilder buffer = new StringBuilder();
@ -54,4 +52,4 @@ public class ScheduledEventsWriterTests extends ESTestCase {
"\n";
assertThat(buffer.toString(), equalTo(expectedString));
}
}
}