Skip to content
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.

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

Refactoring scheduled event to store instant instead of zoned tme zone #39380

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@

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;
Expand All @@ -55,18 +53,18 @@ private static ObjectParser<ScheduledEvent.Builder, Void> createParser(boolean i
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() + "]");
Expand All @@ -83,12 +81,12 @@ public static String documentId(String eventId) {
}

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);
Expand All @@ -98,8 +96,8 @@ public static String documentId(String eventId) {

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();
}
Expand All @@ -108,11 +106,11 @@ public String getDescription() {
return description;
}

public ZonedDateTime getStartTime() {
public Instant getStartTime() {
return startTime;
}

public ZonedDateTime getEndTime() {
public Instant getEndTime() {
return endTime;
}

Expand Down Expand Up @@ -141,9 +139,9 @@ public DetectionRule toDetectionRule(TimeValue bucketSpan) {

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);
Expand All @@ -154,8 +152,8 @@ public DetectionRule toDetectionRule(TimeValue bucketSpan) {
@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);
}
Expand All @@ -164,8 +162,8 @@ public void writeTo(StreamOutput out) throws IOException {
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);
Expand Down Expand Up @@ -197,8 +195,8 @@ public boolean equals(Object obj) {
// 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);
}

Expand All @@ -209,8 +207,8 @@ public int hashCode() {

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;

Expand All @@ -219,12 +217,12 @@ public Builder description(String description) {
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import org.elasticsearch.ElasticsearchStatusException;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.time.DateUtils;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.json.JsonXContent;
Expand All @@ -18,7 +17,7 @@
import org.elasticsearch.xpack.core.ml.job.config.RuleCondition;

import java.io.IOException;
import java.time.ZonedDateTime;
import java.time.Instant;
import java.util.EnumSet;
import java.util.List;

Expand All @@ -27,7 +26,7 @@
public class ScheduledEventTests extends AbstractSerializingTestCase<ScheduledEvent> {

public static ScheduledEvent createScheduledEvent(String calendarId) {
ZonedDateTime start = DateUtils.nowWithMillisResolution();
Instant start = Instant.ofEpochMilli(Instant.now().toEpochMilli());
return new ScheduledEvent(randomAlphaOfLength(10), start, start.plusSeconds(randomIntBetween(1, 10000)),
calendarId, null);
}
Expand Down Expand Up @@ -70,7 +69,7 @@ public void testToDetectionRule() {
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);
}
Expand All @@ -83,11 +82,11 @@ public void testBuild() {
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");
Expand All @@ -96,7 +95,7 @@ public void testBuild() {

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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@

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;
Expand Down Expand Up @@ -56,21 +54,21 @@ public void testScheduledEvents() throws IOException {
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);
Expand Down Expand Up @@ -168,8 +166,8 @@ public void testScheduledEventWithInterimResults() throws IOException {
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);

Expand Down Expand Up @@ -217,8 +215,8 @@ public void testAddEventsToOpenJob() throws Exception {
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);
Expand Down Expand Up @@ -287,8 +285,8 @@ public void testAddOpenedJobToGroupWithCalendar() throws Exception {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@
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;
Expand Down Expand Up @@ -335,7 +335,12 @@ public void testScheduledEventsForJob_withGroup() throws Exception {
}

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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,14 @@ public void testWriteUpdateScheduledEventsMessage() throws IOException {
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));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@
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;
Expand Down Expand Up @@ -239,12 +237,12 @@ public void testWrite_GivenScheduledEvents() throws IOException {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

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;
Expand All @@ -32,12 +30,12 @@ public void testWrite_GivenEmpty() throws IOException {
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();
Expand All @@ -54,4 +52,4 @@ public void testWrite() throws IOException {
"\n";
assertThat(buffer.toString(), equalTo(expectedString));
}
}
}