You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@helix.apache.org by Zhen Zhang <ne...@gmail.com> on 2014/08/05 20:52:39 UTC

Review Request 24332: [HELIX-484] Remove CallbackHandler/ZkCallbackHandler code duplication, [HELIX-486] Remove StateModelFactory/HelixStateModelFactory code duplication

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24332/
-----------------------------------------------------------

Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.


Bugs: HELIX-484 and HELIX-486


Repository: helix-git


Description
-------

Remove CallbackHandler/ZkCallbackHandler code duplication
Remove StateModelFactory/HelixStateModelFactory code duplication


Diffs
-----

  helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetInstance.java a9ecaa0 
  helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetResource.java a54b0a3 
  helix-core/src/main/java/org/apache/helix/api/id/StateModelDefId.java 7c84f0f 
  helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java 6aa3ab9 
  helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java af50eb7 
  helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java 1bb6506 
  helix-core/src/main/java/org/apache/helix/participant/CustomCodeInvoker.java 6c96629 
  helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModelFactory.java a367c81 
  helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyModel.java 3866cf5 
  helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyStateModelFactory.java 51c91cc 
  helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java 2f169cc 
  helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java 95afb70 
  helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java abb7d81 
  helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModel.java ca67d42 
  helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModelFactory.java a205910 
  helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 6599b33 
  helix-core/src/test/java/org/apache/helix/DummyProcessThread.java f51aa1d 
  helix-core/src/test/java/org/apache/helix/TestHelixTaskExecutor.java a3b16e5 
  helix-core/src/test/java/org/apache/helix/TestHelixTaskHandler.java 43b4407 
  helix-core/src/test/java/org/apache/helix/TestHelper.java 879e727 
  helix-core/src/test/java/org/apache/helix/integration/TestAddStateModelFactoryAfterConnect.java 5f37845 
  helix-core/src/test/java/org/apache/helix/integration/TestBatchMessageWrapper.java 6a6837a 
  helix-core/src/test/java/org/apache/helix/integration/TestErrorPartition.java 19af9a7 
  helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle2.java 496a16f 
  helix-core/src/test/java/org/apache/helix/integration/TestMessagingService.java 08954e5 
  helix-core/src/test/java/org/apache/helix/integration/TestMultiClusterController.java c2f9a5c 
  helix-core/src/test/java/org/apache/helix/integration/TestNonOfflineInitState.java 105633a 
  helix-core/src/test/java/org/apache/helix/integration/TestPartitionLevelTransitionConstraint.java 823a9ce 
  helix-core/src/test/java/org/apache/helix/integration/TestPreferenceListAsQueue.java 06a2b56 
  helix-core/src/test/java/org/apache/helix/integration/TestResetInstance.java 5804744 
  helix-core/src/test/java/org/apache/helix/integration/TestResetPartitionState.java 4855b3d 
  helix-core/src/test/java/org/apache/helix/integration/TestResetResource.java 7d28931 
  helix-core/src/test/java/org/apache/helix/integration/TestSchedulerMessage.java 89af602 
  helix-core/src/test/java/org/apache/helix/integration/TestStateTransitionTimeout.java c4304b0 
  helix-core/src/test/java/org/apache/helix/integration/TestZkReconnect.java 18f6fd7 
  helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java 99986ef 
  helix-core/src/test/java/org/apache/helix/integration/manager/TestHelixMultiClusterController.java 18234b5 
  helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java 309ab18 
  helix-core/src/test/java/org/apache/helix/integration/manager/TestStateModelLeak.java d6d7bab 
  helix-core/src/test/java/org/apache/helix/manager/zk/MockMultiClusterController.java 7f8b1a3 
  helix-core/src/test/java/org/apache/helix/manager/zk/MockParticipant.java f107d3d 
  helix-core/src/test/java/org/apache/helix/manager/zk/TestDefaultControllerMsgHandlerFactory.java 8b5b30c 
  helix-core/src/test/java/org/apache/helix/messaging/TestAsyncCallbackSvc.java da686fe 
  helix-core/src/test/java/org/apache/helix/mock/participant/DummyProcess.java 9880605 
  helix-core/src/test/java/org/apache/helix/mock/participant/MockBootstrapModelFactory.java 177e7c4 
  helix-core/src/test/java/org/apache/helix/mock/participant/MockMSModelFactory.java 9325934 
  helix-core/src/test/java/org/apache/helix/mock/participant/MockSchemataModelFactory.java 525e764 
  helix-core/src/test/java/org/apache/helix/model/TestConstraint.java 6f78427 
  helix-core/src/test/java/org/apache/helix/participant/MockZKHelixManager.java efa30da 
  helix-core/src/test/java/org/apache/helix/participant/TestDistControllerStateModelFactory.java 26d65f0 
  helix-examples/src/main/java/org/apache/helix/examples/BootstrapHandler.java f0922f3 
  helix-examples/src/main/java/org/apache/helix/examples/BootstrapProcess.java 2506c01 
  helix-examples/src/main/java/org/apache/helix/examples/DummyParticipant.java c6ab3a4 
  helix-examples/src/main/java/org/apache/helix/examples/ExampleProcess.java 840a963 
  helix-examples/src/main/java/org/apache/helix/examples/LeaderStandbyStateModelFactory.java 43ac5de 
  helix-examples/src/main/java/org/apache/helix/examples/MasterSlaveStateModelFactory.java 71d1412 
  helix-examples/src/main/java/org/apache/helix/examples/OnlineOfflineStateModelFactory.java daf03a9 
  helix-examples/src/main/java/org/apache/helix/examples/Quickstart.java 2f3a677 

Diff: https://reviews.apache.org/r/24332/diff/


Testing
-------


Thanks,

Zhen Zhang


Re: Review Request 24332: [HELIX-484] Remove CallbackHandler/ZkCallbackHandler code duplication, [HELIX-486] Remove StateModelFactory/HelixStateModelFactory code duplication

Posted by Kanak Biscuitwala <ka...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24332/#review49943
-----------------------------------------------------------

Ship it!


Ship It!

- Kanak Biscuitwala


On Aug. 7, 2014, 10:50 a.m., Zhen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24332/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2014, 10:50 a.m.)
> 
> 
> Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
> 
> 
> Bugs: HELIX-484 and HELIX-486
> 
> 
> Repository: helix-git
> 
> 
> Description
> -------
> 
> Remove CallbackHandler/ZkCallbackHandler code duplication
> Remove StateModelFactory/HelixStateModelFactory code duplication
> 
> 
> Diffs
> -----
> 
>   helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetInstance.java a9ecaa0 
>   helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetResource.java a54b0a3 
>   helix-agent/src/main/java/org/apache/helix/agent/AgentStateModel.java d227ac3 
>   helix-agent/src/main/java/org/apache/helix/agent/AgentStateModelFactory.java 69d45ae 
>   helix-core/src/main/java/org/apache/helix/api/StateTransitionHandlerFactory.java 45f56e5 
>   helix-core/src/main/java/org/apache/helix/api/TransitionHandler.java 9717340 
>   helix-core/src/main/java/org/apache/helix/api/id/StateModelDefId.java 7c84f0f 
>   helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java 6aa3ab9 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java af50eb7 
>   helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java 1bb6506 
>   helix-core/src/main/java/org/apache/helix/participant/CustomCodeInvoker.java 6c96629 
>   helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModel.java 0c2eb7c 
>   helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModelFactory.java a367c81 
>   helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyModel.java 3866cf5 
>   helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyStateModelFactory.java 51c91cc 
>   helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java 2f169cc 
>   helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java 95afb70 
>   helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java abb7d81 
>   helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModel.java ca67d42 
>   helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModelFactory.java a205910 
>   helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelParser.java eddeaa5 
>   helix-core/src/main/java/org/apache/helix/task/TaskRunner.java 66abba6 
>   helix-core/src/main/java/org/apache/helix/task/TaskStateModel.java a44a8cb 
>   helix-core/src/main/java/org/apache/helix/task/TaskStateModelFactory.java 2537747 
>   helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 6599b33 
>   helix-core/src/test/java/org/apache/helix/DummyProcessThread.java f51aa1d 
>   helix-core/src/test/java/org/apache/helix/Mocks.java 0303f12 
>   helix-core/src/test/java/org/apache/helix/TestHelixTaskExecutor.java a3b16e5 
>   helix-core/src/test/java/org/apache/helix/TestHelixTaskHandler.java 43b4407 
>   helix-core/src/test/java/org/apache/helix/TestHelper.java 879e727 
>   helix-core/src/test/java/org/apache/helix/integration/TestAddStateModelFactoryAfterConnect.java 5f37845 
>   helix-core/src/test/java/org/apache/helix/integration/TestBatchMessageWrapper.java 6a6837a 
>   helix-core/src/test/java/org/apache/helix/integration/TestCorrectnessOnConnectivityLoss.java abb2a7b 
>   helix-core/src/test/java/org/apache/helix/integration/TestErrorPartition.java 19af9a7 
>   helix-core/src/test/java/org/apache/helix/integration/TestHelixConnection.java 3d02ae8 
>   helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle2.java 496a16f 
>   helix-core/src/test/java/org/apache/helix/integration/TestMessagingService.java 08954e5 
>   helix-core/src/test/java/org/apache/helix/integration/TestMultiClusterController.java c2f9a5c 
>   helix-core/src/test/java/org/apache/helix/integration/TestNonOfflineInitState.java 105633a 
>   helix-core/src/test/java/org/apache/helix/integration/TestPartitionLevelTransitionConstraint.java 823a9ce 
>   helix-core/src/test/java/org/apache/helix/integration/TestPreferenceListAsQueue.java 06a2b56 
>   helix-core/src/test/java/org/apache/helix/integration/TestResetInstance.java 5804744 
>   helix-core/src/test/java/org/apache/helix/integration/TestResetPartitionState.java 4855b3d 
>   helix-core/src/test/java/org/apache/helix/integration/TestResetResource.java 7d28931 
>   helix-core/src/test/java/org/apache/helix/integration/TestSchedulerMessage.java 89af602 
>   helix-core/src/test/java/org/apache/helix/integration/TestStateTransitionTimeout.java c4304b0 
>   helix-core/src/test/java/org/apache/helix/integration/TestZkReconnect.java 18f6fd7 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java 99986ef 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestHelixMultiClusterController.java 18234b5 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java 309ab18 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestStateModelLeak.java d6d7bab 
>   helix-core/src/test/java/org/apache/helix/manager/zk/MockMultiClusterController.java 7f8b1a3 
>   helix-core/src/test/java/org/apache/helix/manager/zk/MockParticipant.java f107d3d 
>   helix-core/src/test/java/org/apache/helix/manager/zk/TestDefaultControllerMsgHandlerFactory.java 8b5b30c 
>   helix-core/src/test/java/org/apache/helix/messaging/TestAsyncCallbackSvc.java da686fe 
>   helix-core/src/test/java/org/apache/helix/mock/participant/DummyProcess.java 9880605 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockBootstrapModelFactory.java 177e7c4 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockBootstrapStateModel.java 79367db 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockMSModelFactory.java 9325934 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockMSStateModel.java 78d9832 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockSchemataModelFactory.java 525e764 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockSchemataStateModel.java c3c1fa5 
>   helix-core/src/test/java/org/apache/helix/model/TestConstraint.java 6f78427 
>   helix-core/src/test/java/org/apache/helix/participant/MockZKHelixManager.java efa30da 
>   helix-core/src/test/java/org/apache/helix/participant/TestDistControllerStateModelFactory.java 26d65f0 
>   helix-core/src/test/java/org/apache/helix/participant/statemachine/TestStateModelParser.java 7c128fb 
>   helix-examples/src/main/java/org/apache/helix/examples/BootstrapHandler.java f0922f3 
>   helix-examples/src/main/java/org/apache/helix/examples/BootstrapProcess.java 2506c01 
>   helix-examples/src/main/java/org/apache/helix/examples/DummyParticipant.java c6ab3a4 
>   helix-examples/src/main/java/org/apache/helix/examples/ExampleProcess.java 840a963 
>   helix-examples/src/main/java/org/apache/helix/examples/LeaderStandbyStateModelFactory.java 43ac5de 
>   helix-examples/src/main/java/org/apache/helix/examples/LogicalModelExample.java 6075d22 
>   helix-examples/src/main/java/org/apache/helix/examples/MasterSlaveStateModelFactory.java 71d1412 
>   helix-examples/src/main/java/org/apache/helix/examples/OnlineOfflineStateModelFactory.java daf03a9 
>   helix-examples/src/main/java/org/apache/helix/examples/Quickstart.java 2f3a677 
>   helix-provisioning/src/main/java/org/apache/helix/provisioning/participant/StatelessServiceStateModel.java f653de8 
>   helix-provisioning/src/main/java/org/apache/helix/provisioning/participant/StatelessServiceStateModelFactory.java 19c1488 
>   recipes/distributed-lock-manager/src/main/java/org/apache/helix/lockmanager/Lock.java 2ca3153 
>   recipes/distributed-lock-manager/src/main/java/org/apache/helix/lockmanager/LockFactory.java ab423f4 
>   recipes/rabbitmq-consumer-group/src/main/java/org/apache/helix/recipes/rabbitmq/Consumer.java 0b164b3 
>   recipes/rabbitmq-consumer-group/src/main/java/org/apache/helix/recipes/rabbitmq/ConsumerStateModel.java 24e4a40 
>   recipes/rabbitmq-consumer-group/src/main/java/org/apache/helix/recipes/rabbitmq/ConsumerStateTransitionHandlerFactory.java 98cce35 
>   recipes/rsync-replicated-file-system/src/main/java/org/apache/helix/filestore/FileStoreStateModel.java 6eaf808 
>   recipes/rsync-replicated-file-system/src/main/java/org/apache/helix/filestore/FileStoreStateModelFactory.java 7e1938c 
>   recipes/task-execution/src/main/java/org/apache/helix/taskexecution/TaskStateModel.java 3c1cab4 
>   recipes/task-execution/src/main/java/org/apache/helix/taskexecution/TaskStateModelFactory.java 6948237 
>   recipes/user-defined-rebalancer/src/main/java/org/apache/helix/userdefinedrebalancer/Lock.java 308ae14 
>   recipes/user-defined-rebalancer/src/main/java/org/apache/helix/userdefinedrebalancer/LockFactory.java c607b1b 
> 
> Diff: https://reviews.apache.org/r/24332/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhen Zhang
> 
>


Re: Review Request 24332: [HELIX-484] Remove CallbackHandler/ZkCallbackHandler code duplication, [HELIX-486] Remove StateModelFactory/HelixStateModelFactory code duplication

Posted by Zhen Zhang <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24332/
-----------------------------------------------------------

(Updated Aug. 7, 2014, 5:50 p.m.)


Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.


Changes
-------

address the comments:
rename StateModel to TransitionHandler and move it the api package
rename HelixStateModelFactory to StateTransitionHandlerFactory and move it to api package


Bugs: HELIX-484 and HELIX-486


Repository: helix-git


Description
-------

Remove CallbackHandler/ZkCallbackHandler code duplication
Remove StateModelFactory/HelixStateModelFactory code duplication


Diffs (updated)
-----

  helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetInstance.java a9ecaa0 
  helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetResource.java a54b0a3 
  helix-agent/src/main/java/org/apache/helix/agent/AgentStateModel.java d227ac3 
  helix-agent/src/main/java/org/apache/helix/agent/AgentStateModelFactory.java 69d45ae 
  helix-core/src/main/java/org/apache/helix/api/StateTransitionHandlerFactory.java 45f56e5 
  helix-core/src/main/java/org/apache/helix/api/TransitionHandler.java 9717340 
  helix-core/src/main/java/org/apache/helix/api/id/StateModelDefId.java 7c84f0f 
  helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java 6aa3ab9 
  helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java af50eb7 
  helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java 1bb6506 
  helix-core/src/main/java/org/apache/helix/participant/CustomCodeInvoker.java 6c96629 
  helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModel.java 0c2eb7c 
  helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModelFactory.java a367c81 
  helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyModel.java 3866cf5 
  helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyStateModelFactory.java 51c91cc 
  helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java 2f169cc 
  helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java 95afb70 
  helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java abb7d81 
  helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModel.java ca67d42 
  helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModelFactory.java a205910 
  helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelParser.java eddeaa5 
  helix-core/src/main/java/org/apache/helix/task/TaskRunner.java 66abba6 
  helix-core/src/main/java/org/apache/helix/task/TaskStateModel.java a44a8cb 
  helix-core/src/main/java/org/apache/helix/task/TaskStateModelFactory.java 2537747 
  helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 6599b33 
  helix-core/src/test/java/org/apache/helix/DummyProcessThread.java f51aa1d 
  helix-core/src/test/java/org/apache/helix/Mocks.java 0303f12 
  helix-core/src/test/java/org/apache/helix/TestHelixTaskExecutor.java a3b16e5 
  helix-core/src/test/java/org/apache/helix/TestHelixTaskHandler.java 43b4407 
  helix-core/src/test/java/org/apache/helix/TestHelper.java 879e727 
  helix-core/src/test/java/org/apache/helix/integration/TestAddStateModelFactoryAfterConnect.java 5f37845 
  helix-core/src/test/java/org/apache/helix/integration/TestBatchMessageWrapper.java 6a6837a 
  helix-core/src/test/java/org/apache/helix/integration/TestCorrectnessOnConnectivityLoss.java abb2a7b 
  helix-core/src/test/java/org/apache/helix/integration/TestErrorPartition.java 19af9a7 
  helix-core/src/test/java/org/apache/helix/integration/TestHelixConnection.java 3d02ae8 
  helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle2.java 496a16f 
  helix-core/src/test/java/org/apache/helix/integration/TestMessagingService.java 08954e5 
  helix-core/src/test/java/org/apache/helix/integration/TestMultiClusterController.java c2f9a5c 
  helix-core/src/test/java/org/apache/helix/integration/TestNonOfflineInitState.java 105633a 
  helix-core/src/test/java/org/apache/helix/integration/TestPartitionLevelTransitionConstraint.java 823a9ce 
  helix-core/src/test/java/org/apache/helix/integration/TestPreferenceListAsQueue.java 06a2b56 
  helix-core/src/test/java/org/apache/helix/integration/TestResetInstance.java 5804744 
  helix-core/src/test/java/org/apache/helix/integration/TestResetPartitionState.java 4855b3d 
  helix-core/src/test/java/org/apache/helix/integration/TestResetResource.java 7d28931 
  helix-core/src/test/java/org/apache/helix/integration/TestSchedulerMessage.java 89af602 
  helix-core/src/test/java/org/apache/helix/integration/TestStateTransitionTimeout.java c4304b0 
  helix-core/src/test/java/org/apache/helix/integration/TestZkReconnect.java 18f6fd7 
  helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java 99986ef 
  helix-core/src/test/java/org/apache/helix/integration/manager/TestHelixMultiClusterController.java 18234b5 
  helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java 309ab18 
  helix-core/src/test/java/org/apache/helix/integration/manager/TestStateModelLeak.java d6d7bab 
  helix-core/src/test/java/org/apache/helix/manager/zk/MockMultiClusterController.java 7f8b1a3 
  helix-core/src/test/java/org/apache/helix/manager/zk/MockParticipant.java f107d3d 
  helix-core/src/test/java/org/apache/helix/manager/zk/TestDefaultControllerMsgHandlerFactory.java 8b5b30c 
  helix-core/src/test/java/org/apache/helix/messaging/TestAsyncCallbackSvc.java da686fe 
  helix-core/src/test/java/org/apache/helix/mock/participant/DummyProcess.java 9880605 
  helix-core/src/test/java/org/apache/helix/mock/participant/MockBootstrapModelFactory.java 177e7c4 
  helix-core/src/test/java/org/apache/helix/mock/participant/MockBootstrapStateModel.java 79367db 
  helix-core/src/test/java/org/apache/helix/mock/participant/MockMSModelFactory.java 9325934 
  helix-core/src/test/java/org/apache/helix/mock/participant/MockMSStateModel.java 78d9832 
  helix-core/src/test/java/org/apache/helix/mock/participant/MockSchemataModelFactory.java 525e764 
  helix-core/src/test/java/org/apache/helix/mock/participant/MockSchemataStateModel.java c3c1fa5 
  helix-core/src/test/java/org/apache/helix/model/TestConstraint.java 6f78427 
  helix-core/src/test/java/org/apache/helix/participant/MockZKHelixManager.java efa30da 
  helix-core/src/test/java/org/apache/helix/participant/TestDistControllerStateModelFactory.java 26d65f0 
  helix-core/src/test/java/org/apache/helix/participant/statemachine/TestStateModelParser.java 7c128fb 
  helix-examples/src/main/java/org/apache/helix/examples/BootstrapHandler.java f0922f3 
  helix-examples/src/main/java/org/apache/helix/examples/BootstrapProcess.java 2506c01 
  helix-examples/src/main/java/org/apache/helix/examples/DummyParticipant.java c6ab3a4 
  helix-examples/src/main/java/org/apache/helix/examples/ExampleProcess.java 840a963 
  helix-examples/src/main/java/org/apache/helix/examples/LeaderStandbyStateModelFactory.java 43ac5de 
  helix-examples/src/main/java/org/apache/helix/examples/LogicalModelExample.java 6075d22 
  helix-examples/src/main/java/org/apache/helix/examples/MasterSlaveStateModelFactory.java 71d1412 
  helix-examples/src/main/java/org/apache/helix/examples/OnlineOfflineStateModelFactory.java daf03a9 
  helix-examples/src/main/java/org/apache/helix/examples/Quickstart.java 2f3a677 
  helix-provisioning/src/main/java/org/apache/helix/provisioning/participant/StatelessServiceStateModel.java f653de8 
  helix-provisioning/src/main/java/org/apache/helix/provisioning/participant/StatelessServiceStateModelFactory.java 19c1488 
  recipes/distributed-lock-manager/src/main/java/org/apache/helix/lockmanager/Lock.java 2ca3153 
  recipes/distributed-lock-manager/src/main/java/org/apache/helix/lockmanager/LockFactory.java ab423f4 
  recipes/rabbitmq-consumer-group/src/main/java/org/apache/helix/recipes/rabbitmq/Consumer.java 0b164b3 
  recipes/rabbitmq-consumer-group/src/main/java/org/apache/helix/recipes/rabbitmq/ConsumerStateModel.java 24e4a40 
  recipes/rabbitmq-consumer-group/src/main/java/org/apache/helix/recipes/rabbitmq/ConsumerStateTransitionHandlerFactory.java 98cce35 
  recipes/rsync-replicated-file-system/src/main/java/org/apache/helix/filestore/FileStoreStateModel.java 6eaf808 
  recipes/rsync-replicated-file-system/src/main/java/org/apache/helix/filestore/FileStoreStateModelFactory.java 7e1938c 
  recipes/task-execution/src/main/java/org/apache/helix/taskexecution/TaskStateModel.java 3c1cab4 
  recipes/task-execution/src/main/java/org/apache/helix/taskexecution/TaskStateModelFactory.java 6948237 
  recipes/user-defined-rebalancer/src/main/java/org/apache/helix/userdefinedrebalancer/Lock.java 308ae14 
  recipes/user-defined-rebalancer/src/main/java/org/apache/helix/userdefinedrebalancer/LockFactory.java c607b1b 

Diff: https://reviews.apache.org/r/24332/diff/


Testing
-------


Thanks,

Zhen Zhang


Re: Review Request 24332: [HELIX-484] Remove CallbackHandler/ZkCallbackHandler code duplication, [HELIX-486] Remove StateModelFactory/HelixStateModelFactory code duplication

Posted by Zhen Zhang <ne...@gmail.com>.

> On Aug. 5, 2014, 7:10 p.m., Kishore Gopalakrishna wrote:
> > Instead of adding HelixStateModelFactory can we rename this class to StateTransitionHandlerFactory and StateModel to TransitionHandler and move it to api? Adding Helix prefix will create more confusion.

sure. will rename it.


> On Aug. 5, 2014, 7:10 p.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java, line 262
> > <https://reviews.apache.org/r/24332/diff/1/?file=652241#file652241line262>
> >
> >     what is the equivalent of removing statemodel

we are replacing string with concrete id. removeStateModelFactory(StateModelDefId id, ...)


> On Aug. 5, 2014, 7:10 p.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java, line 35
> > <https://reviews.apache.org/r/24332/diff/1/?file=652242#file652242line35>
> >
> >     so we are breaking backwards compatibility?

yes. in 0.7 we could break some backward compatibility as long as the error will be obvious and easy to fix. otherwise maintaining two set of interfaces will be painful.


- Zhen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24332/#review49630
-----------------------------------------------------------


On Aug. 5, 2014, 6:52 p.m., Zhen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24332/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2014, 6:52 p.m.)
> 
> 
> Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
> 
> 
> Bugs: HELIX-484 and HELIX-486
> 
> 
> Repository: helix-git
> 
> 
> Description
> -------
> 
> Remove CallbackHandler/ZkCallbackHandler code duplication
> Remove StateModelFactory/HelixStateModelFactory code duplication
> 
> 
> Diffs
> -----
> 
>   helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetInstance.java a9ecaa0 
>   helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetResource.java a54b0a3 
>   helix-core/src/main/java/org/apache/helix/api/id/StateModelDefId.java 7c84f0f 
>   helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java 6aa3ab9 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java af50eb7 
>   helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java 1bb6506 
>   helix-core/src/main/java/org/apache/helix/participant/CustomCodeInvoker.java 6c96629 
>   helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModelFactory.java a367c81 
>   helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyModel.java 3866cf5 
>   helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyStateModelFactory.java 51c91cc 
>   helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java 2f169cc 
>   helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java 95afb70 
>   helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java abb7d81 
>   helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModel.java ca67d42 
>   helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModelFactory.java a205910 
>   helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 6599b33 
>   helix-core/src/test/java/org/apache/helix/DummyProcessThread.java f51aa1d 
>   helix-core/src/test/java/org/apache/helix/TestHelixTaskExecutor.java a3b16e5 
>   helix-core/src/test/java/org/apache/helix/TestHelixTaskHandler.java 43b4407 
>   helix-core/src/test/java/org/apache/helix/TestHelper.java 879e727 
>   helix-core/src/test/java/org/apache/helix/integration/TestAddStateModelFactoryAfterConnect.java 5f37845 
>   helix-core/src/test/java/org/apache/helix/integration/TestBatchMessageWrapper.java 6a6837a 
>   helix-core/src/test/java/org/apache/helix/integration/TestErrorPartition.java 19af9a7 
>   helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle2.java 496a16f 
>   helix-core/src/test/java/org/apache/helix/integration/TestMessagingService.java 08954e5 
>   helix-core/src/test/java/org/apache/helix/integration/TestMultiClusterController.java c2f9a5c 
>   helix-core/src/test/java/org/apache/helix/integration/TestNonOfflineInitState.java 105633a 
>   helix-core/src/test/java/org/apache/helix/integration/TestPartitionLevelTransitionConstraint.java 823a9ce 
>   helix-core/src/test/java/org/apache/helix/integration/TestPreferenceListAsQueue.java 06a2b56 
>   helix-core/src/test/java/org/apache/helix/integration/TestResetInstance.java 5804744 
>   helix-core/src/test/java/org/apache/helix/integration/TestResetPartitionState.java 4855b3d 
>   helix-core/src/test/java/org/apache/helix/integration/TestResetResource.java 7d28931 
>   helix-core/src/test/java/org/apache/helix/integration/TestSchedulerMessage.java 89af602 
>   helix-core/src/test/java/org/apache/helix/integration/TestStateTransitionTimeout.java c4304b0 
>   helix-core/src/test/java/org/apache/helix/integration/TestZkReconnect.java 18f6fd7 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java 99986ef 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestHelixMultiClusterController.java 18234b5 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java 309ab18 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestStateModelLeak.java d6d7bab 
>   helix-core/src/test/java/org/apache/helix/manager/zk/MockMultiClusterController.java 7f8b1a3 
>   helix-core/src/test/java/org/apache/helix/manager/zk/MockParticipant.java f107d3d 
>   helix-core/src/test/java/org/apache/helix/manager/zk/TestDefaultControllerMsgHandlerFactory.java 8b5b30c 
>   helix-core/src/test/java/org/apache/helix/messaging/TestAsyncCallbackSvc.java da686fe 
>   helix-core/src/test/java/org/apache/helix/mock/participant/DummyProcess.java 9880605 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockBootstrapModelFactory.java 177e7c4 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockMSModelFactory.java 9325934 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockSchemataModelFactory.java 525e764 
>   helix-core/src/test/java/org/apache/helix/model/TestConstraint.java 6f78427 
>   helix-core/src/test/java/org/apache/helix/participant/MockZKHelixManager.java efa30da 
>   helix-core/src/test/java/org/apache/helix/participant/TestDistControllerStateModelFactory.java 26d65f0 
>   helix-examples/src/main/java/org/apache/helix/examples/BootstrapHandler.java f0922f3 
>   helix-examples/src/main/java/org/apache/helix/examples/BootstrapProcess.java 2506c01 
>   helix-examples/src/main/java/org/apache/helix/examples/DummyParticipant.java c6ab3a4 
>   helix-examples/src/main/java/org/apache/helix/examples/ExampleProcess.java 840a963 
>   helix-examples/src/main/java/org/apache/helix/examples/LeaderStandbyStateModelFactory.java 43ac5de 
>   helix-examples/src/main/java/org/apache/helix/examples/MasterSlaveStateModelFactory.java 71d1412 
>   helix-examples/src/main/java/org/apache/helix/examples/OnlineOfflineStateModelFactory.java daf03a9 
>   helix-examples/src/main/java/org/apache/helix/examples/Quickstart.java 2f3a677 
> 
> Diff: https://reviews.apache.org/r/24332/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhen Zhang
> 
>


Re: Review Request 24332: [HELIX-484] Remove CallbackHandler/ZkCallbackHandler code duplication, [HELIX-486] Remove StateModelFactory/HelixStateModelFactory code duplication

Posted by Kishore Gopalakrishna <ki...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24332/#review49630
-----------------------------------------------------------


Instead of adding HelixStateModelFactory can we rename this class to StateTransitionHandlerFactory and StateModel to TransitionHandler and move it to api? Adding Helix prefix will create more confusion.


helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java
<https://reviews.apache.org/r/24332/#comment86879>

    what is the equivalent of removing statemodel



helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java
<https://reviews.apache.org/r/24332/#comment86880>

    so we are breaking backwards compatibility?


- Kishore Gopalakrishna


On Aug. 5, 2014, 6:52 p.m., Zhen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24332/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2014, 6:52 p.m.)
> 
> 
> Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
> 
> 
> Bugs: HELIX-484 and HELIX-486
> 
> 
> Repository: helix-git
> 
> 
> Description
> -------
> 
> Remove CallbackHandler/ZkCallbackHandler code duplication
> Remove StateModelFactory/HelixStateModelFactory code duplication
> 
> 
> Diffs
> -----
> 
>   helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetInstance.java a9ecaa0 
>   helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetResource.java a54b0a3 
>   helix-core/src/main/java/org/apache/helix/api/id/StateModelDefId.java 7c84f0f 
>   helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java 6aa3ab9 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java af50eb7 
>   helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java 1bb6506 
>   helix-core/src/main/java/org/apache/helix/participant/CustomCodeInvoker.java 6c96629 
>   helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModelFactory.java a367c81 
>   helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyModel.java 3866cf5 
>   helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyStateModelFactory.java 51c91cc 
>   helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java 2f169cc 
>   helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java 95afb70 
>   helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java abb7d81 
>   helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModel.java ca67d42 
>   helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModelFactory.java a205910 
>   helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 6599b33 
>   helix-core/src/test/java/org/apache/helix/DummyProcessThread.java f51aa1d 
>   helix-core/src/test/java/org/apache/helix/TestHelixTaskExecutor.java a3b16e5 
>   helix-core/src/test/java/org/apache/helix/TestHelixTaskHandler.java 43b4407 
>   helix-core/src/test/java/org/apache/helix/TestHelper.java 879e727 
>   helix-core/src/test/java/org/apache/helix/integration/TestAddStateModelFactoryAfterConnect.java 5f37845 
>   helix-core/src/test/java/org/apache/helix/integration/TestBatchMessageWrapper.java 6a6837a 
>   helix-core/src/test/java/org/apache/helix/integration/TestErrorPartition.java 19af9a7 
>   helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle2.java 496a16f 
>   helix-core/src/test/java/org/apache/helix/integration/TestMessagingService.java 08954e5 
>   helix-core/src/test/java/org/apache/helix/integration/TestMultiClusterController.java c2f9a5c 
>   helix-core/src/test/java/org/apache/helix/integration/TestNonOfflineInitState.java 105633a 
>   helix-core/src/test/java/org/apache/helix/integration/TestPartitionLevelTransitionConstraint.java 823a9ce 
>   helix-core/src/test/java/org/apache/helix/integration/TestPreferenceListAsQueue.java 06a2b56 
>   helix-core/src/test/java/org/apache/helix/integration/TestResetInstance.java 5804744 
>   helix-core/src/test/java/org/apache/helix/integration/TestResetPartitionState.java 4855b3d 
>   helix-core/src/test/java/org/apache/helix/integration/TestResetResource.java 7d28931 
>   helix-core/src/test/java/org/apache/helix/integration/TestSchedulerMessage.java 89af602 
>   helix-core/src/test/java/org/apache/helix/integration/TestStateTransitionTimeout.java c4304b0 
>   helix-core/src/test/java/org/apache/helix/integration/TestZkReconnect.java 18f6fd7 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java 99986ef 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestHelixMultiClusterController.java 18234b5 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java 309ab18 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestStateModelLeak.java d6d7bab 
>   helix-core/src/test/java/org/apache/helix/manager/zk/MockMultiClusterController.java 7f8b1a3 
>   helix-core/src/test/java/org/apache/helix/manager/zk/MockParticipant.java f107d3d 
>   helix-core/src/test/java/org/apache/helix/manager/zk/TestDefaultControllerMsgHandlerFactory.java 8b5b30c 
>   helix-core/src/test/java/org/apache/helix/messaging/TestAsyncCallbackSvc.java da686fe 
>   helix-core/src/test/java/org/apache/helix/mock/participant/DummyProcess.java 9880605 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockBootstrapModelFactory.java 177e7c4 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockMSModelFactory.java 9325934 
>   helix-core/src/test/java/org/apache/helix/mock/participant/MockSchemataModelFactory.java 525e764 
>   helix-core/src/test/java/org/apache/helix/model/TestConstraint.java 6f78427 
>   helix-core/src/test/java/org/apache/helix/participant/MockZKHelixManager.java efa30da 
>   helix-core/src/test/java/org/apache/helix/participant/TestDistControllerStateModelFactory.java 26d65f0 
>   helix-examples/src/main/java/org/apache/helix/examples/BootstrapHandler.java f0922f3 
>   helix-examples/src/main/java/org/apache/helix/examples/BootstrapProcess.java 2506c01 
>   helix-examples/src/main/java/org/apache/helix/examples/DummyParticipant.java c6ab3a4 
>   helix-examples/src/main/java/org/apache/helix/examples/ExampleProcess.java 840a963 
>   helix-examples/src/main/java/org/apache/helix/examples/LeaderStandbyStateModelFactory.java 43ac5de 
>   helix-examples/src/main/java/org/apache/helix/examples/MasterSlaveStateModelFactory.java 71d1412 
>   helix-examples/src/main/java/org/apache/helix/examples/OnlineOfflineStateModelFactory.java daf03a9 
>   helix-examples/src/main/java/org/apache/helix/examples/Quickstart.java 2f3a677 
> 
> Diff: https://reviews.apache.org/r/24332/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhen Zhang
> 
>