[
https://issues.apache.org/jira/browse/HBASE-6036?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13279276#comment-13279276
]
[email protected] 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
