> On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/HelixController.java, line 23 > > <https://reviews.apache.org/r/15895/diff/1/?file=392042#file392042line23> > > > > what is the difference between helixautocontroller and helixcontroller, > > I probably missed this as part of 0.7.0
This is CONTROLLER_PARTICIPANT. Created HELIX-329 to track possibly renaming it. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/DistributedLeaderElection.java, > > line 129 > > <https://reviews.apache.org/r/15895/diff/1/?file=392045#file392045line129> > > > > can we get rid of ZKPropertyTransferServer ? Created HELIX-330. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/DistributedLeaderElection.java, > > line 136 > > <https://reviews.apache.org/r/15895/diff/1/?file=392045#file392045line136> > > > > Is port good enough? should we have host as well Added host. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/DistributedLeaderElection.java, > > line 132 > > <https://reviews.apache.org/r/15895/diff/1/?file=392045#file392045line132> > > > > Fix this typo, we should remove the Zkpropertytransferserver, this will > > remove dependency on restlet Fixed the typo. Removing the property transfer server is tracked by HELIX-330. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java, > > line 239 > > <https://reviews.apache.org/r/15895/diff/1/?file=392046#file392046line239> > > > > can we fix this message ? Changed to say "Helix Role: ..." Is that the problem you were referring to? > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java, > > line 316 > > <https://reviews.apache.org/r/15895/diff/1/?file=392046#file392046line316> > > > > why is this empty ? Fixed. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java, > > line 321 > > <https://reviews.apache.org/r/15895/diff/1/?file=392046#file392046line321> > > > > this is confusing how will the usage be at the participant and > > spectator, will they not be calling monitoringservice similar to messaging > > service Renamed to MonitoringServer to help clarify things. Also created a base interface called MonitoringServerOwner instead of defining these functions in multiple places. Setting and getting the monitoring server is controller-specific, and this is reflected in HelixController and HelixAutoController. Unfortunately, for compatibility, these functions have to be exposed in the HelixManager interface. There will be a separate MonitoringClient/Monitorable interface which any HelixRole can use, and that will also be exposed by HelixManager. These can't really be symmetric because monitoring itself isn't symmetric, so using the same functions doesn't really make sense. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java, > > line 255 > > <https://reviews.apache.org/r/15895/diff/1/?file=392049#file392049line255> > > > > can we make use of log.warn(msg,e) instead of msg + e Fixed. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixLeaderElection.java, > > line 146 > > <https://reviews.apache.org/r/15895/diff/1/?file=392050#file392050line146> > > > > I see this code in two places. Any way we can have this in one place Made it a public static method and changed the other places to call it. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/model/LiveInstance.java, line 42 > > <https://reviews.apache.org/r/15895/diff/1/?file=392052#file392052line42> > > > > why is ZKPROPERTY and MONITORING_PORT part of LiveInstanceProperty Created a new HelixProperty called Leader and moved these out. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/model/MonitoringConfig.java, line > > 47 > > <https://reviews.apache.org/r/15895/diff/1/?file=392053#file392053line47> > > > > we should avoid providing this constructor rt? This is required. See HelixProperty#convertToTypedInstance, which is called by ZKHelixDataAccessor#getProperty. > On Nov. 27, 2013, 9:08 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/monitoring/MonitoringService.java, > > line 27 > > <https://reviews.apache.org/r/15895/diff/1/?file=392055#file392055line27> > > > > We should probably call this MonitoringServer instead of Service ? Changed it. - Kanak ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15895/#review29529 ----------------------------------------------------------- On Dec. 2, 2013, 2:59 p.m., Kanak Biscuitwala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15895/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2013, 2:59 p.m.) > > > Review request for helix, Zhen Zhang and Kishore Gopalakrishna. > > > Bugs: HELIX-319 > > > Repository: helix-git > > > Description > ------- > > commit 084ce23546ba6f8c2fe9c5826fe360ce85d09527 > Author: Kanak Biscuitwala <[email protected]> > Date: Wed Nov 27 16:20:36 2013 -0800 > > [HELIX-319] Plug Riemann server into Helix, config management, first take > > :100644 100644 91ec809... 5bb6558... M > helix-core/src/main/java/org/apache/helix/HelixAutoController.java > :100644 100644 9565cfb... d55a0a1... M > helix-core/src/main/java/org/apache/helix/HelixController.java > :100644 100644 54e9943... 05ccde2... M > helix-core/src/main/java/org/apache/helix/HelixManager.java > :100644 100644 ebd7a35... 228863a... M > helix-core/src/main/java/org/apache/helix/PropertyKey.java > :100644 100644 9836020... bcc68b6... M > helix-core/src/main/java/org/apache/helix/manager/zk/DistributedLeaderElection.java > :100644 100644 e52427c... 33dcc06... M > helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java > :100644 100644 b71304a... 1beb7f3... M > helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java > :100644 100644 136d47e... 75fa69f... M > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixAutoController.java > :100644 100644 f116c76... 0e140a5... M > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java > :100644 100644 77da158... d1bdcb0... M > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixLeaderElection.java > :100644 100644 95c9848... 65847d5... M > helix-core/src/main/java/org/apache/helix/model/HelixConfigScope.java > :100644 100644 e9348ec... 21d045b... M > helix-core/src/main/java/org/apache/helix/model/LiveInstance.java > :000000 100644 0000000... b9aa17c... A > helix-core/src/main/java/org/apache/helix/model/MonitoringConfig.java > :100644 100644 d65ffd3... 87cabcf... M > helix-core/src/main/java/org/apache/helix/model/builder/HelixConfigScopeBuilder.java > :000000 100644 0000000... 8ea6c43... A > helix-core/src/main/java/org/apache/helix/monitoring/MonitoringService.java > :100644 100644 919eb00... 26843ee... M > helix-core/src/test/java/org/apache/helix/Mocks.java > :100644 100644 fc9b7d5... aa500e3... M > helix-core/src/test/java/org/apache/helix/controller/stages/DummyClusterManager.java > :100644 100644 d97b22a... b7f2de6... M > helix-core/src/test/java/org/apache/helix/participant/MockZKHelixManager.java > :000000 100644 0000000... 2001d31... A helix-monitor-server/DISCLAIMER > :000000 100644 0000000... 413913f... A helix-monitor-server/LICENSE > :000000 100644 0000000... e070e15... A helix-monitor-server/NOTICE > :000000 100644 0000000... 36e840d... A helix-monitor-server/pom.xml > :000000 100644 0000000... c2d08a1... A > helix-monitor-server/src/assemble/assembly.xml > :000000 100644 0000000... 4b3dc31... A > helix-monitor-server/src/main/config/log4j.properties > :000000 100644 0000000... e95e9bb... A > helix-monitor-server/src/main/java/org/apache/helix/monitoring/RiemannMonitoringService.java > :000000 100644 0000000... 08c3bce... A > helix-monitor-server/src/main/resources/riemann.config > :000000 100644 0000000... 90910aa... A > helix-monitor-server/src/test/conf/testng.xml > :000000 100644 0000000... ee64ca2... A > helix-monitor-server/src/test/java/org/apache/helix/monitoring/BasicMonitoringTest.java > :100644 100644 010023c... c220e2b... M pom.xml > > > 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/ControllerResource.java > ea7be42 > > helix-admin-webapp/src/main/java/org/apache/helix/webapp/resources/SchedulerTasksResource.java > 5803e98 > > helix-admin-webapp/src/test/java/org/apache/helix/tools/TestHelixAdminScenariosRest.java > 6a0e331 > helix-core/src/main/java/org/apache/helix/HelixAutoController.java 91ec809 > helix-core/src/main/java/org/apache/helix/HelixController.java 9565cfb > helix-core/src/main/java/org/apache/helix/HelixManager.java 54e9943 > helix-core/src/main/java/org/apache/helix/PropertyKey.java ebd7a35 > helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java > b01a3ec > > helix-core/src/main/java/org/apache/helix/api/accessor/ControllerAccessor.java > 609e458 > > helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java > eec745e > > helix-core/src/main/java/org/apache/helix/manager/zk/DistributedLeaderElection.java > 9836020 > > helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java > e52427c > > helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixDataAccessor.java > 471530c > helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java > b71304a > > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixAutoController.java > 136d47e > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java > f116c76 > > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixLeaderElection.java > 77da158 > helix-core/src/main/java/org/apache/helix/model/HelixConfigScope.java > 95c9848 > helix-core/src/main/java/org/apache/helix/model/Leader.java PRE-CREATION > helix-core/src/main/java/org/apache/helix/model/LiveInstance.java e9348ec > helix-core/src/main/java/org/apache/helix/model/MonitoringConfig.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/model/builder/HelixConfigScopeBuilder.java > d65ffd3 > helix-core/src/main/java/org/apache/helix/monitoring/MonitoringServer.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/monitoring/MonitoringServerOwner.java > PRE-CREATION > > helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerElection.java > 0e8c6fd > helix-core/src/test/java/org/apache/helix/Mocks.java 919eb00 > > helix-core/src/test/java/org/apache/helix/controller/stages/DummyClusterManager.java > fc9b7d5 > > helix-core/src/test/java/org/apache/helix/integration/TestDistributedCMMain.java > 8135191 > > helix-core/src/test/java/org/apache/helix/integration/TestDistributedClusterController.java > f3def23 > > helix-core/src/test/java/org/apache/helix/integration/TestStandAloneCMMain.java > 6eb7a8c > > helix-core/src/test/java/org/apache/helix/integration/ZkIntegrationTestBase.java > 9188e61 > > helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java > 8625c0c > > helix-core/src/test/java/org/apache/helix/integration/manager/TestDistributedControllerManager.java > aa00a8d > helix-core/src/test/java/org/apache/helix/manager/zk/TestZkFlapping.java > 79c74f5 > > helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAutoController.java > 28b1477 > > helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixController.java > 0127edb > > helix-core/src/test/java/org/apache/helix/participant/MockZKHelixManager.java > d97b22a > > helix-core/src/test/java/org/apache/helix/participant/TestDistControllerElection.java > 28068ec > helix-core/src/test/java/org/apache/helix/tools/TestHelixAdminCli.java > 4d7f93c > helix-monitor-server/.gitignore PRE-CREATION > helix-monitor-server/DISCLAIMER PRE-CREATION > helix-monitor-server/LICENSE PRE-CREATION > helix-monitor-server/NOTICE PRE-CREATION > helix-monitor-server/pom.xml PRE-CREATION > helix-monitor-server/src/assemble/assembly.xml PRE-CREATION > helix-monitor-server/src/main/config/log4j.properties PRE-CREATION > > helix-monitor-server/src/main/java/org/apache/helix/monitoring/RiemannMonitoringServer.java > PRE-CREATION > helix-monitor-server/src/main/resources/riemann.config PRE-CREATION > helix-monitor-server/src/test/conf/testng.xml PRE-CREATION > > helix-monitor-server/src/test/java/org/apache/helix/monitoring/BasicMonitoringTest.java > PRE-CREATION > pom.xml 010023c > > Diff: https://reviews.apache.org/r/15895/diff/ > > > Testing > ------- > > New test verifies that the server registers. > > > Thanks, > > Kanak Biscuitwala > >
