----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16071/#review29922 -----------------------------------------------------------
Ship it! Please fix what ever you can and file jira for big changes/future enhancements helix-core/src/main/java/org/apache/helix/ZNRecord.java <https://reviews.apache.org/r/16071/#comment57392> are we changing the definition of how subtract works ? helix-core/src/main/java/org/apache/helix/api/accessor/ResourceAccessor.java <https://reviews.apache.org/r/16071/#comment57393> why is this static and made public? helix-core/src/main/java/org/apache/helix/controller/stages/MessageSelectionStage.java <https://reviews.apache.org/r/16071/#comment57394> why do we allow adding resource without having statemodel? If this is the case where config exists but idealstate does not then we should not be processing that resource in the pipeline helix-core/src/main/java/org/apache/helix/controller/stages/ResourceCurrentState.java <https://reviews.apache.org/r/16071/#comment57395> what is the String, should that be an Info class ? helix-core/src/main/java/org/apache/helix/controller/stages/ResourceCurrentState.java <https://reviews.apache.org/r/16071/#comment57396> Can we have an Info class ? helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java <https://reviews.apache.org/r/16071/#comment57397> why are we resetting the info? How do we know that the controller has already processed that requested state? think of this scenario controllers sends msg 1 but before processing msg1, participant sets requested state, participant now reads msg1 and resets the requested state. Which means controller never saw the requested state. Is my understanding correct? helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java <https://reviews.apache.org/r/16071/#comment57398> should this be done always or we need a check that taskresult.getInfo is not null ? helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java <https://reviews.apache.org/r/16071/#comment57399> what does instanceof void mean helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java <https://reviews.apache.org/r/16071/#comment57400> Shouldn't keyBuilder have changed to accept typed objects helix-core/src/main/java/org/apache/helix/model/CurrentState.java <https://reviews.apache.org/r/16071/#comment57406> Having info class is better no ? helix-core/src/main/java/org/apache/helix/task/TaskConfig.java <https://reviews.apache.org/r/16071/#comment57407> Shouldn't taskconfig and taskcontext be extending rebalancerconfig and rebalancercontext ? helix-core/src/main/java/org/apache/helix/task/TaskDag.java <https://reviews.apache.org/r/16071/#comment57408> Nice - Kishore Gopalakrishna On Dec. 6, 2013, 7:34 p.m., Zhen Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16071/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2013, 7:34 p.m.) > > > Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna. > > > Repository: helix-git > > > Description > ------- > > [HELIX-336] Add support for task framework > > > Diffs > ----- > > helix-core/src/main/java/org/apache/helix/ZNRecord.java 37cd5eb > > helix-core/src/main/java/org/apache/helix/api/accessor/ResourceAccessor.java > b308b98 > > helix-core/src/main/java/org/apache/helix/controller/stages/CurrentStateComputationStage.java > c036b14 > > helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java > 7704378 > > helix-core/src/main/java/org/apache/helix/controller/stages/MessageSelectionStage.java > 9adc833 > > helix-core/src/main/java/org/apache/helix/controller/stages/ResourceCurrentState.java > f04afd0 > > helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java > 8381f4a > > helix-core/src/main/java/org/apache/helix/messaging/handling/HelixTaskResult.java > 22c4fcd > helix-core/src/main/java/org/apache/helix/model/CurrentState.java 5c9bcbc > > helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModel.java > b88262b > helix-core/src/main/java/org/apache/helix/task/TargetState.java e69de29 > helix-core/src/main/java/org/apache/helix/task/Task.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskConfig.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskConstants.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskContext.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskDag.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskDriver.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskFactory.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskPartitionState.java > e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskRebalancer.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskResult.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskRunner.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskState.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskStateModel.java e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskStateModelFactory.java > e69de29 > helix-core/src/main/java/org/apache/helix/task/TaskUtil.java e69de29 > helix-core/src/main/java/org/apache/helix/task/Workflow.java e69de29 > helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java e69de29 > helix-core/src/main/java/org/apache/helix/task/WorkflowContext.java e69de29 > helix-core/src/main/java/org/apache/helix/task/beans/TaskBean.java e69de29 > helix-core/src/main/java/org/apache/helix/task/beans/WorkflowBean.java > e69de29 > helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 1d02275 > > helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java > 8127626 > helix-core/src/test/java/org/apache/helix/TestZNRecord.java 9ff4849 > > helix-core/src/test/java/org/apache/helix/integration/ZkIntegrationTestBase.java > 9188e61 > > helix-core/src/test/java/org/apache/helix/integration/task/TestTaskRebalancer.java > e69de29 > > helix-core/src/test/java/org/apache/helix/integration/task/TestTaskRebalancerStopResume.java > e69de29 > helix-core/src/test/java/org/apache/helix/integration/task/TestUtil.java > e69de29 > > helix-core/src/test/java/org/apache/helix/integration/task/WorkflowGenerator.java > e69de29 > > Diff: https://reviews.apache.org/r/16071/diff/ > > > Testing > ------- > > all tests pass locally > > > Thanks, > > Zhen Zhang > >
