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.

Sign up for GitHub

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

Jump to bottom

feature: only allow autoscaling in a direction once a full metric window has elapsed #125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+66 −28
Open

feature: only allow autoscaling in a direction once a full metric window has elapsed #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 48 additions & 28 deletions src/autoscaler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,40 +122,60 @@ export default class AutoscaleProcessor {
if (scaleMetrics && scaleMetrics.length > 0) {
// check if we should scale up the group
if (this.evalScaleConditionForAllPeriods(ctx, scaleMetrics, count, group, 'up')) {
desiredCount = desiredCount + group.scalingOptions.scaleUpQuantity;
if (desiredCount > group.scalingOptions.maxDesired) {
desiredCount = group.scalingOptions.maxDesired;
}
if (await this.instanceGroupManager.allowAutoscalingByDirection(group.name, 'up')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We want to be able to react fast especially to scaling up and this might alter this behaviour.

  • For example, let's say scaleUpPeriodsCount = 3, and scaleUpThreshold = 5. At time x, we might see the following metrics per period: 4, 3, 3. Which means we scale up.

  • We wait for gracePeriodTTLSec for the instances to come up and be operational and then we check again the new metrics window. But if the usage is high, the newly instances will be immediately used and the sliding window will become 3, 3, 2. Which means we should scale up again.

  • You change suggests we should not scale up then, but instead wait 2 more scalePeriod and only then take the decision. This might be too late.

If we do want to play with having the grace period larger, I would prefer to leave the choice of the grace period in the hands of the admin of the group. Maybe we should look into adding a configurable grace period per direction per group, and the admin can set it to the desired value, such as scaleUpPeriodsCount * group.scalingOptions.scalePeriod for up.

// only actually autoscale in this direction if we haven't done so recently within the grace period
desiredCount = desiredCount + group.scalingOptions.scaleUpQuantity;
if (desiredCount > group.scalingOptions.maxDesired) {
desiredCount = group.scalingOptions.maxDesired;
}

await this.audit.saveAutoScalerActionItem(group.name, {
timestamp: Date.now(),
actionType: 'increaseDesiredCount',
count: count,
oldDesiredCount: group.scalingOptions.desiredCount,
newDesiredCount: desiredCount,
scaleMetrics: scaleMetrics.slice(0, group.scalingOptions.scaleUpPeriodsCount),
});
await this.audit.saveAutoScalerActionItem(group.name, {
timestamp: Date.now(),
actionType: 'increaseDesiredCount',
count: count,
oldDesiredCount: group.scalingOptions.desiredCount,
newDesiredCount: desiredCount,
scaleMetrics: scaleMetrics.slice(0, group.scalingOptions.scaleUpPeriodsCount),
});

await this.updateDesiredCount(ctx, desiredCount, group);
await this.instanceGroupManager.setAutoScaleGracePeriod(group);
await this.updateDesiredCount(ctx, desiredCount, group);
await Promise.allSettled([
this.instanceGroupManager.setAutoScaleGracePeriod(group),
this.instanceGroupManager.setAutoScaleGracePeriodByDirection(group, 'up'),
]);
} else {
ctx.logger.info(
`[AutoScaler] Skipping autoscale to ${desiredCount} for group ${group.name} with ${count} instances, still in grace period`,
);
}
} else if (this.evalScaleConditionForAllPeriods(ctx, scaleMetrics, count, group, 'down')) {
// next check if we should scale down the group
desiredCount = group.scalingOptions.desiredCount - group.scalingOptions.scaleDownQuantity;
if (desiredCount < group.scalingOptions.minDesired) {
desiredCount = group.scalingOptions.minDesired;
}
if (await this.instanceGroupManager.allowAutoscalingByDirection(group.name, 'down')) {
// only actually autoscale in this direction if we haven't done so recently within the grace period
desiredCount = group.scalingOptions.desiredCount - group.scalingOptions.scaleDownQuantity;
if (desiredCount < group.scalingOptions.minDesired) {
desiredCount = group.scalingOptions.minDesired;
}

await this.audit.saveAutoScalerActionItem(group.name, {
timestamp: Date.now(),
actionType: 'decreaseDesiredCount',
count: count,
oldDesiredCount: group.scalingOptions.desiredCount,
newDesiredCount: desiredCount,
scaleMetrics: scaleMetrics.slice(0, group.scalingOptions.scaleDownPeriodsCount),
});
await this.audit.saveAutoScalerActionItem(group.name, {
timestamp: Date.now(),
actionType: 'decreaseDesiredCount',
count: count,
oldDesiredCount: group.scalingOptions.desiredCount,
newDesiredCount: desiredCount,
scaleMetrics: scaleMetrics.slice(0, group.scalingOptions.scaleDownPeriodsCount),
});

await this.updateDesiredCount(ctx, desiredCount, group);
await this.instanceGroupManager.setAutoScaleGracePeriod(group);
await this.updateDesiredCount(ctx, desiredCount, group);
await Promise.allSettled([
this.instanceGroupManager.setAutoScaleGracePeriod(group),
this.instanceGroupManager.setAutoScaleGracePeriodByDirection(group, 'down'),
]);
} else {
ctx.logger.info(
`[AutoScaler] Skipping autoscale to ${desiredCount} for group ${group.name} with ${count} instances, still in grace period`,
);
}
} else {
// otherwise neither action is needed
ctx.logger.info(
Expand Down
18 changes: 18 additions & 0 deletions src/instance_group.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,24 @@ export default class InstanceGroupManager {
return !(result !== null && result.length > 0);
}

// keeps the autoscaling activity from occuring if we have scaled in this direction within the grace period
async allowAutoscalingByDirection(group: string, direction: string): Promise<boolean> {
const result = await this.redisClient.get(`autoScaleGracePeriodByDirection:${group}:${direction}`);
return !(result !== null && result.length > 0);
}

// starts autoscaling grace period after autoscaling has occurred in this direction
async setAutoScaleGracePeriodByDirection(group: InstanceGroup, direction: string): Promise<boolean> {
// count of periods in metric window from group based on direction
const scalePeriods =
direction == 'down' ? group.scalingOptions.scaleDownPeriodsCount : group.scalingOptions.scaleUpPeriodsCount;

// grace period is the greater of group grace period or metric window size
const directionalTTL = Math.max(group.gracePeriodTTLSec, scalePeriods * group.scalingOptions.scalePeriod);

return this.setValue(`setAutoScaleGracePeriodByDirection:${group.name}:${direction}`, directionalTTL);
}

async setAutoScaleGracePeriod(group: InstanceGroup): Promise<boolean> {
return this.setValue(`autoScaleGracePeriod:${group.name}`, group.gracePeriodTTLSec);
}
Expand Down