-
Notifications
You must be signed in to change notification settings - Fork 224
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
Create condition based rebalancer #2846
Create condition based rebalancer #2846
Conversation
...-core/src/main/java/org/apache/helix/controller/rebalancer/condition/RebalanceCondition.java
Outdated
Show resolved
Hide resolved
helix-core/src/main/java/org/apache/helix/controller/rebalancer/ConditionBasedRebalancer.java
Outdated
Show resolved
Hide resolved
helix-core/src/main/java/org/apache/helix/controller/rebalancer/ConditionBasedRebalancer.java
Outdated
Show resolved
Hide resolved
helix-core/src/main/java/org/apache/helix/controller/rebalancer/ConditionBasedRebalancer.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.
Need the test add for this PR.
In addition, thinking about where to put the configuration for the rebalancer. My haunch is in IdealState simple fields.
package org.apache.helix.controller.rebalancer.condition; | ||
|
||
public interface RebalanceCondition { | ||
boolean shouldPerformRebalance(); |
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 think we need a generic string -> string map for input ? Or directly pass the cache object.
Because condition is a piece of code logic but the real computation is happening at runtime with input.
...rc/main/java/org/apache/helix/controller/rebalancer/condition/RebalanceConditionBuilder.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.
lgtm
This PR is ready to be merged. Approved by @junkaixue |
Create condition based rebalancer
Create condition based rebalancer
Create condition based rebalancer
Create condition based rebalancer
Issues
resolves #2822
Description
This PR initiates the condition based rebalancer which can perform restricted shard shuffling based on conditions. Will add more tests if the overall skeleton makes sense.
Tests
The following tests are written for this issue:
N/A
The following is the result of the "mvn test" command on the appropriate module:
(If CI test fails due to known issue, please specify the issue and test PR locally. Then copy & paste the result of "mvn test" to here.)
Changes that Break Backward Compatibility (Optional)
(Consider including all behavior changes for public methods or API. Also include these changes in merge description so that other developers are aware of these changes. This allows them to make relevant code changes in feature branches accounting for the new method/API behavior.)
Documentation (Optional)
(Link the GitHub wiki you added)
Commits
Code Quality
(helix-style-intellij.xml if IntelliJ IDE is used)