-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Introduce autoscaling decisions #53934
Conversation
This is the first in a series of commits that will introduce the autoscaling deciders framework. This commit introduces the basic framework for representing autoscaling decisions.
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.
LGTM.
|
||
} | ||
|
||
public static class MultipleAutoscalingDecision extends AutoscalingDecision { |
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.
I would prefer to not extend AutoscalingDecision
. This could let AutoscalingDecision
be a final class and we avoid throwing UnsupportedOperationException
below (and remove decisions()
from single decision).
Autoscaling deciders should not really have to choose between using a single or multi decision. The multi-decision is only for the "framework". MultipleAutoscalingDecision
could be an inner class to AutoscalingService
or co-located with the service.
I know this is contrary to lots of other code, so please consider this optional.
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.
Thanks for the input. I have made a refactoring here. Would you let me know your thoughts?
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.
Thanks for doing the refactor, this looks great.
...g/src/main/java/org/elasticsearch/xpack/autoscaling/action/GetAutoscalingDecisionAction.java
Show resolved
Hide resolved
.../autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/AutoscalingDecisionTests.java
Outdated
Show resolved
Hide resolved
.../autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/AutoscalingDecisionTests.java
Outdated
Show resolved
Hide resolved
…k/autoscaling/AutoscalingDecisionTests.java Co-Authored-By: Henning Andersen <33268011+henningandersen@users.noreply.github.com>
…k/autoscaling/AutoscalingDecisionTests.java Co-Authored-By: Henning Andersen <33268011+henningandersen@users.noreply.github.com>
@elasticmachine update branch |
This is the first in a series of commits that will introduce the autoscaling deciders framework. This commit introduces the basic framework for representing autoscaling decisions.
This is the first in a series of commits that will introduce the autoscaling deciders framework. This commit introduces the basic framework for representing autoscaling decisions.