dasahcc commented on a change in pull request #1061:
URL: https://github.com/apache/helix/pull/1061#discussion_r437624616
##########
File path:
helix-core/src/main/java/org/apache/helix/task/GenericTaskAssignmentCalculator.java
##########
@@ -65,9 +65,9 @@
@Override
public Map<String, SortedSet<Integer>> getTaskAssignment(CurrentStateOutput
currStateOutput,
- ResourceAssignment prevAssignment, Collection<String> instances,
JobConfig jobCfg,
- final JobContext jobContext, WorkflowConfig workflowCfg, WorkflowContext
workflowCtx,
- Set<Integer> partitionSet, Map<String, IdealState> idealStateMap) {
+ Collection<String> instances, JobConfig jobCfg, final JobContext
jobContext,
Review comment:
Better not change public API, we can deprecate it and add a new one.
##########
File path:
helix-core/src/main/java/org/apache/helix/task/FixedTargetTaskAssignmentCalculator.java
##########
@@ -78,11 +78,11 @@ public
FixedTargetTaskAssignmentCalculator(AssignableInstanceManager assignableI
@Override
public Map<String, SortedSet<Integer>> getTaskAssignment(CurrentStateOutput
currStateOutput,
Review comment:
So it may not be safe to change public API directly. My suggestion is to
deprecate the method and create a new one if necessary.
This deprecated function can call the new function.
##########
File path:
helix-core/src/main/java/org/apache/helix/task/DeprecatedTaskRebalancer.java
##########
@@ -110,10 +109,9 @@
* @return map of instances to set of partition numbers
*/
Review comment:
I dont think any one is using this class. Shall we remove it? Or at
least, let's deprecate the class instead of changing the public API for
backward incompatible change.
##########
File path:
helix-core/src/main/java/org/apache/helix/task/FixedTargetTaskRebalancer.java
##########
@@ -48,13 +48,11 @@
}
Review comment:
Same here. Shall we remove this class?
##########
File path:
helix-core/src/main/java/org/apache/helix/task/GenericTaskRebalancer.java
##########
@@ -40,18 +40,18 @@
@Override
public Set<Integer> getAllTaskPartitions(JobConfig jobCfg, JobContext jobCtx,
- WorkflowConfig workflowCfg, WorkflowContext workflowCtx,
WorkflowControllerDataProvider cache) {
- return taskAssignmentCalculator
- .getAllTaskPartitions(jobCfg, jobCtx, workflowCfg, workflowCtx,
cache.getIdealStates());
+ WorkflowConfig workflowCfg, WorkflowContext workflowCtx,
Review comment:
Same here. Shall we remove the rebalancer related class. Or, at least,
deprecate it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
