> On Feb. 25, 2014, 7:19 a.m., Kanak Biscuitwala wrote:
> > helix-monitor-client/src/main/java/org/apache/helix/monitoring/RiemannMonitoringClient.java,
> >  line 262
> > <https://reviews.apache.org/r/18455/diff/1/?file=503198#file503198line262>
> >
> >     resource.hashCode() works too

fixed


> On Feb. 25, 2014, 7:19 a.m., Kanak Biscuitwala wrote:
> > helix-monitor-client/src/main/java/org/apache/helix/monitoring/RiemannMonitoringClient.java,
> >  line 135
> > <https://reviews.apache.org/r/18455/diff/1/?file=503198#file503198line135>
> >
> >     Typo: _monitoringServicePartitionNum

fixed


> On Feb. 25, 2014, 7:19 a.m., Kanak Biscuitwala wrote:
> > helix-monitor-client/src/main/java/org/apache/helix/monitoring/RiemannMonitoringClient.java,
> >  line 111
> > <https://reviews.apache.org/r/18455/diff/1/?file=503198#file503198line111>
> >
> >     Can you make this configurable?

fixed


> On Feb. 25, 2014, 7:19 a.m., Kanak Biscuitwala wrote:
> > helix-core/src/main/java/org/apache/helix/monitoring/MonitoringClient.java, 
> > line 47
> > <https://reviews.apache.org/r/18455/diff/1/?file=503193#file503193line47>
> >
> >     I would probably prefer MonitoringEvent to have a getResource(), but I 
> > could go either way.

Since we have to add a ResourceId argument to MonitoringClient#every(), it 
would be more consistent to have ResourceId argument to MonitoringClient#send() 
methods.


> On Feb. 25, 2014, 7:19 a.m., Kanak Biscuitwala wrote:
> > helix-monitor-client/src/main/java/org/apache/helix/monitoring/RiemannMonitoringClient.java,
> >  line 93
> > <https://reviews.apache.org/r/18455/diff/1/?file=503198#file503198line93>
> >
> >     Any way to keep this ZK-independent?

we need to create a spectator inside the client. if we have something like 
HelixConnection that abstracts the back store layer, we can make it 
zk-independent. for now, we might have to live with it


- Zhen


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


On Feb. 25, 2014, 6:01 a.m., Zhen Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18455/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2014, 6:01 a.m.)
> 
> 
> Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
> 
> 
> Repository: helix-git
> 
> 
> Description
> -------
> 
> [HELIX-319] refactor MonitoringClient to accommodate distributed monitoring 
> server
> 
> 
> Diffs
> -----
> 
>   helix-core/src/main/java/org/apache/helix/HelixAutoController.java fdab2a6 
>   helix-core/src/main/java/org/apache/helix/HelixController.java 098fd96 
>   helix-core/src/main/java/org/apache/helix/HelixManager.java 17c94e5 
>   
> helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java
>  65a192a 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java 
> 83eba53 
>   
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixAutoController.java
>  3967ed9 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java 
> 9da59b9 
>   
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixLeaderElection.java
>  3c5b3db 
>   helix-core/src/main/java/org/apache/helix/monitoring/MonitoringClient.java 
> a794eab 
>   helix-core/src/main/java/org/apache/helix/monitoring/MonitoringServer.java 
> 3168bb2 
>   helix-core/src/test/java/org/apache/helix/Mocks.java 87d4e68 
>   
> helix-core/src/test/java/org/apache/helix/controller/stages/DummyClusterManager.java
>  e07f0b5 
>   
> helix-core/src/test/java/org/apache/helix/participant/MockZKHelixManager.java 
> 0b8395e 
>   
> helix-monitor-client/src/main/java/org/apache/helix/monitoring/RiemannMonitoringClient.java
>  20b0825 
>   
> helix-monitor-server/src/main/java/org/apache/helix/monitoring/RiemannMonitoringServer.java
>  36719aa 
>   
> helix-monitor-server/src/test/java/org/apache/helix/monitoring/BasicMonitoringTest.java
>  6680f00 
>   
> helix-monitor-server/src/test/java/org/apache/helix/monitoring/TestClientServerMonitoring.java
>  8b7f839 
> 
> Diff: https://reviews.apache.org/r/18455/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhen Zhang
> 
>

Reply via email to