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

Singularity Request - Timezone field (API) #1150

Merged
merged 14 commits into from
Aug 5, 2016
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 @@ -24,6 +24,7 @@ public class SingularityRequest {
private final Optional<String> schedule;
private final Optional<String> quartzSchedule;
private final Optional<ScheduleType> scheduleType;
private final Optional<String> scheduleTimeZone;

private final Optional<Long> killOldNonLongRunningTasksAfterMillis;
private final Optional<Long> scheduledExpectedRuntimeMillis;
Expand Down Expand Up @@ -57,7 +58,7 @@ public SingularityRequest(@JsonProperty("id") String id, @JsonProperty("requestT
@JsonProperty("numRetriesOnFailure") Optional<Integer> numRetriesOnFailure, @JsonProperty("schedule") Optional<String> schedule, @JsonProperty("instances") Optional<Integer> instances,
@JsonProperty("rackSensitive") Optional<Boolean> rackSensitive, @JsonProperty("loadBalanced") Optional<Boolean> loadBalanced,
@JsonProperty("killOldNonLongRunningTasksAfterMillis") Optional<Long> killOldNonLongRunningTasksAfterMillis, @JsonProperty("scheduleType") Optional<ScheduleType> scheduleType,
@JsonProperty("quartzSchedule") Optional<String> quartzSchedule, @JsonProperty("rackAffinity") Optional<List<String>> rackAffinity,
@JsonProperty("quartzSchedule") Optional<String> quartzSchedule, @JsonProperty("scheduleTimeZone") Optional<String> scheduleTimeZone, @JsonProperty("rackAffinity") Optional<List<String>> rackAffinity,
@JsonProperty("slavePlacement") Optional<SlavePlacement> slavePlacement, @JsonProperty("requiredSlaveAttributes") Optional<Map<String, String>> requiredSlaveAttributes,
@JsonProperty("allowedSlaveAttributes") Optional<Map<String, String>> allowedSlaveAttributes, @JsonProperty("scheduledExpectedRuntimeMillis") Optional<Long> scheduledExpectedRuntimeMillis,
@JsonProperty("waitAtLeastMillisAfterTaskFinishesForReschedule") Optional<Long> waitAtLeastMillisAfterTaskFinishesForReschedule, @JsonProperty("group") Optional<String> group,
Expand All @@ -76,6 +77,7 @@ public SingularityRequest(@JsonProperty("id") String id, @JsonProperty("requestT
this.killOldNonLongRunningTasksAfterMillis = killOldNonLongRunningTasksAfterMillis;
this.scheduleType = scheduleType;
this.quartzSchedule = quartzSchedule;
this.scheduleTimeZone = scheduleTimeZone;
this.rackAffinity = rackAffinity;
this.slavePlacement = slavePlacement;
this.requiredSlaveAttributes = requiredSlaveAttributes;
Expand Down Expand Up @@ -108,6 +110,7 @@ public SingularityRequestBuilder toBuilder() {
.setKillOldNonLongRunningTasksAfterMillis(killOldNonLongRunningTasksAfterMillis)
.setScheduleType(scheduleType)
.setQuartzSchedule(quartzSchedule)
.setScheduleTimeZone(scheduleTimeZone)
.setRackAffinity(copyOfList(rackAffinity))
.setWaitAtLeastMillisAfterTaskFinishesForReschedule(waitAtLeastMillisAfterTaskFinishesForReschedule)
.setSlavePlacement(slavePlacement)
Expand Down Expand Up @@ -144,6 +147,10 @@ public Optional<String> getQuartzSchedule() {
return quartzSchedule;
}

public Optional<String> getScheduleTimeZone() {
return scheduleTimeZone;
}

public Optional<Integer> getInstances() {
return instances;
}
Expand Down Expand Up @@ -280,6 +287,7 @@ public String toString() {
", numRetriesOnFailure=" + numRetriesOnFailure +
", schedule=" + schedule +
", quartzSchedule=" + quartzSchedule +
", scheduleTimeZone=" + scheduleTimeZone +
", scheduleType=" + scheduleType +
", killOldNonLongRunningTasksAfterMillis=" + killOldNonLongRunningTasksAfterMillis +
", scheduledExpectedRuntimeMillis=" + scheduledExpectedRuntimeMillis +
Expand Down Expand Up @@ -316,6 +324,7 @@ public boolean equals(Object o) {
Objects.equals(numRetriesOnFailure, request.numRetriesOnFailure) &&
Objects.equals(schedule, request.schedule) &&
Objects.equals(quartzSchedule, request.quartzSchedule) &&
Objects.equals(scheduleTimeZone, request.scheduleTimeZone) &&
Objects.equals(scheduleType, request.scheduleType) &&
Objects.equals(killOldNonLongRunningTasksAfterMillis, request.killOldNonLongRunningTasksAfterMillis) &&
Objects.equals(scheduledExpectedRuntimeMillis, request.scheduledExpectedRuntimeMillis) &&
Expand All @@ -338,6 +347,6 @@ public boolean equals(Object o) {

@Override
public int hashCode() {
return Objects.hash(id, requestType, owners, numRetriesOnFailure, schedule, quartzSchedule, scheduleType, killOldNonLongRunningTasksAfterMillis, scheduledExpectedRuntimeMillis, waitAtLeastMillisAfterTaskFinishesForReschedule, instances, rackSensitive, rackAffinity, slavePlacement, requiredSlaveAttributes, allowedSlaveAttributes, loadBalanced, group, readOnlyGroups, bounceAfterScale, emailConfigurationOverrides, hideEvenNumberAcrossRacksHint, taskLogErrorRegex, taskLogErrorRegexCaseSensitive);
return Objects.hash(id, requestType, owners, numRetriesOnFailure, schedule, quartzSchedule, scheduleTimeZone, scheduleType, killOldNonLongRunningTasksAfterMillis, scheduledExpectedRuntimeMillis, waitAtLeastMillisAfterTaskFinishesForReschedule, instances, rackSensitive, rackAffinity, slavePlacement, requiredSlaveAttributes, allowedSlaveAttributes, loadBalanced, group, readOnlyGroups, bounceAfterScale, emailConfigurationOverrides, hideEvenNumberAcrossRacksHint, taskLogErrorRegex, taskLogErrorRegexCaseSensitive);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public class SingularityRequestBuilder {

private Optional<String> schedule;
private Optional<String> quartzSchedule;
private Optional<String> scheduleTimeZone;
private Optional<ScheduleType> scheduleType;

private Optional<Long> killOldNonLongRunningTasksAfterMillis;
Expand Down Expand Up @@ -57,6 +58,7 @@ public SingularityRequestBuilder(String id, RequestType requestType) {
this.rackSensitive = Optional.absent();
this.loadBalanced = Optional.absent();
this.quartzSchedule = Optional.absent();
this.scheduleTimeZone = Optional.absent();
this.rackAffinity = Optional.absent();
this.slavePlacement = Optional.absent();
this.requiredSlaveAttributes = Optional.absent();
Expand All @@ -74,7 +76,7 @@ public SingularityRequestBuilder(String id, RequestType requestType) {
}

public SingularityRequest build() {
return new SingularityRequest(id, requestType, owners, numRetriesOnFailure, schedule, instances, rackSensitive, loadBalanced, killOldNonLongRunningTasksAfterMillis, scheduleType, quartzSchedule,
return new SingularityRequest(id, requestType, owners, numRetriesOnFailure, schedule, instances, rackSensitive, loadBalanced, killOldNonLongRunningTasksAfterMillis, scheduleType, quartzSchedule, scheduleTimeZone,
rackAffinity, slavePlacement, requiredSlaveAttributes, allowedSlaveAttributes, scheduledExpectedRuntimeMillis, waitAtLeastMillisAfterTaskFinishesForReschedule, group, readOnlyGroups,
bounceAfterScale, skipHealthchecks, emailConfigurationOverrides, Optional.<Boolean>absent(), hideEvenNumberAcrossRacksHint, taskLogErrorRegex, taskLogErrorRegexCaseSensitive);
}
Expand Down Expand Up @@ -173,6 +175,15 @@ public SingularityRequestBuilder setQuartzSchedule(Optional<String> quartzSchedu
return this;
}

public Optional<String> getScheduleTimeZone() {
return scheduleTimeZone;
}

public SingularityRequestBuilder setScheduleTimeZone(Optional<String> scheduleTimeZone) {
this.scheduleTimeZone = scheduleTimeZone;
return this;
}

public Optional<List<String>> getRackAffinity() {
return rackAffinity;
}
Expand Down Expand Up @@ -289,6 +300,7 @@ public String toString() {
", numRetriesOnFailure=" + numRetriesOnFailure +
", schedule=" + schedule +
", quartzSchedule=" + quartzSchedule +
", scheduleTimeZone=" + scheduleTimeZone +
", scheduleType=" + scheduleType +
", killOldNonLongRunningTasksAfterMillis=" + killOldNonLongRunningTasksAfterMillis +
", scheduledExpectedRuntimeMillis=" + scheduledExpectedRuntimeMillis +
Expand Down Expand Up @@ -326,6 +338,7 @@ public boolean equals(Object o) {
Objects.equals(numRetriesOnFailure, that.numRetriesOnFailure) &&
Objects.equals(schedule, that.schedule) &&
Objects.equals(quartzSchedule, that.quartzSchedule) &&
Objects.equals(scheduleTimeZone, that.scheduleTimeZone) &&
Objects.equals(scheduleType, that.scheduleType) &&
Objects.equals(killOldNonLongRunningTasksAfterMillis, that.killOldNonLongRunningTasksAfterMillis) &&
Objects.equals(scheduledExpectedRuntimeMillis, that.scheduledExpectedRuntimeMillis) &&
Expand All @@ -349,7 +362,7 @@ public boolean equals(Object o) {

@Override
public int hashCode() {
return Objects.hash(id, requestType, owners, numRetriesOnFailure, schedule, quartzSchedule, scheduleType, killOldNonLongRunningTasksAfterMillis,
return Objects.hash(id, requestType, owners, numRetriesOnFailure, schedule, quartzSchedule, scheduleTimeZone, scheduleType, killOldNonLongRunningTasksAfterMillis,
scheduledExpectedRuntimeMillis, waitAtLeastMillisAfterTaskFinishesForReschedule, instances, rackSensitive, rackAffinity, slavePlacement,
requiredSlaveAttributes, allowedSlaveAttributes, loadBalanced, group, readOnlyGroups, bounceAfterScale, skipHealthchecks, emailConfigurationOverrides,
hideEvenNumberAcrossRacksHint, taskLogErrorRegex, taskLogErrorRegexCaseSensitive);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.TimeZone;
import java.util.UUID;
import java.util.regex.Pattern;

import javax.inject.Singleton;

import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.StringUtils;
import org.dmfs.rfc5545.recur.InvalidRecurrenceRuleException;
import org.dmfs.rfc5545.recur.RecurrenceRule;
Expand All @@ -36,7 +38,6 @@
import com.hubspot.singularity.SingularityDeploy;
import com.hubspot.singularity.SingularityDeployBuilder;
import com.hubspot.singularity.SingularityRequest;
import com.hubspot.singularity.WebExceptions;
import com.hubspot.singularity.SingularityWebhook;
import com.hubspot.singularity.config.SingularityConfiguration;
import com.hubspot.singularity.data.history.DeployHistoryHelper;
Expand Down Expand Up @@ -156,7 +157,7 @@ public SingularityRequest checkSingularityRequest(SingularityRequest request, Op
quartzSchedule = getQuartzScheduleFromCronSchedule(originalSchedule);
}

checkBadRequest(isValidCronSchedule(quartzSchedule), "Schedule %s (from: %s) was not valid", quartzSchedule, originalSchedule);
checkBadRequest(isValidCronSchedule(quartzSchedule), "Schedule %s (from: %s) is not valid", quartzSchedule, originalSchedule);
} else {
checkForValidRFC5545Schedule(request.getSchedule().get());
}
Expand All @@ -165,6 +166,12 @@ public SingularityRequest checkSingularityRequest(SingularityRequest request, Op
checkBadRequest(!request.getScheduleType().isPresent(), "ScheduleType can only be set for scheduled requests");
}

if (request.getScheduleTimeZone().isPresent()) {
if (!ArrayUtils.contains(TimeZone.getAvailableIDs(), request.getScheduleTimeZone().get())) {
badRequest("scheduleTimeZone %s does not map to a valid Java TimeZone object (e.g. 'US/Eastern' or 'GMT')", request.getScheduleTimeZone().get());
}
}

if (!request.isLongRunning()) {
checkBadRequest(!request.isLoadBalanced(), "non-longRunning (scheduled/oneoff) requests can not be load balanced");
checkBadRequest(!request.isRackSensitive(), "non-longRunning (scheduled/oneoff) requests can not be rack sensitive");
Expand All @@ -186,7 +193,7 @@ private void checkForValidRFC5545Schedule(String schedule) {
try {
new RecurrenceRule(schedule);
} catch (InvalidRecurrenceRuleException ex) {
badRequest("Schedule %s was not a valid RFC5545 schedule, error was: %s", schedule, ex);
badRequest("Schedule %s is not a valid RFC5545 schedule, error is: %s", schedule, ex);
}
}

Expand All @@ -197,7 +204,7 @@ public SingularityWebhook checkSingularityWebhook(SingularityWebhook webhook) {
try {
new URI(webhook.getUri());
} catch (URISyntaxException e) {
WebExceptions.badRequest("Invalid URI provided");
badRequest("Invalid URI provided");
}

return webhook;
Expand Down Expand Up @@ -415,7 +422,7 @@ private String getNewDayOfWeekValue(String schedule, int dayOfWeekValue) {
newDayOfWeekValue = "SAT";
break;
default:
WebExceptions.badRequest("Schedule %s is invalid, day of week (%s) is not 0-7", schedule, dayOfWeekValue);
badRequest("Schedule %s is invalid, day of week (%s) is not 0-7", schedule, dayOfWeekValue);
break;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.TimeZone;
import java.util.concurrent.TimeUnit;

import javax.inject.Singleton;
Expand Down Expand Up @@ -39,9 +40,9 @@
import com.hubspot.singularity.ScheduleType;
import com.hubspot.singularity.SingularityCreateResult;
import com.hubspot.singularity.SingularityDeployMarker;
import com.hubspot.singularity.SingularityDeployProgress;
import com.hubspot.singularity.SingularityDeployStatistics;
import com.hubspot.singularity.SingularityDeployStatisticsBuilder;
import com.hubspot.singularity.SingularityDeployProgress;
import com.hubspot.singularity.SingularityMachineAbstraction;
import com.hubspot.singularity.SingularityPendingDeploy;
import com.hubspot.singularity.SingularityPendingRequest;
Expand Down Expand Up @@ -695,6 +696,9 @@ private Optional<Long> getNextRunAt(SingularityRequest request, RequestState sta
} else {
scheduleFrom = new Date(now);
final CronExpression cronExpression = new CronExpression(request.getQuartzScheduleSafe());
if (request.getScheduleTimeZone().isPresent()) {
cronExpression.setTimeZone(TimeZone.getTimeZone(request.getScheduleTimeZone().get()));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick, but the variable here is probably unnecessary. also, if the variable is needed, try to make all your variables final unless they definitely need to be mutable (see above for an example)

nextRunAtDate = cronExpression.getNextValidTimeAfter(scheduleFrom);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import java.util.Set;
import java.util.concurrent.TimeUnit;

import javax.ws.rs.WebApplicationException;

import org.apache.mesos.Protos.Offer;
import org.apache.mesos.Protos.SlaveID;
import org.apache.mesos.Protos.TaskID;
Expand Down Expand Up @@ -39,6 +41,7 @@
import com.hubspot.singularity.RequestCleanupType;
import com.hubspot.singularity.RequestState;
import com.hubspot.singularity.RequestType;
import com.hubspot.singularity.ScheduleType;
import com.hubspot.singularity.SingularityDeploy;
import com.hubspot.singularity.SingularityDeployBuilder;
import com.hubspot.singularity.SingularityDeployProgress;
Expand Down Expand Up @@ -3058,6 +3061,66 @@ public void testCronScheduleChanges() throws Exception {
Assert.assertEquals(newScheduleQuartz, requestManager.getRequest(requestId).get().getRequest().getQuartzScheduleSafe());
}

@Test(expected = WebApplicationException.class)
public void testInvalidQuartzTimeZoneErrors() {
SingularityRequest req = new SingularityRequestBuilder(requestId, RequestType.SCHEDULED)
.setQuartzSchedule(Optional.of("*/1 * * * * ? 2020"))
.setScheduleType(Optional.of(ScheduleType.QUARTZ))
.setScheduleTimeZone(Optional.of("invalid_timezone"))
.build();

requestResource.postRequest(req);
}

@Test
public void testDifferentQuartzTimeZones() {
final Optional<String> schedule = Optional.of("* 30 14 22 3 ? 2083");

SingularityRequest requestEST = new SingularityRequestBuilder("est_id", RequestType.SCHEDULED)
.setSchedule(schedule)
.setScheduleType(Optional.of(ScheduleType.QUARTZ))
.setScheduleTimeZone(Optional.of("EST")) // fixed in relation to GMT
.build();

SingularityRequest requestGMT = new SingularityRequestBuilder("gmt_id", RequestType.SCHEDULED)
.setSchedule(schedule)
.setScheduleType(Optional.of(ScheduleType.QUARTZ))
.setScheduleTimeZone(Optional.of("GMT"))
.build();

requestResource.postRequest(requestEST);
requestResource.postRequest(requestGMT);

SingularityDeploy deployEST = new SingularityDeployBuilder(requestEST.getId(), "est_deploy_id")
.setCommand(Optional.of("sleep 1"))
.build();

SingularityDeploy deployGMT = new SingularityDeployBuilder(requestGMT.getId(), "gmt_deploy_id")
.setCommand(Optional.of("sleep 1"))
.build();

deployResource.deploy(new SingularityDeployRequest(deployEST, Optional.<Boolean>absent(), Optional.<String>absent(), Optional.<SingularityRequest>absent()));
deployResource.deploy(new SingularityDeployRequest(deployGMT, Optional.<Boolean>absent(), Optional.<String>absent(), Optional.<SingularityRequest>absent()));

deployChecker.checkDeploys();
scheduler.drainPendingQueue(stateCacheProvider.get());

final long nextRunEST;
final long nextRunGMT;
final long fiveHoursInMilliseconds = TimeUnit.HOURS.toMillis(5);
final List<SingularityPendingTaskId> pendingTaskIds = taskManager.getPendingTaskIds();
if (pendingTaskIds.get(0).getRequestId().equals(requestEST.getId())) {
nextRunEST = pendingTaskIds.get(0).getNextRunAt();
nextRunGMT = pendingTaskIds.get(1).getNextRunAt();
} else {
nextRunEST = pendingTaskIds.get(1).getNextRunAt();
nextRunGMT = pendingTaskIds.get(0).getNextRunAt();
}

// GMT happens first, so EST is a larger timestamp
Assert.assertEquals(nextRunEST - nextRunGMT, fiveHoursInMilliseconds);
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using TimeUnit.HOURS.toMillis() instead

}

@Test
public void testUsesNewRequestDataFromPendingDeploy() {
initRequest();
Expand Down