> On Aug. 1, 2014, 8:38 p.m., Kanak Biscuitwala wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java, > > line 195 > > <https://reviews.apache.org/r/24190/diff/1/?file=648666#file648666line195> > > > > It's unsafe to call HelixConnection#connect in a constructor.
have to do this; otherwise HelixManager.getStateMachineEngine().registerStateModelFactory() will through exception if helix-connection is not connected. > On Aug. 1, 2014, 8:38 p.m., Kanak Biscuitwala wrote: > > helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestHelixAdminScenariosRest.java, > > line 335 > > <https://reviews.apache.org/r/24190/diff/1/?file=648652#file648652line335> > > > > MockMultiClusterController is better fixed > On Aug. 1, 2014, 8:38 p.m., Kanak Biscuitwala wrote: > > helix-core/src/main/java/org/apache/helix/HelixAdministrator.java, line 29 > > <https://reviews.apache.org/r/24190/diff/1/?file=648657#file648657line29> > > > > Should this have getters/creators for HelixAdmin and ClusterAccessor? > > > > Alternatively, maybe make a default HelixRole implementation for the > > administrator adaptor. fixed > On Aug. 1, 2014, 8:38 p.m., Kanak Biscuitwala wrote: > > helix-core/src/main/java/org/apache/helix/HelixSpectator.java, line 24 > > <https://reviews.apache.org/r/24190/diff/1/?file=648661#file648661line24> > > > > This should either support adding listeners, or just use a default > > HelixRole implementation. fixed > On Aug. 1, 2014, 8:38 p.m., Kanak Biscuitwala wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java, > > line 444 > > <https://reviews.apache.org/r/24190/diff/1/?file=648666#file648666line444> > > > > This may be a good opportunity to remove getConfigAccessor since we > > deprecated it in an older version. undeprecated > On Aug. 1, 2014, 8:38 p.m., Kanak Biscuitwala wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java, > > line 564 > > <https://reviews.apache.org/r/24190/diff/1/?file=648666#file648666line564> > > > > Remove TODO removed > On Aug. 1, 2014, 8:38 p.m., Kanak Biscuitwala wrote: > > helix-core/src/main/java/org/apache/helix/messaging/handling/HelixTaskExecutor.java, > > line 663 > > <https://reviews.apache.org/r/24190/diff/1/?file=648674#file648674line663> > > > > log instead of printing, make message more descriptive fixed > On Aug. 1, 2014, 8:38 p.m., Kanak Biscuitwala wrote: > > recipes/jobrunner-yarn/src/main/java/org/apache/helix/provisioning/yarn/example/JobRunnerMain.java, > > line 114 > > <https://reviews.apache.org/r/24190/diff/1/?file=648768#file648768line114> > > > > Remove TODO fixed - Zhen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24190/#review49377 ----------------------------------------------------------- On Aug. 4, 2014, 8:10 p.m., Zhen Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24190/ > ----------------------------------------------------------- > > (Updated Aug. 4, 2014, 8:10 p.m.) > > > Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna. > > > Bugs: HELIX-376 > > > Repository: helix-git > > > Description > ------- > > Remove HelixConnection/HelixManager duplicate code > > > Diffs > ----- > > > helix-admin-webapp/src/main/java/org/apache/helix/webapp/resources/ClusterResource.java > b22d801 > > helix-admin-webapp/src/main/java/org/apache/helix/webapp/resources/ConfigResource.java > 3c384d4 > > helix-admin-webapp/src/main/java/org/apache/helix/webapp/resources/ConstraintResource.java > 675d0ec > > helix-admin-webapp/src/main/java/org/apache/helix/webapp/resources/ControllerResource.java > ea7be42 > > helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestHelixAdminScenariosRest.java > 66065c3 > > helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetInstance.java > b89a067 > > helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetPartitionState.java > 8cd6f42 > > helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetResource.java > 464edc4 > helix-agent/src/test/java/org/apache/helix/agent/TestHelixAgent.java > cbf0582 > helix-core/src/main/java/org/apache/helix/ConfigAccessor.java 3589165 > helix-core/src/main/java/org/apache/helix/ConfigChangeListener.java 1dbf2fe > helix-core/src/main/java/org/apache/helix/HelixAutoController.java 91ec809 > helix-core/src/main/java/org/apache/helix/HelixConnection.java ff5f458 > helix-core/src/main/java/org/apache/helix/HelixManager.java 73313c0 > helix-core/src/main/java/org/apache/helix/HelixMultiClusterController.java > PRE-CREATION > helix-core/src/main/java/org/apache/helix/HelixRole.java ffcb700 > helix-core/src/main/java/org/apache/helix/HelixService.java 40e9bae > helix-core/src/main/java/org/apache/helix/api/id/AdministratorId.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java > 7bb214e > > helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java > b6c16b5 > > helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java > 6f34953 > > helix-core/src/main/java/org/apache/helix/controller/stages/TaskAssignmentStage.java > 9d6228e > helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java > 65fe2f9 > > helix-core/src/main/java/org/apache/helix/manager/zk/ControllerManagerHelper.java > 9a817e3 > > helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java > ef17715 > helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java > f95f6ee > helix-core/src/main/java/org/apache/helix/manager/zk/ZkCallbackHandler.java > 5961fe3 > > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixAutoController.java > 1d4b225 > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java > bec6f5c > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java > f9529b7 > > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixMultiClusterController.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java > d3ee8d1 > > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixRoleDefaultImpl.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/messaging/DefaultMessagingService.java > e799e38 > > helix-core/src/main/java/org/apache/helix/messaging/handling/HelixTaskExecutor.java > 0ac3b58 > helix-core/src/main/java/org/apache/helix/model/ConfigScope.java 292ef0f > > helix-core/src/main/java/org/apache/helix/model/builder/ConfigScopeBuilder.java > a8ce835 > > helix-core/src/main/java/org/apache/helix/participant/CustomCodeInvoker.java > a736d71 > > helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyModel.java > aa21ee3 > > helix-core/src/main/java/org/apache/helix/spectator/RoutingTableProvider.java > 9bba660 > helix-core/src/test/java/org/apache/helix/Mocks.java a3a6044 > helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java 3e69327 > helix-core/src/test/java/org/apache/helix/TestHelper.java 8328a15 > helix-core/src/test/java/org/apache/helix/TestZKCallback.java 50ed0df > helix-core/src/test/java/org/apache/helix/ZkTestHelper.java 444c069 > helix-core/src/test/java/org/apache/helix/api/TestNewStages.java b5d218d > > helix-core/src/test/java/org/apache/helix/controller/stages/DummyClusterManager.java > 73ba122 > > helix-core/src/test/java/org/apache/helix/controller/stages/TestRebalancePipeline.java > 922dde6 > helix-core/src/test/java/org/apache/helix/integration/IntegrationTest.java > bb862f3 > helix-core/src/test/java/org/apache/helix/integration/TestAddClusterV2.java > 8dca7c8 > > helix-core/src/test/java/org/apache/helix/integration/TestAddNodeAfterControllerStart.java > 01d760a > > helix-core/src/test/java/org/apache/helix/integration/TestAddStateModelFactoryAfterConnect.java > 98e4294 > > helix-core/src/test/java/org/apache/helix/integration/TestAutoIsWithEmptyMap.java > 8ae722b > > helix-core/src/test/java/org/apache/helix/integration/TestAutoRebalance.java > 9802e1c > > helix-core/src/test/java/org/apache/helix/integration/TestAutoRebalancePartitionLimit.java > eef3826 > > helix-core/src/test/java/org/apache/helix/integration/TestBasicSpectator.java > 54dd97c > helix-core/src/test/java/org/apache/helix/integration/TestBatchMessage.java > 099d47f > > helix-core/src/test/java/org/apache/helix/integration/TestBatchMessageWrapper.java > a1f2b4d > > helix-core/src/test/java/org/apache/helix/integration/TestBucketizedResource.java > 0c97b13 > > helix-core/src/test/java/org/apache/helix/integration/TestCarryOverBadCurState.java > aa0bf33 > > helix-core/src/test/java/org/apache/helix/integration/TestCleanupExternalView.java > 521864b > > helix-core/src/test/java/org/apache/helix/integration/TestClusterStartsup.java > afe35f5 > > helix-core/src/test/java/org/apache/helix/integration/TestCorrectnessOnConnectivityLoss.java > dba7014 > > helix-core/src/test/java/org/apache/helix/integration/TestCustomIdealState.java > 610d574 > > helix-core/src/test/java/org/apache/helix/integration/TestCustomizedIdealStateRebalancer.java > b03a48b > helix-core/src/test/java/org/apache/helix/integration/TestDisable.java > ce88a34 > > helix-core/src/test/java/org/apache/helix/integration/TestDisableCustomCodeRunner.java > 08ffd54 > > helix-core/src/test/java/org/apache/helix/integration/TestDisableResource.java > 1419084 > > helix-core/src/test/java/org/apache/helix/integration/TestDistributedCMMain.java > 0d7d33c > > helix-core/src/test/java/org/apache/helix/integration/TestDistributedClusterController.java > cf7884a > helix-core/src/test/java/org/apache/helix/integration/TestDriver.java > 9660f10 > helix-core/src/test/java/org/apache/helix/integration/TestDrop.java ac20652 > helix-core/src/test/java/org/apache/helix/integration/TestDropResource.java > d4faf84 > > helix-core/src/test/java/org/apache/helix/integration/TestEnablePartitionDuringDisable.java > 83e3001 > > helix-core/src/test/java/org/apache/helix/integration/TestEntropyFreeNodeBounce.java > 4119fc6 > > helix-core/src/test/java/org/apache/helix/integration/TestErrorPartition.java > edc2965 > > helix-core/src/test/java/org/apache/helix/integration/TestExternalViewUpdates.java > 835f81e > > helix-core/src/test/java/org/apache/helix/integration/TestFullAutoNodeTagging.java > 6704fa9 > > helix-core/src/test/java/org/apache/helix/integration/TestHelixCustomCodeRunner.java > 2d95811 > > helix-core/src/test/java/org/apache/helix/integration/TestInstanceAutoJoin.java > c6d963d > > helix-core/src/test/java/org/apache/helix/integration/TestInvalidAutoIdealState.java > 1f1af0e > > helix-core/src/test/java/org/apache/helix/integration/TestInvalidResourceRebalance.java > 2cedb83 > > helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle.java > 1442979 > > helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle2.java > 2731b79 > > helix-core/src/test/java/org/apache/helix/integration/TestMessagingService.java > 7df9e8b > > helix-core/src/test/java/org/apache/helix/integration/TestNonOfflineInitState.java > 1329cbe > helix-core/src/test/java/org/apache/helix/integration/TestNullReplica.java > 544fbd5 > > helix-core/src/test/java/org/apache/helix/integration/TestParticipantNameCollision.java > d4d56df > > helix-core/src/test/java/org/apache/helix/integration/TestPartitionLevelTransitionConstraint.java > 2f27fd2 > helix-core/src/test/java/org/apache/helix/integration/TestPauseSignal.java > 2d87e61 > > helix-core/src/test/java/org/apache/helix/integration/TestPreferenceListAsQueue.java > 6de604b > > helix-core/src/test/java/org/apache/helix/integration/TestRedefineStateModelDef.java > 1ce31f4 > > helix-core/src/test/java/org/apache/helix/integration/TestReelectedPipelineCorrectness.java > 5595d0c > > helix-core/src/test/java/org/apache/helix/integration/TestRenamePartition.java > c692ed5 > > helix-core/src/test/java/org/apache/helix/integration/TestResetInstance.java > e6b9c2d > > helix-core/src/test/java/org/apache/helix/integration/TestResetPartitionState.java > 85ab192 > > helix-core/src/test/java/org/apache/helix/integration/TestResetResource.java > 60983af > > helix-core/src/test/java/org/apache/helix/integration/TestRestartParticipant.java > e9d2b45 > helix-core/src/test/java/org/apache/helix/integration/TestSchemataSM.java > 9adf374 > > helix-core/src/test/java/org/apache/helix/integration/TestSessionExpiryInTransition.java > bbca923 > > helix-core/src/test/java/org/apache/helix/integration/TestSharedConnection.java > 665db05 > > helix-core/src/test/java/org/apache/helix/integration/TestStandAloneCMMain.java > c1854c8 > > helix-core/src/test/java/org/apache/helix/integration/TestStandAloneCMSessionExpiry.java > da93e12 > > helix-core/src/test/java/org/apache/helix/integration/TestStartMultipleControllersWithSameName.java > 04c0352 > > helix-core/src/test/java/org/apache/helix/integration/TestStateTransitionTimeout.java > e5ff171 > helix-core/src/test/java/org/apache/helix/integration/TestSwapInstance.java > 283055c > > helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java > 111c4d2 > > helix-core/src/test/java/org/apache/helix/integration/TestZkSessionExpiry.java > 8b4e889 > > helix-core/src/test/java/org/apache/helix/integration/ZkStandAloneCMTestBase.java > 8eaf2e7 > > helix-core/src/test/java/org/apache/helix/integration/manager/ClusterControllerManager.java > b8f0f2b > > helix-core/src/test/java/org/apache/helix/integration/manager/ClusterDistributedController.java > a17ccc1 > > helix-core/src/test/java/org/apache/helix/integration/manager/MockParticipantManager.java > 917be17 > > helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java > 877cf3c > > helix-core/src/test/java/org/apache/helix/integration/manager/TestControllerManager.java > 1544dc8 > > helix-core/src/test/java/org/apache/helix/integration/manager/TestDistributedControllerManager.java > f915c4f > > helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java > 4d46883 > > helix-core/src/test/java/org/apache/helix/integration/manager/TestStateModelLeak.java > b82f156 > > helix-core/src/test/java/org/apache/helix/integration/manager/TestZkCallbackHandlerLeak.java > 650f13f > > helix-core/src/test/java/org/apache/helix/integration/task/TestIndependentTaskRebalancer.java > 2e2c8b6 > > helix-core/src/test/java/org/apache/helix/integration/task/TestTaskRebalancer.java > e39615d > > helix-core/src/test/java/org/apache/helix/integration/task/TestTaskRebalancerStopResume.java > 6de361d > helix-core/src/test/java/org/apache/helix/manager/MockListener.java 376481e > helix-core/src/test/java/org/apache/helix/manager/zk/MockController.java > PRE-CREATION > > helix-core/src/test/java/org/apache/helix/manager/zk/MockMultiClusterController.java > PRE-CREATION > helix-core/src/test/java/org/apache/helix/manager/zk/MockParticipant.java > PRE-CREATION > > helix-core/src/test/java/org/apache/helix/manager/zk/TestHandleNewSession.java > ca0d4ab > > helix-core/src/test/java/org/apache/helix/manager/zk/TestLiveInstanceBounce.java > e59dd0c > > helix-core/src/test/java/org/apache/helix/manager/zk/TestZKLiveInstanceData.java > 54f81cd > > helix-core/src/test/java/org/apache/helix/manager/zk/TestZkClusterManager.java > 0a770d0 > helix-core/src/test/java/org/apache/helix/manager/zk/TestZkFlapping.java > 6a5f002 > > helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAutoController.java > 96952d0 > > helix-core/src/test/java/org/apache/helix/manager/zk/TestZkManagerFlappingDetection.java > 86aa6e3 > > helix-core/src/test/java/org/apache/helix/manager/zk/TestZkStateChangeListener.java > a9c028c > helix-core/src/test/java/org/apache/helix/manager/zk/ZkConnTestHelper.java > PRE-CREATION > > helix-core/src/test/java/org/apache/helix/messaging/handling/TestConfigThreadpoolSize.java > 80a46fa > > helix-core/src/test/java/org/apache/helix/messaging/handling/TestResourceThreadpoolSize.java > 97a56be > > helix-core/src/test/java/org/apache/helix/mock/controller/MockController.java > 4ddaac4 > > helix-core/src/test/java/org/apache/helix/mock/controller/MockControllerProcess.java > 193abd3 > helix-core/src/test/java/org/apache/helix/model/TestConstraint.java 4d5dd95 > > helix-core/src/test/java/org/apache/helix/monitoring/TestClusterStatusMonitorLifecycle.java > c31b641 > > helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestDropResourceMetricsReset.java > 17e1837 > > helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestResetClusterMetrics.java > 5497138 > > helix-core/src/test/java/org/apache/helix/participant/MockZKHelixManager.java > 11cdd34 > > helix-core/src/test/java/org/apache/helix/store/zk/TestZkManagerWithAutoFallbackStore.java > 3e5e068 > helix-core/src/test/java/org/apache/helix/testutil/TestUtil.java bc62fca > > helix-core/src/test/java/org/apache/helix/tools/TestClusterStateVerifier.java > ec43664 > helix-core/src/test/java/org/apache/helix/tools/TestHelixAdminCli.java > 6d27dcb > > helix-provisioning/src/main/java/org/apache/helix/provisioning/yarn/AppMasterLauncher.java > 4064e10 > > recipes/jobrunner-yarn/src/main/java/org/apache/helix/provisioning/yarn/example/JobRunnerMain.java > e588ea8 > > recipes/jobrunner-yarn/src/main/java/org/apache/helix/provisioning/yarn/example/MyTaskService.java > 7c50e53 > > Diff: https://reviews.apache.org/r/24190/diff/ > > > Testing > ------- > > tests pass locally > > > Thanks, > > Zhen Zhang > >
