> On 2012-01-17 17:12:20, Ivan Kelly wrote:
> > The bulk of the code in this patch could be removed if zookeeper's 
> > ZKMBeanRegistry was updated to allow easy extension. Have you looked at 
> > changing the ZK code? Of course, we'd need to continue to use the code in 
> > this patch until a version of zookeeper with the change is released, but i 
> > think it would make things cleaner.

yes. updating zookeeper MBeanRegistry would be better.

currently a hard coded DOMAIN is used in zookeeper MBeanRegistry#makeObjectName.

so it would be easy to extends MBeanRegistry#makeObjectName(path, bean) to 
MBeanRegistry#makeObjectName(domain, path, bean) to make things easier.


> On 2012-01-17 17:12:20, Ivan Kelly wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/jmx/BKMBeanRegistry.java,
> >  line 39
> > <https://reviews.apache.org/r/3451/diff/1/?file=67674#file67674line39>
> >
> >     Why is this static?

the code follows what zookeeper MBeanRegistry did. it tried to make 
MBeanRegistry a singleton. so it return instance in a static method 
#getInstance.


- Sijie


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


On 2012-01-11 01:04:57, Sijie Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3451/
> -----------------------------------------------------------
> 
> (Updated 2012-01-11 01:04:57)
> 
> 
> Review request for bookkeeper.
> 
> 
> Summary
> -------
> 
> we can extends/reuses zookeeper JMX until to monitor and manage bookie server
> 
> 
> This addresses bug BOOKKEEPER-95.
>     https://issues.apache.org/jira/browse/BOOKKEEPER-95
> 
> 
> Diffs
> -----
> 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/jmx/BKMBeanInfo.java 
> PRE-CREATION 
>   
> bookkeeper-server/src/main/java/org/apache/bookkeeper/jmx/BKMBeanRegistry.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3451/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sijie
> 
>

Reply via email to