Execute autoscaler synchronously on scheduler thread #158
Conversation
corindwyer
commented
Aug 12, 2017
corindwyer
commented
- change autoscaler to be synchronously executed as part of the scheduling iteration while keeping the callback execution on a separate thread
- change shortfall analysis implementation to be synchronously executed as part of the autoscaler
- add withAutoscaleDisabledVmDurationInSecs for users that want to disable VMs for a fixed duration that is different from the cooldown value
…ing iteration while keeping the callback execution on a separate thread change shortfall analysis implementation to be synchronously executed as part of the autoscaler add withAutoscaleDisabledVmDurationInSecs for users that want to disable VMs for a fixed duration that is different from the cooldown value
| } | ||
| } | ||
| return () -> { | ||
| if (!lock.tryLock()) |
There was a problem hiding this comment.
Yes, we need to release the lock here. Still need to confirm if this class is responsible for preventing multithreaded calls to the scheduling loop or if it is even supposed to protect the main thread from making multiple calls.
| * @see <a href="https://github.com/Netflix/Fenzo/wiki/Autoscaling">Autoscaling</a> | ||
| */ | ||
| public Builder withAutoscaleDisabledVmDurationInSecs(long disabledVmDurationInSecs) { | ||
| if(disabledVmDurationInSecs > 0L) |
| if (values != null && values.length == 2) | ||
| ports = Math.max(ports, values[0] + values[1]); | ||
| for (AssignableVirtualMachine avm : avms) { | ||
| Map<VMResource, Double> maxResources = avm.getMaxResources(); |
There was a problem hiding this comment.
Need to confirm if maxResources is set already by the time this is called
There was a problem hiding this comment.
we confirmed this is set because the pseudo scheduling happens after the main scheduling.
| StateMonitor() { | ||
| lock = new AtomicBoolean(false); | ||
| } | ||
| private final ReentrantLock lock = new ReentrantLock(); |
There was a problem hiding this comment.
Why is this needed to be reentrant?
| @@ -466,6 +484,9 @@ private TaskScheduler(Builder builder) { | |||
| autoScaler.setDelayScaleDownBySecs(builder.delayAutoscaleDownBySecs); | |||
| if(builder.delayAutoscaleUpBySecs > 0L) | |||
| autoScaler.setDelayScaleUpBySecs(builder.delayAutoscaleUpBySecs); | |||
| if (builder.disabledVmDurationInSecs > 0L) { | |||
| autoScaler.setDisabledVmDurationInSecs(builder.disabledVmDurationInSecs); | |||
There was a problem hiding this comment.
There is a risk in this being separate from cooldown secs. For example, the VM maybe enabled too soon and start getting assignments before it is terminated.
There was a problem hiding this comment.
added javadoc to outline that this value should be greater than the cooldown period and is only useful if the autoscaling action may take more time to execute than your cooldown period.
revert StateMonitor to the previous implementation create a new pseudo scheduling method that ignores the lock
