hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-6036) Add Cluster-level PB-based calls to HMasterInterface (minus file-format related calls)
Date Fri, 18 May 2012 22:45:09 GMT

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

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


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


Patch looks good.  Minor nits below.  See what you think.


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

    So, its ok changing the public-facing API because 0.96 is going to be the singularity?



src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/5157/#comment17327>

    FYI, convention is space after the comma -- its easier to read.



src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/5157/#comment17328>

    You should write this as
    
    LOG.info("Checking master connection", e);
    
    Should it be warn?



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

    Why we take it if unused?



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

    Nit: You should look at the javadoc generated by this markup.  You'll see that it comes
out nothing like how you have it here formatted.  For future.



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

    Two spaces in hbase and hadoop for 'tab' .  This 'return' is 4 or 6 spaces over?



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

    White space



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

    No need of the controller?  Would we ever need it?  If not, don't pass it?



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

    ditto



src/main/protobuf/Master.proto
<https://reviews.apache.org/r/5157/#comment17335>

    Should this be MasterRunningRequest rather than IsMasterRunningRequest?
    
    Or, is it just that you have a pattern going here where the Messages match the rpc in
name?
    
    If the latter, thats good enough for me.



src/main/protobuf/Master.proto
<https://reviews.apache.org/r/5157/#comment17336>

    This method and message name is awkward.  To match your IsMasterRunning, this should be
IsBalancer?


- Michael


On 2012-05-17 20:33:52, Gregory Chanan wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/5157/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-05-17 20:33:52)
bq.  
bq.  
bq.  Review request for hbase and Michael Stack.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Convert the cluster-level calls that do not touch the file-format related calls (see
HBASE-5453). These are:
bq.  IsMasterRunning
bq.  Shutdown
bq.  StopMaster
bq.  Balance
bq.  LoadBalancerIs (was synchronousBalanceSwitch/balanceSwitch)
bq.  
bq.  
bq.  This addresses bug HBASE-6036.
bq.      https://issues.apache.org/jira/browse/HBASE-6036
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 007d90b 
bq.    src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 5cac9af 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java 80c2165 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java 16ac781 
bq.    src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java 4348d20 
bq.    src/main/java/org/apache/hadoop/hbase/protobuf/generated/MasterProtos.java 944e403

bq.    src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 87a7a06 
bq.    src/main/protobuf/Master.proto PRE-CREATION 
bq.    src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java be52644 
bq.    src/test/java/org/apache/hadoop/hbase/master/TestHMasterRPCException.java 9ff83c5 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java
a24f937 
bq.  
bq.  Diff: https://reviews.apache.org/r/5157/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Ran unit tests, all passed.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Gregory
bq.  
bq.


                
> Add Cluster-level PB-based calls to HMasterInterface (minus file-format related calls)
> --------------------------------------------------------------------------------------
>
>                 Key: HBASE-6036
>                 URL: https://issues.apache.org/jira/browse/HBASE-6036
>             Project: HBase
>          Issue Type: Task
>          Components: ipc, master, migration
>            Reporter: Gregory Chanan
>            Assignee: Gregory Chanan
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6036.patch
>
>
> This should be a subtask of HBASE-5445, but since that is a subtask, I can't also make
this a subtask (apparently).
> Convert the cluster-level calls that do not touch the file-format related calls (see
HBASE-5453).  These are:
> IsMasterRunning
> Shutdown
> StopMaster
> Balance
> LoadBalancerIs (was synchronousBalanceSwitch/balanceSwitch)

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