[
https://issues.apache.org/jira/browse/HBASE-5621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253909#comment-13253909
]
jirapos...@reviews.apache.org 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?
Let me move it to client.
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
Will move to client.
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?
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.
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?
I tried but it is hard. Let me think about it again.
bq. On 2012-04-13 23:59:40, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnection.java, line 223
bq. > <https://reviews.apache.org/r/4714/diff/1/?file=101784#file101784line223>
bq. >
bq. > Yeah, does this belong in here?
Will fix it.
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.
Let me think about it and fix it.
bq. On 2012-04-13 23:59:40, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java,
line 1376
bq. >
<https://reviews.apache.org/r/4714/diff/1/?file=101785#file101785line1376>
bq. >
bq. > Yeah, I wonder if this getClient and getAdmin should be @Overrides?
Shoudl they be methods of HCM altogether? Would that be better? OR should it
happen elsewhere? In HBaseAdmin or in HTable where we put together protocol
and connection?
Let me think about it and fix it.
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?
Some are moved to RegionServer as I did for 5620
- Jimmy
-----------------------------------------------------------
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
