[
https://issues.apache.org/jira/browse/HBASE-5935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13270199#comment-13270199
]
jirapos...@reviews.apache.org commented on HBASE-5935:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5060/#review7667
-----------------------------------------------------------
Looks good. Some small potatoes below. On this, "I've put the new generated
calls in HMasterInterface rather than a new class, because it is easier to have
one class for now, until I convert everything. This will only be temporary,
but let me know how you want to handle. Bump the version number?" ... when
will HMasterInterface go away? What else all is to be done? Sure on bump the
version number. Pretty soon they'll be ignored so no harm using up a few.
Good stuff G.
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/5060/#comment16879>
Is it right catching this here? You seem to handle it at a higher level up
in the move invocation? Won't this clause catch all the possible SEs?
Or I'm suffering myopia and there is more going on than just this call
invocation?
src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java
<https://reviews.apache.org/r/5060/#comment16885>
OK. Not even deprecated. Just removed. Like Jimmy's remove of
HRegionInterface and we can do this because these are internal APIs
src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java
<https://reviews.apache.org/r/5060/#comment16880>
It does seem a little messy having to hoist all these pb classes up in here
AND into HMasterInterface.
HMasterInterface is going to go away?
src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<https://reviews.apache.org/r/5060/#comment16883>
This seems like a big improvement, having an actual return.
src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<https://reviews.apache.org/r/5060/#comment16881>
FYI, spaces between if and open paren... and usually around operators.
Just FYI.
src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<https://reviews.apache.org/r/5060/#comment16882>
This needs paren (though its in the original)
src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<https://reviews.apache.org/r/5060/#comment16884>
All IOEs now need to get converted to a SE? I added an HBaseException
recently. Its a checked exception. Should we have an HBaseServiceExceptoin?
SE is pb?
src/main/protobuf/Master.proto
<https://reviews.apache.org/r/5060/#comment16886>
Great. Is this all that is in the master interface?
src/main/protobuf/RegionServerStatus.proto
<https://reviews.apache.org/r/5060/#comment16887>
Good
- Michael
On 2012-05-08 01:34:20, Gregory Chanan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/5060/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-05-08 01:34:20)
bq.
bq.
bq. Review request for hbase, Michael Stack and Jimmy Xiang.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Converted all the region-level calls in HMasterInterface to use Protobufs
(move, assign, unassign).
bq.
bq. I've put the new generated calls in HMasterInterface rather than a new
class, because it is easier to have one class for now, until I convert
everything. This will only be temporary, but let me know how you want to
handle. Bump the version number?
bq.
bq.
bq. This addresses bug HBASE-5935.
bq. https://issues.apache.org/jira/browse/HBASE-5935
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java ce81547
bq. src/main/java/org/apache/hadoop/hbase/master/HMaster.java ccc7119
bq. src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 5d4be3f
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
a5e03e1
bq.
src/main/java/org/apache/hadoop/hbase/protobuf/generated/MasterProtos.java
PRE-CREATION
bq. src/main/protobuf/Master.proto PRE-CREATION
bq. src/main/protobuf/RegionServerStatus.proto 9d7728f
bq. src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java e5de603
bq.
src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java
83297cc
bq.
bq. Diff: https://reviews.apache.org/r/5060/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Gregory
bq.
bq.
> Add Region-level PB-based calls to HMasterInterface
> ---------------------------------------------------
>
> Key: HBASE-5935
> URL: https://issues.apache.org/jira/browse/HBASE-5935
> Project: HBase
> Issue Type: Task
> Components: ipc, master, migration
> Reporter: Gregory Chanan
> Assignee: Gregory Chanan
> Fix For: 0.96.0
>
>
> This should be a subtask of HBASE-5445, but since that is a subtask, I can't
> also make this a subtask (apparently).
> This is for converting the region-level calls, i.e.:
> moveRegion
> assignRegion
> unassignRegion
--
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
