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-5620) Convert the client protocol of HRegionInterface to PB
Date Wed, 11 Apr 2012 06:22:30 GMT

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

jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6833
-----------------------------------------------------------


I got as far as HTable.  Still have more to go.  This is some pretty great work Jimmy.  This
stuff is hard.  One thought I was having that is a little related (but it can be safely ignored)
is what if there were only one method on an HRegionServer and you gave it an array of Gets,
Puts, Delete, Increment, pb messages and you just let the regionserver sort it out.  is that
even posssible?  I'll do rest of review tomorrow.


src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<https://reviews.apache.org/r/4629/#comment15218>

    I said it before but I like this code removal



src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/4629/#comment15219>

    What about the EOFE you added today?  Does that need to go in here anywhere?



src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/4629/#comment15220>

    So, to be clear, if a client does not close a scanner now, how does it get cleaned up
on the server?  Does it live forever?



src/main/java/org/apache/hadoop/hbase/client/HConnection.java
<https://reviews.apache.org/r/4629/#comment15221>

    Does this feel right Jimmy?  Seems odd asking an HConnection for a ClientProtocol?  It
feels like a ClientProtocol should have an HConnection?  Or that we'd put together a ClientProtocol
+ an HConnection for it to use in a different object altogether.  What you think?
    
    Maybe this is a question for another client implementation that we do later?   But would
be interested if you had any ideas on this.



src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment15222>

    Rename to hbase.clientprotocol.class and CLIENT_PROTOCOL_CLASS.  Seems more consistent?



src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment15223>

    Rename too accordingly.



src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment15225>

    This is pb VersionedProtocol?



src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment15224>

    Want to adjust this message?



src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment15226>

    This set of protocols is covered by the synchronize held above? i.e. all access on protocols
are under a block like this?



src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment15227>

    Is there a difference here?  We create an ISA only when we need it previously?  Now we
create it always?  Is there a diff here?


- Michael


On 2012-04-07 21:30:32, Jimmy Xiang wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/4629/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-04-07 21:30:32)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  This is the client protocol part of region interface. The admin protocol part will be
done in a different jira.
bq.  
bq.  The HRegionInterface is still there since the admin part is not done yet.  The other
reason is that in case some people still wants the old interface
bq.  
bq.  Filters, comparators and coprocessor parameters are still Writable.  They will be addressed
in different jiras.
bq.  
bq.  The existing client interface is not changed so that we don't break existing clients.
bq.  
bq.  
bq.  This addresses bug HBASE-5620.
bq.      https://issues.apache.org/jira/browse/HBASE-5620
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java 0129ee9 
bq.    src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java 97293aa 
bq.    src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 16e4017 
bq.    src/main/java/org/apache/hadoop/hbase/client/HConnection.java 5d43086 
bq.    src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 0b783ce 
bq.    src/main/java/org/apache/hadoop/hbase/client/HTable.java aa7652f 
bq.    src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java 9903df3 
bq.    src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java ddcf9ad 
bq.    src/main/java/org/apache/hadoop/hbase/filter/ParseConstants.java 1acbdab 
bq.    src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489 
bq.    src/main/java/org/apache/hadoop/hbase/io/TimeRange.java d135393 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java 05ae717 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 1316457 
bq.    src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 19ae18c

bq.    src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 2eb57de 
bq.    src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 4026da0 
bq.    src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionAdminProtos.java 2169310

bq.    src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0

bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 8a61f7d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 703e73d

bq.    src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java b520f3f 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java PRE-CREATION 
bq.    src/main/protobuf/Admin.proto PRE-CREATION 
bq.    src/main/protobuf/Client.proto PRE-CREATION 
bq.    src/main/protobuf/RegionAdmin.proto c64d68b 
bq.    src/main/protobuf/RegionClient.proto 358382b 
bq.    src/main/protobuf/hbase.proto da78788 
bq.    src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java c7284dc 
bq.    src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java 0042468

bq.    src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java e34d8bc

bq.    src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3 
bq.    src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 41616c8 
bq.    src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 6ed4ba2 
bq.    src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java b4dcb83 
bq.    src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java ceca6f5 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java cac2989 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java a1bf73b

bq.  
bq.  Diff: https://reviews.apache.org/r/4629/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  "mvn -PrunAllTests clean test" is green, except some flaky tests which need to run again.
bq.  
bq.  Also tested it on a real cluster with ycsb and bigtop.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Jimmy
bq.  
bq.


                
> Convert the client protocol of HRegionInterface to PB
> -----------------------------------------------------
>
>                 Key: HBASE-5620
>                 URL: https://issues.apache.org/jira/browse/HBASE-5620
>             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