> On March 14, 2014, 11:58 p.m., Kishore Gopalakrishna wrote: > > shud we have a separate package under monitoring for Riemann. Are we > > emitting any metrics.
yes. we can have a separate package for Riemann. we are not emitting metrics > On March 14, 2014, 11:58 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/monitoring/MonitoringClient.java, > > line 48 > > <https://reviews.apache.org/r/19240/diff/1/?file=520033#file520033line48> > > > > why do we have batch boolean in the method. Should we not have the > > interval in the constructor ? > > Kanak Biscuitwala wrote: > Since we have sendAndFlush, the batch argument is probably unnecessary. agree. the batch argument can be removed. if batchSize>1, use batchClient, otherwise use normal client > On March 14, 2014, 11:58 p.m., Kishore Gopalakrishna wrote: > > helix-monitor-client/src/main/java/org/apache/helix/monitoring/RiemannClientWrapper.java, > > line 31 > > <https://reviews.apache.org/r/19240/diff/1/?file=520037#file520037line31> > > > > this State will confuse with Helix State, can we make this private if > > its not used outside of this class or rename the enum will change to private > On March 14, 2014, 11:58 p.m., Kishore Gopalakrishna wrote: > > helix-core/src/main/java/org/apache/helix/monitoring/MonitoringEvent.java, > > line 258 > > <https://reviews.apache.org/r/19240/diff/1/?file=520034#file520034line258> > > > > can we have only one shardingKey. Does it make sense to call this > > shardingKey. Instead user should probably think about the scopes (cluster, > > resource, partition, participant) ? ShardingKey is probably for advanced > > users. what do you think? we can have a sharding strategy, user can specify a scope or a string. we can provide "cluster + resource" as the default sharding strategy. > On March 14, 2014, 11:58 p.m., Kishore Gopalakrishna wrote: > > helix-monitor-server/src/main/java/org/apache/helix/monitoring/RiemannAgent.java, > > line 73 > > <https://reviews.apache.org/r/19240/diff/1/?file=520038#file520038line73> > > > > can we create constants for heartbeat, running ttl etc will do > On March 14, 2014, 11:58 p.m., Kishore Gopalakrishna wrote: > > helix-monitor-server/src/main/java/org/apache/helix/monitoring/RiemannAlertProxy.java, > > line 59 > > <https://reviews.apache.org/r/19240/diff/1/?file=520039#file520039line59> > > > > dont we have to call zkClient.waitUntilconnected or something ? it's fine. zkclient operations are all wrapped with retryUntilConnected(...) > On March 14, 2014, 11:58 p.m., Kishore Gopalakrishna wrote: > > helix-monitor-server/src/main/java/org/apache/helix/monitoring/RiemannAlertProxy.java, > > line 72 > > <https://reviews.apache.org/r/19240/diff/1/?file=520039#file520039line72> > > > > what happens if zkClient is disconnected and how are the error > > conditions handled. Can we have some protection here so that it does not > > bombard ZK more than X writes per second? we should add throttling here. if zkconnection is down, we can't send any alerts to Helix controller. - Zhen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19240/#review37279 ----------------------------------------------------------- On March 14, 2014, 10:17 p.m., Zhen Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19240/ > ----------------------------------------------------------- > > (Updated March 14, 2014, 10:17 p.m.) > > > Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna. > > > Repository: helix-git > > > Description > ------- > > another refactoring based on last iteration: > 1) remove spectating on monitoring cluster, use a simple round-robin approach > to find the first available monitoring server > 2) add a sharding key in MonitoringEvent > 3) refactor AlertProxy, using direct java function call from clj instead of > jetty/http > > > Diffs > ----- > > helix-core/src/main/java/org/apache/helix/monitoring/MonitoringClient.java > 743f8b4 > helix-core/src/main/java/org/apache/helix/monitoring/MonitoringEvent.java > 80006fb > > helix-monitor-client/src/main/java/org/apache/helix/monitoring/ClientUtil.java > e69de29 > > helix-monitor-client/src/main/java/org/apache/helix/monitoring/RawRiemannClient.java > e69de29 > > helix-monitor-client/src/main/java/org/apache/helix/monitoring/RiemannClientWrapper.java > e69de29 > > helix-monitor-server/src/main/java/org/apache/helix/monitoring/RiemannAgent.java > 61e3d6c > > helix-monitor-server/src/main/java/org/apache/helix/monitoring/RiemannAlertProxy.java > 5fe8ebe > > helix-monitor-server/src/main/java/org/apache/helix/monitoring/RiemannConfigs.java > 193b763 > > helix-monitor-server/src/test/java/org/apache/helix/monitoring/IntegrationTest.java > 9d28dc3 > > helix-monitor-server/src/test/java/org/apache/helix/monitoring/MonitoringTestHelper.java > e69de29 > > helix-monitor-server/src/test/java/org/apache/helix/monitoring/TestClientServerMonitoring.java > 588bc35 > > helix-monitor-server/src/test/java/org/apache/helix/monitoring/TestRiemannAgent.java > 39ece64 > > helix-monitor-server/src/test/java/org/apache/helix/monitoring/TestRiemannAlertProxy.java > 700682a > > helix-monitor-server/src/test/java/org/apache/helix/monitoring/TestRiemannClientWrapper.java > e69de29 > > helix-monitor-server/src/test/java/org/apache/helix/monitoring/TestRiemannMonitoringServer.java > f7d0585 > > Diff: https://reviews.apache.org/r/19240/diff/ > > > Testing > ------- > > > Thanks, > > Zhen Zhang > >
