-
Notifications
You must be signed in to change notification settings - Fork 30
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
Scale down threshold #187
Scale down threshold #187
Conversation
lib/template.js
Outdated
@@ -403,7 +403,7 @@ module.exports = (options = {}) => { | |||
StepAdjustments: [ | |||
{ | |||
ScalingAdjustment: -100, | |||
MetricIntervalLowerBound: 0.0 | |||
MetricIntervalLowerBound: -1.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think this should be a MetricIntervalUpperBound
of 0
.
The metric is ApproximateNumberOfMessagesNotVisible
. When the metric value is < 1, the alarm triggers, and invokes the scaling policy. The breach value should be -1 at that point (since the metric value will be 0
). Any breach value less than zero is supposed to trigger this step adjustment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* change scale-down MetricIntervalLowerBound to MetricIntervalUpperBound
* ♻ that container * Add logging (#185) * adds logging of watcher-level errors, worker receives, and completion status * prefixed logs from child processes * fixes logger factory to accept a message * --> false for legibility * move binary split to dependency * package lock changes * Scale down threshold (#187) * change scale-down MetricIntervalLowerBound to MetricIntervalUpperBound * exit main loop after workers finish * resolve() after all workers return instead of exiting * fix tests and mocks * cleanup * logs * another log * use logger and process.stdout for logs * more logs * edit logs * Add alarms to "♻️ that container" PR (#198) * Add alarms and alarm docs * Add failedPlacementAlarmPeriods * Add CloudWatch Alarms snapshots * Update template jest snapshots * Add CloudWatch Alarms snapshots * Add failedworker and failedworkerplacement metric * Typo r/LogGroup/Logs * Change metric name * Metric Filter of worker errors to "[failure]" * Have current published version instead of undefined * Jake's Review * uh update-jest * Update alarms.md * Add template validation tests (#215) * Add travis user * Ensure this fails * Add validation for notificationEmail or notificationTopic * Add minSize and maxSize of service scaleup and scaledown, deadletter queue threshold, info to doc (#211) * Closes #208, #207, #206, #182, #149, #72, #15 (cherry picked from commit 8de328df79ccf52b8d612c625891555808c2fa0e) * Add minSize as option * update jest tests * Change MinSize to 0 * update jest * identation and minSize to 0 * Add deadletterThreshold info in Worker-retry-cycle * Update tests with maxSize property * remove superfluous logging * add fresh mode as a watchbot option * if else * freshMode * console log * typeof * true * concise * add fresh * fix tests * fix binary test * update snapshots * Allow users to write to any volume (#200) * Restrict writes to volumes and clean them after every job * Try out the `ReadOnlyRootFilesystem` option * Capitalization * Add watchbot-log * use strict * No need to chmod now
Was getting an error from the scale down alarm
"error": "No step adjustment found for metric value [0.0] and breach threshold 1.0",
I think this is due to the lower bound being set at 0.0. Reading step adjustment docs, a threshold of 1 and lowerbound of 0.0 is effectively only a policy for thresholds at
1.0
or above. This pr adjusts the LowerBound to include0.0
.We could do this a billion ways (I think a MetricIntervalUpperBound of 0.0 would also work); I'm pretty agnostic on the cleanest way to define the step adjustment.