hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-4070) [Coprocessors] Improve region server metrics to report loaded coprocessors to master
Date Thu, 29 Sep 2011 22:43:47 GMT

    [ https://issues.apache.org/jira/browse/HBASE-4070?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13117694#comment-13117694
] 

jiraposter@reviews.apache.org commented on HBASE-4070:
------------------------------------------------------


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



src/main/java/org/apache/hadoop/hbase/HServerLoad.java
<https://reviews.apache.org/r/2029/#comment5082>

    an extra indent?



src/main/java/org/apache/hadoop/hbase/HServerLoad.java
<https://reviews.apache.org/r/2029/#comment5083>

    Just a naming suggestion for walCoprocessors=>coprocessors. Although right now WALObserver
is the only RS level CP, but it'd better not to indicate here.



src/main/java/org/apache/hadoop/hbase/HServerLoad.java
<https://reviews.apache.org/r/2029/#comment5085>

    getSimpleName() only returns the class name w/o package info. I think we should use the
whole class name for the comparator. 



src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/2029/#comment5092>

    do we really need to provide it thru HTAdmin? if we do, the returned value is not correct
in case of exception. 



src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java
<https://reviews.apache.org/r/2029/#comment5087>

    The new method is a little odd. how about just getCoprocessors(). 



src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<https://reviews.apache.org/r/2029/#comment5091>

    still the method name?



src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/2029/#comment5096>

    same as HMasterInterface. can we just generateCoprocessorString() => String getCoprocessors()?



src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/2029/#comment5097>

    is it okay just reuse the existing serverLoad, without rebuilding the whole thing? 
    
    this.serverInfo.getLoad()
    
    but it may not be a big issue. 



src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
<https://reviews.apache.org/r/2029/#comment5098>

    The file TestCoprocessorEndpoint is particularly for testing cp endpoint. It's odd to
put the new test cases to this file. Maybe TestClassLoading is a better place. 



src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
<https://reviews.apache.org/r/2029/#comment5100>

    according the previous override comparator in HRL, the cps should be in alphabet order.
so you don't need to compare this 2 strings. 


- Mingjie


On 2011-09-29 20:15:12, Eugene Koontz wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2029/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-09-29 20:15:12)
bq.  
bq.  
bq.  Review request for hbase and Mingjie Lai.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Proposed fix for HBASE-4070. 
bq.  
bq.  
bq.  This addresses bug HBASE-4070.
bq.      https://issues.apache.org/jira/browse/HBASE-4070
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/jamon/org/apache/hbase/tmpl/master/MasterStatusTmpl.jamon abeb850 
bq.    src/main/jamon/org/apache/hbase/tmpl/regionserver/RSStatusTmpl.jamon be6fceb 
bq.    src/main/java/org/apache/hadoop/hbase/HServerLoad.java 0c680e4 
bq.    src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java a55a4b1 
bq.    src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java dbae4fd 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java a069400 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java 270f3f3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 96b763b 
bq.    src/main/ruby/hbase/admin.rb b244ffe 
bq.    src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java a8f2a9c

bq.  
bq.  Diff: https://reviews.apache.org/r/2029/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Two new tests : testRegionServerCoprocessorReported() and testMasterServerCoprocessorsReported()
added to src/test/java/o.a.h.h/coprocessor/TestCoprocessorEndpoint.java.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Eugene
bq.  
bq.


                
> [Coprocessors] Improve region server metrics to report loaded coprocessors to master
> ------------------------------------------------------------------------------------
>
>                 Key: HBASE-4070
>                 URL: https://issues.apache.org/jira/browse/HBASE-4070
>             Project: HBase
>          Issue Type: Improvement
>    Affects Versions: 0.90.3
>            Reporter: Mingjie Lai
>            Assignee: Eugene Koontz
>         Attachments: HBASE-4070.patch, HBASE-4070.patch, master-web-ui.jpg, rs-status-web-ui.jpg
>
>
> HBASE-3512 is about listing loaded cp classes at shell. To make it more generic, we need
a way to report this piece of information from region to master (or just at region server
level). So later on, we can display the loaded class names at shell as well as web console.


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message