Skip to content

Commit c4b55c3

Browse files
authored
Merge pull request #2246 from HubSpot/request-level-maxscale
Add request-level maximum scale option
2 parents 8e4a548 + ae2a35e commit c4b55c3

4 files changed

Lines changed: 104 additions & 1 deletion

File tree

SingularityBase/src/main/java/com/hubspot/singularity/SingularityRequest.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@
1414
import java.util.Set;
1515

1616
@Schema(
17-
description = "Settings that apply to all tasks and deploys assocaited with this request"
17+
description = "Settings that apply to all tasks and deploys associated with this request"
1818
)
1919
public class SingularityRequest {
2020
private final String id;
2121
private final RequestType requestType;
2222
private final Optional<List<String>> owners;
2323
private final Optional<Integer> numRetriesOnFailure;
24+
private final Optional<Integer> maxScale;
2425
private final Optional<String> schedule;
2526
private final Optional<String> quartzSchedule;
2627
private final Optional<ScheduleType> scheduleType;
@@ -61,6 +62,7 @@ public SingularityRequest(
6162
@JsonProperty("requestType") RequestType requestType,
6263
@JsonProperty("owners") Optional<List<String>> owners,
6364
@JsonProperty("numRetriesOnFailure") Optional<Integer> numRetriesOnFailure,
65+
@JsonProperty("maxScale") Optional<Integer> maxScale,
6466
@JsonProperty("schedule") Optional<String> schedule,
6567
@JsonProperty("instances") Optional<Integer> instances,
6668
@JsonProperty("rackSensitive") Optional<Boolean> rackSensitive,
@@ -129,6 +131,7 @@ public SingularityRequest(
129131
this.id = checkNotNull(id, "id cannot be null");
130132
this.owners = owners;
131133
this.numRetriesOnFailure = numRetriesOnFailure;
134+
this.maxScale = maxScale;
132135
this.schedule = schedule;
133136
this.rackSensitive = rackSensitive;
134137
this.instances = instances;
@@ -183,6 +186,7 @@ public SingularityRequestBuilder toBuilder() {
183186
.setLoadBalanced(loadBalanced)
184187
.setInstances(instances)
185188
.setNumRetriesOnFailure(numRetriesOnFailure)
189+
.setMaxScale(maxScale)
186190
.setOwners(copyOfList(owners))
187191
.setRackSensitive(rackSensitive)
188192
.setSchedule(schedule)
@@ -243,6 +247,10 @@ public Optional<Integer> getNumRetriesOnFailure() {
243247
return numRetriesOnFailure;
244248
}
245249

250+
public Optional<Integer> getMaxScale() {
251+
return maxScale;
252+
}
253+
246254
@Schema(nullable = true, description = "A schedule in cron, RFC5545, or quartz format")
247255
public Optional<String> getSchedule() {
248256
return schedule;
@@ -586,6 +594,7 @@ public boolean equals(Object o) {
586594
requestType == that.requestType &&
587595
Objects.equals(owners, that.owners) &&
588596
Objects.equals(numRetriesOnFailure, that.numRetriesOnFailure) &&
597+
Objects.equals(maxScale, that.maxScale) &&
589598
Objects.equals(schedule, that.schedule) &&
590599
Objects.equals(quartzSchedule, that.quartzSchedule) &&
591600
Objects.equals(scheduleType, that.scheduleType) &&
@@ -639,6 +648,7 @@ public int hashCode() {
639648
requestType,
640649
owners,
641650
numRetriesOnFailure,
651+
maxScale,
642652
schedule,
643653
quartzSchedule,
644654
scheduleType,
@@ -686,6 +696,8 @@ public String toString() {
686696
owners +
687697
", numRetriesOnFailure=" +
688698
numRetriesOnFailure +
699+
", maxScale=" +
700+
maxScale +
689701
", schedule=" +
690702
schedule +
691703
", quartzSchedule=" +

SingularityBase/src/main/java/com/hubspot/singularity/SingularityRequestBuilder.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ public class SingularityRequestBuilder {
1414

1515
private Optional<List<String>> owners;
1616
private Optional<Integer> numRetriesOnFailure;
17+
private Optional<Integer> maxScale;
1718

1819
private Optional<String> schedule;
1920
private Optional<String> quartzSchedule;
@@ -60,6 +61,7 @@ public SingularityRequestBuilder(String id, RequestType requestType) {
6061
this.requestType = checkNotNull(requestType, "requestType cannot be null");
6162
this.owners = Optional.empty();
6263
this.numRetriesOnFailure = Optional.empty();
64+
this.maxScale = Optional.empty();
6365
this.schedule = Optional.empty();
6466
this.scheduleType = Optional.empty();
6567
this.killOldNonLongRunningTasksAfterMillis = Optional.empty();
@@ -99,6 +101,7 @@ public SingularityRequest build() {
99101
requestType,
100102
owners,
101103
numRetriesOnFailure,
104+
maxScale,
102105
schedule,
103106
instances,
104107
rackSensitive,
@@ -187,6 +190,15 @@ public SingularityRequestBuilder setNumRetriesOnFailure(
187190
return this;
188191
}
189192

193+
public Optional<Integer> getMaxScale() {
194+
return maxScale;
195+
}
196+
197+
public SingularityRequestBuilder setMaxScale(Optional<Integer> maxScale) {
198+
this.maxScale = maxScale;
199+
return this;
200+
}
201+
190202
public Optional<String> getSchedule() {
191203
return schedule;
192204
}
@@ -535,6 +547,7 @@ public boolean equals(Object o) {
535547
requestType == that.requestType &&
536548
Objects.equals(owners, that.owners) &&
537549
Objects.equals(numRetriesOnFailure, that.numRetriesOnFailure) &&
550+
Objects.equals(maxScale, that.maxScale) &&
538551
Objects.equals(schedule, that.schedule) &&
539552
Objects.equals(quartzSchedule, that.quartzSchedule) &&
540553
Objects.equals(scheduleTimeZone, that.scheduleTimeZone) &&
@@ -588,6 +601,7 @@ public int hashCode() {
588601
requestType,
589602
owners,
590603
numRetriesOnFailure,
604+
maxScale,
591605
schedule,
592606
quartzSchedule,
593607
scheduleTimeZone,
@@ -635,6 +649,8 @@ public String toString() {
635649
owners +
636650
", numRetriesOnFailure=" +
637651
numRetriesOnFailure +
652+
", maxScale=" +
653+
maxScale +
638654
", schedule=" +
639655
schedule +
640656
", quartzSchedule=" +

SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ public class SingularityValidator {
119119
private final AgentManager agentManager;
120120
private final LoadBalancerClient loadBalancerClient;
121121

122+
private final SingularityConfiguration singularityConfiguration;
123+
122124
@Inject
123125
public SingularityValidator(
124126
SingularityConfiguration configuration,
@@ -187,6 +189,8 @@ public SingularityValidator(
187189
this.disasterManager = disasterManager;
188190
this.agentManager = agentManager;
189191
this.loadBalancerClient = loadBalancerClient;
192+
193+
this.singularityConfiguration = configuration;
190194
}
191195

192196
public SingularityRequest checkSingularityRequest(
@@ -230,13 +234,23 @@ public SingularityRequest checkSingularityRequest(
230234
!request.getInstances().isPresent() || request.getInstances().get() > 0,
231235
"Instances must be greater than 0"
232236
);
237+
233238
checkBadRequest(
234239
request.getInstancesSafe() <= maxInstancesPerRequest,
235240
"Instances (%s) be greater than %s (maxInstancesPerRequest in mesos configuration)",
236241
request.getInstancesSafe(),
237242
maxInstancesPerRequest
238243
);
239244

245+
if (request.getRequestType().isLongRunning() && request.getMaxScale().isPresent()) {
246+
checkBadRequest(
247+
request.getInstancesSafe() <= request.getMaxScale().get(),
248+
"Instances (%s) cannot be greater than %s (maxScale in request)",
249+
request.getInstancesSafe(),
250+
request.getMaxScale().get()
251+
);
252+
}
253+
240254
if (request.getTaskPriorityLevel().isPresent()) {
241255
checkBadRequest(
242256
request.getTaskPriorityLevel().get() >= 0 &&
@@ -1200,6 +1214,15 @@ public void checkScale(SingularityRequest request, Optional<Integer> previousSca
12001214
requiredAgentCount,
12011215
currentActiveAgentCount
12021216
);
1217+
1218+
if (request.getRequestType().isLongRunning() && request.getMaxScale().isPresent()) {
1219+
checkBadRequest(
1220+
request.getInstancesSafe() <= request.getMaxScale().get(),
1221+
"Instances (%s) cannot be greater than %s (maxScale in request)",
1222+
request.getInstancesSafe(),
1223+
request.getMaxScale().get()
1224+
);
1225+
}
12031226
}
12041227
}
12051228

SingularityService/src/test/java/com/hubspot/singularity/data/ValidatorTest.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,58 @@ public void itForbidsTooLongDeployId() {
161161
);
162162
}
163163

164+
@Test
165+
public void itForbidsInstancesGreaterThanRequestMaxScale() {
166+
int globalMaxScale = configuration
167+
.getMesosConfiguration()
168+
.getMaxNumInstancesPerRequest();
169+
int requestMaxScale = globalMaxScale - 5;
170+
SingularityRequest request = new SingularityRequestBuilder(
171+
"requestId",
172+
RequestType.SERVICE
173+
)
174+
.setMaxScale(Optional.of(requestMaxScale)) // request level max scale < global max scale
175+
.setInstances(Optional.of(requestMaxScale + 1)) // instances > request level max scale
176+
.build();
177+
178+
Assertions.assertThrows(
179+
WebApplicationException.class,
180+
() ->
181+
validator.checkSingularityRequest(
182+
request,
183+
Optional.empty(),
184+
Optional.empty(),
185+
Optional.empty()
186+
)
187+
);
188+
}
189+
190+
@Test
191+
public void itForbidsInstancesGreaterThanGlobalMaxScale() {
192+
int globalMaxScale = configuration
193+
.getMesosConfiguration()
194+
.getMaxNumInstancesPerRequest();
195+
int requestMaxScale = globalMaxScale + 5;
196+
SingularityRequest request = new SingularityRequestBuilder(
197+
"requestId",
198+
RequestType.SERVICE
199+
)
200+
.setMaxScale(Optional.of(requestMaxScale)) // global max scale < request level max scale
201+
.setInstances(Optional.of(globalMaxScale + 1)) // instances > global max scale (mesos config)
202+
.build();
203+
204+
Assertions.assertThrows(
205+
WebApplicationException.class,
206+
() ->
207+
validator.checkSingularityRequest(
208+
request,
209+
Optional.empty(),
210+
Optional.empty(),
211+
Optional.empty()
212+
)
213+
);
214+
}
215+
164216
@Test
165217
public void itForbidsRunNowOfScheduledWhenAlreadyRunning() {
166218
String deployID = "deploy";

0 commit comments

Comments
 (0)