You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Sijie Guo <gu...@gmail.com> on 2012/01/11 02:02:38 UTC

Review Request: BOOKKEEPER-95: extends zookeeper JMX to monitor and manage bookie server

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

Review request for bookkeeper.


Summary
-------

we can extends/reuses zookeeper JMX until to monitor and manage bookie server


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


Re: Review Request: BOOKKEEPER-95: extends zookeeper JMX to monitor and manage bookie server

Posted by Sijie Guo <gu...@gmail.com>.

> 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
> 
>


Re: Review Request: BOOKKEEPER-95: extends zookeeper JMX to monitor and manage bookie server

Posted by Ivan Kelly <iv...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3451/#review4424
-----------------------------------------------------------


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.


bookkeeper-server/src/main/java/org/apache/bookkeeper/jmx/BKMBeanRegistry.java
<https://reviews.apache.org/r/3451/#comment9935>

    Why is this static? 


- Ivan


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
> 
>


Re: Review Request: BOOKKEEPER-95: extends zookeeper JMX to monitor and manage bookie server

Posted by Sijie Guo <gu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3451/
-----------------------------------------------------------

(Updated 2012-01-11 01:04:57.992380)


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