-
Notifications
You must be signed in to change notification settings - Fork 575
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
Setup ProvisionerState for rightsizing clusters #1646
Setup ProvisionerState for rightsizing clusters #1646
Conversation
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.
@kaitlynp1206 Thanks for the PR!
Left some comments.
cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/ProvisionerState.java
Outdated
Show resolved
Hide resolved
...in/java/com/linkedin/kafka/cruisecontrol/config/constants/CruiseControlParametersConfig.java
Outdated
Show resolved
Hide resolved
.../main/java/com/linkedin/kafka/cruisecontrol/config/constants/CruiseControlRequestConfig.java
Outdated
Show resolved
Hide resolved
cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/Provisioner.java
Outdated
Show resolved
Hide resolved
cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/ProvisionerState.java
Outdated
Show resolved
Hide resolved
...ontrol/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/parameters/ParameterUtils.java
Outdated
Show resolved
Hide resolved
...control/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/response/RightsizeResult.java
Outdated
Show resolved
Hide resolved
...control/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/response/RightsizeResult.java
Outdated
Show resolved
Hide resolved
...control/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/response/RightsizeResult.java
Outdated
Show resolved
Hide resolved
...se-control/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/CruiseControlEndPoint.java
Show resolved
Hide resolved
70e6364
to
d0a4836
Compare
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 updates! Left more comments.
-- Also please note that there are test failures (I'd recommend making sure all tests pass before submitting your updates -- i.e. a submitted PR should have already been tested for existing and new unit tests. (please see https://github.com/linkedin/cruise-control/blob/master/CONTRIBUTING.md)):
com.linkedin.kafka.cruisecontrol.servlet.parameters.RequestParameterTest > checkOpenApiSpec FAILED
java.lang.IllegalArgumentException: Schema Parser does not support HTTP methods other than GET/POST
at com.linkedin.kafka.cruisecontrol.servlet.parameters.RequestParameterTest.parseEndpoint(RequestParameterTest.java:82)
at com.linkedin.kafka.cruisecontrol.servlet.parameters.RequestParameterTest.parseSchema(RequestParameterTest.java:101)
at com.linkedin.kafka.cruisecontrol.servlet.parameters.RequestParameterTest.checkOpenApiSpec(RequestParameterTest.java:59)
com.linkedin.kafka.cruisecontrol.servlet.response.ResponseTest > checkOpenAPISpec FAILED
java.lang.NullPointerException
at com.linkedin.kafka.cruisecontrol.servlet.response.ResponseTest.checkSchema(ResponseTest.java:139)
at com.linkedin.kafka.cruisecontrol.servlet.response.ResponseTest.lambda$checkSchema$2(ResponseTest.java:180)
at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:684)
at com.linkedin.kafka.cruisecontrol.servlet.response.ResponseTest.checkSchema(ResponseTest.java:156)
at com.linkedin.kafka.cruisecontrol.servlet.response.ResponseTest.lambda$checkOperation$0(ResponseTest.java:111)
at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:684)
at com.linkedin.kafka.cruisecontrol.servlet.response.ResponseTest.lambda$checkOperation$1(ResponseTest.java:107)
at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:684)
at com.linkedin.kafka.cruisecontrol.servlet.response.ResponseTest.checkOperation(ResponseTest.java:106)
at com.linkedin.kafka.cruisecontrol.servlet.response.ResponseTest.checkOpenAPISpec(ResponseTest.java:95)
...e-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/GoalViolationDetector.java
Outdated
Show resolved
Hide resolved
cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/Provisioner.java
Outdated
Show resolved
Hide resolved
cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/ProvisionerState.java
Outdated
Show resolved
Hide resolved
cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/ProvisionerState.java
Outdated
Show resolved
Hide resolved
cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/ProvisionerState.java
Outdated
Show resolved
Hide resolved
...se-control/src/test/java/com/linkedin/kafka/cruisecontrol/detector/ProvisionerStateTest.java
Outdated
Show resolved
Hide resolved
...se-control/src/test/java/com/linkedin/kafka/cruisecontrol/detector/ProvisionerStateTest.java
Show resolved
Hide resolved
Co-authored-by: Kaitlyn Paglia <kpaglia@linkedin.com>
d0a4836
to
b382625
Compare
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 updates!
Left more comments.
...ontrol/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/parameters/ParameterUtils.java
Outdated
Show resolved
Hide resolved
b382625
to
30da017
Compare
30da017
to
8230a05
Compare
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 updates -- left more comments
...control/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/response/RightsizeResult.java
Show resolved
Hide resolved
...control/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/response/RightsizeResult.java
Outdated
Show resolved
Hide resolved
...ol/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/handler/sync/RightsizeRequest.java
Outdated
Show resolved
Hide resolved
...ol/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/handler/sync/RightsizeRequest.java
Outdated
Show resolved
Hide resolved
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 updates -- left a few more comments.
...ol/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/handler/sync/RightsizeRequest.java
Outdated
Show resolved
Hide resolved
...ol/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/handler/sync/RightsizeRequest.java
Outdated
Show resolved
Hide resolved
...ol/src/main/java/com/linkedin/kafka/cruisecontrol/servlet/handler/sync/RightsizeRequest.java
Show resolved
Hide resolved
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.
@kaitlynp1206
Thanks for your contribution, which brings Cruise Control a step closer to achieve its vision of fully automated execution of the necessary actions to satisfy SLO requirements while saving time (instantaneous execution), effort (no human intervention), and cost (hardware, VM, and FTEs).
-- LGTM.
This PR resolves #1645.
Created a provisioner state object to track changes for rightsizing a cluster.
Added a test endpoint
Rightsize
to perform integration testing on the provisioner.