Skip to content
This repository was archived by the owner on Mar 31, 2023. It is now read-only.

Commit f48e471

Browse files
author
tbak
authored
Handle inactive agent termination cycle independently from the active one (#155)
* Handle inactive agent termination cycle independently from the active one Sharing the same timestamp between the inactive and active server groups, introduces undesired scale-time dependencies between the two. * Code review updates for "Handle inactive agent termination cycle independently from the active one" * More updates to "Handle inactive agent termination cycle independently from the active one"
1 parent 29f6eff commit f48e471

File tree

1 file changed

+67
-38
lines changed

1 file changed

+67
-38
lines changed

‎fenzo-core/src/main/java/com/netflix/fenzo/AutoScaler.java‎

Lines changed: 67 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ private static class ScalingActivity {
6464
private long scaleUpRequestedAt;
6565
private long scaleDownAt;
6666
private long scaleDownRequestedAt;
67+
private long inactiveScaleDownAt;
68+
private long inactiveScaleDownRequestedAt;
6769
private int shortfall;
6870
private int scaledNumInstances;
6971
private AutoScaleAction.Type type;
@@ -197,6 +199,10 @@ private boolean shouldScaleDown(long now, ScalingActivity prevScalingActivity, A
197199
return shouldScaleNow(false, now, prevScalingActivity, rule);
198200
}
199201

202+
private boolean shouldScaleDownInactive(long now, ScalingActivity prevScalingActivity, AutoScaleRule rule) {
203+
return now > (Math.max(activeVmGroups.getLastSetAt(), prevScalingActivity.inactiveScaleDownAt) + rule.getCoolDownSecs() * 1000);
204+
}
205+
200206
private void processScalingNeeds(HostAttributeGroup hostAttributeGroup, ConcurrentMap<String, ScalingActivity> scalingActivityMap, AssignableVMs assignableVMs) {
201207
AutoScaleRule rule = hostAttributeGroup.rule;
202208
long now = System.currentTimeMillis();
@@ -205,7 +211,28 @@ private void processScalingNeeds(HostAttributeGroup hostAttributeGroup, Concurre
205211
int excess = hostAttributeGroup.shortFall>0? 0 : hostAttributeGroup.idleHosts.size() - rule.getMaxIdleHostsToKeep();
206212
int inactiveIdleCount = hostAttributeGroup.idleInactiveHosts.size();
207213

208-
if ((inactiveIdleCount > 0 || excess > 0) && shouldScaleDown(now, prevScalingActivity, rule)) {
214+
List<String> allHostsToTerminate = new ArrayList<>();
215+
if(inactiveIdleCount > 0 && shouldScaleDownInactive(now, prevScalingActivity, rule)) {
216+
ScalingActivity scalingActivity = scalingActivityMap.get(rule.getRuleName());
217+
long lastReqstAge = (now - scalingActivity.inactiveScaleDownRequestedAt) / 1000L;
218+
if(delayScaleDownBySecs>0L && lastReqstAge > 2 * delayScaleDownBySecs) { // reset the request at time
219+
scalingActivity.inactiveScaleDownRequestedAt = now;
220+
}
221+
else if(delayScaleDownBySecs == 0L || lastReqstAge > delayScaleDownBySecs) {
222+
scalingActivity.inactiveScaleDownRequestedAt = 0L;
223+
scalingActivity.inactiveScaleDownAt = now;
224+
Map<String, String> hostsToTerminate = getInactiveHostsToTerminate(hostAttributeGroup.idleInactiveHosts);
225+
StringBuilder sBuilder = new StringBuilder();
226+
for (String host : hostsToTerminate.keySet()) {
227+
sBuilder.append(host).append(", ");
228+
}
229+
logger.info("Scaling down inactive hosts " + rule.getRuleName() + " by "
230+
+ hostsToTerminate.size() + " hosts (" + sBuilder.toString() + ")");
231+
allHostsToTerminate.addAll(hostsToTerminate.values());
232+
}
233+
}
234+
235+
if (excess > 0 && shouldScaleDown(now, prevScalingActivity, rule)) {
209236
ScalingActivity scalingActivity = scalingActivityMap.get(rule.getRuleName());
210237
long lastReqstAge = (now - scalingActivity.scaleDownRequestedAt) / 1000L;
211238
if(delayScaleDownBySecs>0L && lastReqstAge > 2 * delayScaleDownBySecs) { // reset the request at time
@@ -215,11 +242,11 @@ else if(delayScaleDownBySecs == 0L || lastReqstAge > delayScaleDownBySecs) {
215242
final int size = vmCollection.size(rule.getRuleName());
216243
if (rule.getMinSize() > (size - excess))
217244
excess = Math.max(0, size - rule.getMinSize());
218-
if (excess > 0 || inactiveIdleCount > 0) {
245+
if (excess > 0) {
219246
scalingActivity.scaleDownRequestedAt = 0L;
220247
scalingActivity.scaleDownAt = now;
221248
scalingActivity.shortfall = hostAttributeGroup.shortFall;
222-
Map<String, String> hostsToTerminate = getHostsToTerminate(hostAttributeGroup.idleHosts, hostAttributeGroup.idleInactiveHosts, excess);
249+
Map<String, String> hostsToTerminate = getHostsToTerminate(hostAttributeGroup.idleHosts, excess);
223250
scalingActivity.scaledNumInstances = hostsToTerminate.size();
224251
scalingActivity.type = AutoScaleAction.Type.Down;
225252
StringBuilder sBuilder = new StringBuilder();
@@ -229,14 +256,10 @@ else if(delayScaleDownBySecs == 0L || lastReqstAge > delayScaleDownBySecs) {
229256
}
230257
logger.info("Scaling down " + rule.getRuleName() + " by "
231258
+ hostsToTerminate.size() + " hosts (" + sBuilder.toString() + ")");
232-
callback.call(
233-
new ScaleDownAction(rule.getRuleName(), hostsToTerminate.values())
234-
);
259+
allHostsToTerminate.addAll(hostsToTerminate.values());
235260
}
236261
}
237-
}
238-
239-
if(hostAttributeGroup.shortFall>0 || (excess<=0 && shouldScaleUp(now, prevScalingActivity, rule))) {
262+
} else if(hostAttributeGroup.shortFall>0 || (excess<=0 && shouldScaleUp(now, prevScalingActivity, rule))) {
240263
if (hostAttributeGroup.shortFall>0 || rule.getMinIdleHostsToKeep() > hostAttributeGroup.idleHosts.size()) {
241264
// scale up to rule.getMaxIdleHostsToKeep() instead of just until rule.getMinIdleHostsToKeep()
242265
// but, if not shouldScaleUp(), then, scale up due to shortfall
@@ -261,13 +284,15 @@ else if(delayScaleUpBySecs ==0L || lastReqstAge > delayScaleUpBySecs) {
261284
scalingActivity.type = AutoScaleAction.Type.Up;
262285
logger.info("Scaling up " + rule.getRuleName() + " by "
263286
+ shortage + " hosts");
264-
callback.call(
265-
new ScaleUpAction(rule.getRuleName(), shortage)
266-
);
287+
callback.call(new ScaleUpAction(rule.getRuleName(), shortage));
267288
}
268289
}
269290
}
270291
}
292+
// Run scale-down action after scale up (if any)
293+
if(!allHostsToTerminate.isEmpty()) {
294+
callback.call(new ScaleDownAction(rule.getRuleName(), allHostsToTerminate));
295+
}
271296
}
272297

273298
private void populateIdleResources(List<VirtualMachineLease> idleResources,
@@ -318,42 +343,46 @@ private long getInitialCoolDown(long coolDownSecs) {
318343
return System.currentTimeMillis()- coolDownSecs*1000 + initialCoolDownInPastSecs*1000;
319344
}
320345

321-
private Map<String, String> getHostsToTerminate(List<VirtualMachineLease> activeHosts, List<VirtualMachineLease> inactiveHosts, int excess) {
346+
private Map<String, String> getHostsToTerminate(List<VirtualMachineLease> idleHosts, int excess) {
322347
if(scaleDownConstraintExecutor == null) {
323-
return activeHosts.isEmpty() ? Collections.emptyMap() : getHostsToTerminateLegacy(activeHosts, excess);
348+
return idleHosts.isEmpty() ? Collections.emptyMap() : getHostsToTerminateLegacy(idleHosts, excess);
324349
} else {
325-
return getHostsToTerminateUsingCriteria(activeHosts, inactiveHosts, excess);
350+
return getHostsToTerminateUsingCriteria(idleHosts, excess);
326351
}
327352
}
328353

329-
private Map<String, String> getHostsToTerminateUsingCriteria(List<VirtualMachineLease> activeHosts,List<VirtualMachineLease> inactiveHosts, int excess) {
354+
private Map<String, String> getInactiveHostsToTerminate(List<VirtualMachineLease> inactiveIdleHosts) {
355+
if(scaleDownConstraintExecutor == null) {
356+
return Collections.emptyMap();
357+
} else {
358+
Map<String, String> hostsMap = new HashMap<>();
359+
List<VirtualMachineLease> result = scaleDownConstraintExecutor.evaluate(inactiveIdleHosts);
360+
result.forEach(lease -> hostsMap.put(lease.hostname(), getMappedHostname(lease)));
361+
return hostsMap;
362+
}
363+
}
364+
365+
private Map<String, String> getHostsToTerminateUsingCriteria(List<VirtualMachineLease> idleHosts, int excess) {
330366
Map<String, String> hostsMap = new HashMap<>();
331367

332-
if(excess <= 0) {
333-
// We have only inactive idle instances to scale down
334-
List<VirtualMachineLease> result = scaleDownConstraintExecutor.evaluate(inactiveHosts);
335-
result.forEach(lease -> hostsMap.put(lease.hostname(), getMappedHostname(lease)));
336-
} else {
337-
List<VirtualMachineLease> allIdle = new ArrayList<>(activeHosts);
338-
allIdle.addAll(inactiveHosts);
339-
List<VirtualMachineLease> result = scaleDownConstraintExecutor.evaluate(allIdle);
340-
341-
// The final result should contain only excess number of active hosts. Enforce this invariant.
342-
Set<String> activeIds = activeHosts.stream().map(VirtualMachineLease::hostname).collect(Collectors.toSet());
343-
int activeCounter = 0;
344-
Iterator<VirtualMachineLease> it = result.iterator();
345-
while(it.hasNext()) {
346-
VirtualMachineLease leaseToRemove = it.next();
347-
if(activeIds.contains(leaseToRemove.hostname())) {
348-
if(activeCounter < excess) {
349-
activeCounter++;
350-
} else {
351-
it.remove();
352-
}
368+
List<VirtualMachineLease> allIdle = new ArrayList<>(idleHosts);
369+
List<VirtualMachineLease> result = scaleDownConstraintExecutor.evaluate(allIdle);
370+
371+
// The final result should contain only excess number of active hosts. Enforce this invariant.
372+
Set<String> activeIds = idleHosts.stream().map(VirtualMachineLease::hostname).collect(Collectors.toSet());
373+
int activeCounter = 0;
374+
Iterator<VirtualMachineLease> it = result.iterator();
375+
while(it.hasNext()) {
376+
VirtualMachineLease leaseToRemove = it.next();
377+
if(activeIds.contains(leaseToRemove.hostname())) {
378+
if(activeCounter < excess) {
379+
activeCounter++;
380+
} else {
381+
it.remove();
353382
}
354383
}
355-
result.forEach(lease -> hostsMap.put(lease.hostname(), getMappedHostname(lease)));
356384
}
385+
result.forEach(lease -> hostsMap.put(lease.hostname(), getMappedHostname(lease)));
357386

358387
return hostsMap;
359388
}

0 commit comments

Comments
 (0)