hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-5621) Convert admin protocol of HRegionInterface to PB
Date Sat, 14 Apr 2012 00:31:18 GMT

    [ https://issues.apache.org/jira/browse/HBASE-5621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253909#comment-13253909
] 

jiraposter@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

        

Mime
View raw message