[
https://issues.apache.org/jira/browse/HBASE-5621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253982#comment-13253982
]
[email protected] commented on HBASE-5621:
------------------------------------------------------
bq. On 2012-04-13 23:59:40, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/AdminProtocol.java, line 28
bq. > <https://reviews.apache.org/r/4714/diff/1/?file=101780#file101780line28>
bq. >
bq. > Is this right? It does more than talk to a regionserver? You have
to have really nice comments on your class now that you are at the top level
Jimmy (smile)
bq. >
bq. > This class is only used by HBaseAdmin? Is that right? Or do other
classes use it? If only HBaseAdmin, maybe it should not be top level?
bq.
bq. Jimmy Xiang wrote:
bq. Let me move it to client.
And it should not be public I'd say.
bq. On 2012-04-13 23:59:40, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ClientProtocol.java, line 29
bq. > <https://reviews.apache.org/r/4714/diff/1/?file=101781#file101781line29>
bq. >
bq. > Ditto
bq.
bq. Jimmy Xiang wrote:
bq. Will move to client.
Is this only used out of the client package? If so, yeah, move it there I'd
say.
bq. On 2012-04-13 23:59:40, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java, line
632
bq. > <https://reviews.apache.org/r/4714/diff/1/?file=101782#file101782line632>
bq. >
bq. > Do we have to have protobuf stuff sprinkled all over the code base?
Its not too bad but just wondering. Maybe we do but just wondering. I suppose
there is nothing to do about it. If we did same operation over and over w/
some pb stuff and the output was a non-pb object, then yes, we could hide the
pb stuff over in a class but what we do here is particular to this method....
Can't generalize.
bq. >
bq. > I do see that you have added some static methods into protobuf
utils. This one is not generic so doesn't deserve to go there?
bq.
bq. Jimmy Xiang wrote:
bq. For getRegionInfo, I used to put it in the protobuf util since it is
used lots of places. I will move it to protobuf util.
Agree. If used more than once, move it over to protobuf util otherwise I don't
see a way around not importing protobuf classes when its a particular usage.
bq. On 2012-04-13 23:59:40, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnection.java, line 32
bq. > <https://reviews.apache.org/r/4714/diff/1/?file=101784#file101784line32>
bq. >
bq. > Did you say you were going to remove these from here?
bq.
bq. Jimmy Xiang wrote:
bq. I tried but it is hard. Let me think about it again.
Even if this stuff just moved to HCM, it'd give us a clean HConnection.
bq. On 2012-04-13 23:59:40, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnection.java, line 260
bq. > <https://reviews.apache.org/r/4714/diff/1/?file=101784#file101784line260>
bq. >
bq. > ditto
bq. >
bq. > An AdminProtocol should have an HConnection?
bq. >
bq. > Even if you moved this up into HConnectionManager for now... that'd
be better.
bq.
bq. Jimmy Xiang wrote:
bq. Let me think about it and fix it.
Thanks.
bq. On 2012-04-13 23:59:40, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java,
line 2814
bq. >
<https://reviews.apache.org/r/4714/diff/1/?file=101799#file101799line2814>
bq. >
bq. > Where has allt his code gone?
bq.
bq. Jimmy Xiang wrote:
bq. Some are moved to RegionServer as I did for 5620
But I did not see it in the patch? Its already moved?
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4714/#review6922
-----------------------------------------------------------
On 2012-04-13 23:03:35, Jimmy Xiang wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4714/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-04-13 23:03:35)
bq.
bq.
bq. Review request for hbase.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This is the admin part of HBase-5443. AdminProtocol part.
bq.
bq.
bq. This addresses bug HBASE-5621.
bq. https://issues.apache.org/jira/browse/HBASE-5621
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/main/java/org/apache/hadoop/hbase/AdminProtocol.java PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/ClientProtocol.java PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java
408db79
bq. src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java ee16e72
bq. src/main/java/org/apache/hadoop/hbase/client/HConnection.java 23f8e5a
bq. src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
820e2a9
bq. src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
fe80fcf
bq. src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java 2a9d86e
bq. src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 3a3a79f
bq. src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java b7afa58
bq. src/main/java/org/apache/hadoop/hbase/ipc/RpcEngine.java dd0a1bc
bq. src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 9f159f2
bq. src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
ab33ac7
bq. src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 70901fe
bq. src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java
422e865
bq. src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java
3d6a23a
bq. src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java b056830
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
a912cc3
bq.
src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java
e78e56d
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
0d22c0e
bq. src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
9487a1c
bq.
src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
04fe8b6
bq. src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 61f8128
bq. src/main/java/org/apache/hadoop/hbase/util/HBaseFsckRepair.java ee4f556
bq. src/main/protobuf/Admin.proto 132c5dd
bq. src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java
d6ae0e2
bq.
src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java
3cfc02b
bq.
src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java
8af0f91
bq. src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java aa7f51b
bq. src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java
7dd60de
bq. src/test/java/org/apache/hadoop/hbase/client/TestHTableUtil.java 24f878e
bq.
src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java
301ee27
bq. src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java
a59e152
bq. src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
b84a115
bq. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
cedf31e
bq. src/test/java/org/apache/hadoop/hbase/master/TestMaster.java 3122b15
bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
c0ac12c
bq.
src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java
d0cad45
bq. src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java a06942d
bq.
bq. Diff: https://reviews.apache.org/r/4714/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. All unit tests passed.
bq.
bq.
bq. Thanks,
bq.
bq. Jimmy
bq.
bq.
> Convert admin protocol of HRegionInterface to PB
> ------------------------------------------------
>
> Key: HBASE-5621
> URL: https://issues.apache.org/jira/browse/HBASE-5621
> Project: HBase
> Issue Type: Sub-task
> Components: ipc, master, migration, regionserver
> Reporter: Jimmy Xiang
> Assignee: Jimmy Xiang
> Fix For: 0.96.0
>
>
--
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
