[
https://issues.apache.org/jira/browse/HBASE-5621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253893#comment-13253893
]
jirapos...@reviews.apache.org commented on HBASE-5621:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4714/#review6922
-----------------------------------------------------------
A few questions below... Looks great.
src/main/java/org/apache/hadoop/hbase/AdminProtocol.java
<https://reviews.apache.org/r/4714/#comment15405>
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)
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?
src/main/java/org/apache/hadoop/hbase/ClientProtocol.java
<https://reviews.apache.org/r/4714/#comment15406>
Ditto
src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java
<https://reviews.apache.org/r/4714/#comment15407>
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.
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?
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/4714/#comment15408>
Great stuff.
src/main/java/org/apache/hadoop/hbase/client/HConnection.java
<https://reviews.apache.org/r/4714/#comment15409>
Did you say you were going to remove these from here?
src/main/java/org/apache/hadoop/hbase/client/HConnection.java
<https://reviews.apache.org/r/4714/#comment15410>
Yeah, does this belong in here?
src/main/java/org/apache/hadoop/hbase/client/HConnection.java
<https://reviews.apache.org/r/4714/#comment15411>
ditto
An AdminProtocol should have an HConnection?
Even if you moved this up into HConnectionManager for now... that'd be
better.
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4714/#comment15412>
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?
src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java
<https://reviews.apache.org/r/4714/#comment15413>
Hurray!
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4714/#comment15414>
Where has allt his code gone?
- Michael
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
