You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Jimmy Xiang (Created) (JIRA)" <ji...@apache.org> on 2012/03/22 18:58:22 UTC
[jira] [Created] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
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
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13255704#comment-13255704 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
@Stack, not every invocation will throw an exception. In case it throws an exception, it should be a ServiceException for pb. It used to be IOException. Without the change, for pb calls, it won't get a ServiceException in case something goes wrong. It gets an undeclared exception
with the cause to be an IOE, and the upper layer doesn't know how to handle it.
The Set in Invocation is used to decide if a protocol a pb one, so ServiceException should be used. I put it there because it is
used for both WritableRpcEngine and SecureRpcEngine.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Todd Lipcon (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13262176#comment-13262176 ]
Todd Lipcon commented on HBASE-5620:
------------------------------------
Hey Jimmy. Any difference on throughput?
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252197#comment-13252197 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6868
-----------------------------------------------------------
What you want to do Jimmy? On next iteration, if all tests pass, should we check it in? I think its missing unit tests for the new protobuf utility. We should add those. I'd be fine adding them in a separate patch if that would suit you. This thing is already massive. We could commit to a branch too but that probably doesn't get us far; you have your own setup where you are to which you can commit, right?
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
<https://reviews.apache.org/r/4629/#comment15293>
Fix comment.
Should you explain your intent for this class too? That HRegionServer will wither away? I think it woud be useful since it was not plain to me at first but after you explained it, I was on your side.
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
<https://reviews.apache.org/r/4629/#comment15294>
Shouldn't this implement Server and RegionServerServices?
I would love to take the time to break this class up into smaller pieces but thats not for this JIRA. Can do that after this goes in.
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
<https://reviews.apache.org/r/4629/#comment15295>
Should this class implement Closeable, Stoppable and Abortable?
src/main/protobuf/Client.proto
<https://reviews.apache.org/r/4629/#comment15297>
Call this keyValueBytes then so method names give more of a clue as to what you'll be getting; save on the surprises?
- 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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Todd Lipcon (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13254961#comment-13254961 ]
Todd Lipcon commented on HBASE-5620:
------------------------------------
Did anyone do any benchmarks here? It concerns me to have this committed without even knowing how it affects performance.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252208#comment-13252208 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > 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.
bq.
bq. Jimmy Xiang wrote:
bq. Thanks for reviewing. As to one method to handle all actions, it is supported by the multi call. It is too complex to match the response.
bq.
bq. Michael Stack wrote:
bq. Sorry, I don't get the last part? The response is too hard to make when the query is lots of types?
bq.
bq. Jimmy Xiang wrote:
bq. Right, basically the code is not efficient.
Whats your thoughts on making it efficient? Is it possible?
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6833
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252619#comment-13252619 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-12 00:03:44, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java, line 187
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100889#file100889line187>
bq. >
bq. > Do these have unit tests? Looks like it would be easy enough to add and could find some nasty bugs early.
bq.
bq. Jimmy Xiang wrote:
bq. Not for now, I can add it in a separate patch if it is ok with you.
Yes. Make it a blocker though. I see unit tests finding bugs in this stuff.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6863
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Hadoop QA (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252990#comment-13252990 ]
Hadoop QA commented on HBASE-5620:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12522499/hbase-5620_v4.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 30 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
-1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.regionserver.wal.TestHLogSplit
org.apache.hadoop.hbase.replication.TestMultiSlaveReplication
org.apache.hadoop.hbase.regionserver.wal.TestHLog
org.apache.hadoop.hbase.replication.TestMasterReplication
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1501//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1501//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1501//console
This message is automatically generated.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253799#comment-13253799 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
@Stack, I will move ClientProtocol.java and AdminProtocol.java to top level in HBASE-5621 since they are common. I added HBASE-5785 to track the unit test issue.
@Ted, can I check the licenses without doing a release build?
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252936#comment-13252936 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/
-----------------------------------------------------------
(Updated 2012-04-12 22:33:53.615686)
Review request for hbase.
Changes
-------
Minor code change. Fixed a bug.
Summary
-------
This is the client protocol part of region interface. The admin protocol part will be done in a different jira.
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
Filters, comparators and coprocessor parameters are still Writable. They will be addressed in different jiras.
The existing client interface is not changed so that we don't break existing clients.
This addresses bug HBASE-5620.
https://issues.apache.org/jira/browse/HBASE-5620
Diffs (updated)
-----
src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java 0129ee9
src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java 3167f23
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 16e4017
src/main/java/org/apache/hadoop/hbase/client/HConnection.java 5d43086
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 0b783ce
src/main/java/org/apache/hadoop/hbase/client/HTable.java aa7652f
src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java 9903df3
src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java ddcf9ad
src/main/java/org/apache/hadoop/hbase/filter/ParseConstants.java 1acbdab
src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489
src/main/java/org/apache/hadoop/hbase/io/TimeRange.java d135393
src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java 05ae717
src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0
src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 0573c68
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java b71ae66
src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 2eb57de
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 4026da0
src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionAdminProtos.java 2169310
src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 9b34e61
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 703e73d
src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java b520f3f
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java PRE-CREATION
src/main/protobuf/Admin.proto PRE-CREATION
src/main/protobuf/Client.proto PRE-CREATION
src/main/protobuf/RegionAdmin.proto c64d68b
src/main/protobuf/RegionClient.proto 358382b
src/main/protobuf/hbase.proto da78788
src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java c7284dc
src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java 0042468
src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java e34d8bc
src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3
src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 41616c8
src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 6ed4ba2
src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java b4dcb83
src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java ceca6f5
src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java cac2989
src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java a1bf73b
Diff: https://reviews.apache.org/r/4629/diff
Testing
-------
"mvn -PrunAllTests clean test" is green, except some flaky tests which need to run again.
Also tested it on a real cluster with ycsb and bigtop.
Thanks,
Jimmy
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Hadoop QA (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253169#comment-13253169 ]
Hadoop QA commented on HBASE-5620:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12522531/hbase-5620_v4.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 30 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
-1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.io.hfile.TestForceCacheImportantBlocks
org.apache.hadoop.hbase.replication.TestReplication
org.apache.hadoop.hbase.mapreduce.TestWALPlayer
org.apache.hadoop.hbase.regionserver.wal.TestHLogSplit
org.apache.hadoop.hbase.replication.TestMultiSlaveReplication
org.apache.hadoop.hbase.regionserver.wal.TestHLog
org.apache.hadoop.hbase.replication.TestMasterReplication
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1508//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1508//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1508//console
This message is automatically generated.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13256897#comment-13256897 ]
Hudson commented on HBASE-5620:
-------------------------------
Integrated in HBase-TRUNK #2781 (See [https://builds.apache.org/job/HBase-TRUNK/2781/])
HBASE-5810 HBASE-5620 Convert the client protocol of HRegionInterface to PB addendum (Revision 1327629)
Result = FAILURE
stack :
Files :
* /hbase/trunk/security/src/main/java/org/apache/hadoop/hbase/ipc/SecureRpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/RpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5620:
-------------------------------
Attachment: hbase-5620_v4.patch
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253538#comment-13253538 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
TestWALPlayer passed for me. I didn't have the latest from trunk?
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252617#comment-13252617 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-12 04:50:15, Michael Stack wrote:
bq. > What you want to do Jimmy? On next iteration, if all tests pass, should we check it in? I think its missing unit tests for the new protobuf utility. We should add those. I'd be fine adding them in a separate patch if that would suit you. This thing is already massive. We could commit to a branch too but that probably doesn't get us far; you have your own setup where you are to which you can commit, right?
Could you please check it in if all tests pass? It can save me some rebasing/merging efforts. :)
I can add unit tests for the new protobuf utility in a separate patch. I do have my own setup to commit my changes. I am also working on HBASE-5621.
I can resolve other issues in HBASE-5621, or separate patches.
bq. On 2012-04-12 04:50:15, Michael Stack wrote:
bq. > src/main/protobuf/Client.proto, line 56
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100900#file100900line56>
bq. >
bq. > Call this keyValueBytes then so method names give more of a clue as to what you'll be getting; save on the surprises?
Sure, will fix.
bq. On 2012-04-12 04:50:15, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 143
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100898#file100898line143>
bq. >
bq. > Should this class implement Closeable, Stoppable and Abortable?
RegionServerServices defined Stoppable, Abortable. Now RegionServer implements RegionServerServices.
Not sure about Closeable.
bq. On 2012-04-12 04:50:15, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 98
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100898#file100898line98>
bq. >
bq. > Shouldn't this implement Server and RegionServerServices?
bq. >
bq. > I would love to take the time to break this class up into smaller pieces but thats not for this JIRA. Can do that after this goes in.
RegionServer now implements RegionServerServices which extends Server.
It will be great to break this class up.
bq. On 2012-04-12 04:50:15, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 94
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100898#file100898line94>
bq. >
bq. > Fix comment.
bq. >
bq. > Should you explain your intent for this class too? That HRegionServer will wither away? I think it woud be useful since it was not plain to me at first but after you explained it, I was on your side.
Sure, added comments. Thanks.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6868
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253708#comment-13253708 ]
stack commented on HBASE-5620:
------------------------------
It passed for me. Let me commit this monster.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13257264#comment-13257264 ]
Hudson commented on HBASE-5620:
-------------------------------
Integrated in HBase-TRUNK-security #175 (See [https://builds.apache.org/job/HBase-TRUNK-security/175/])
HBASE-5810 HBASE-5620 Convert the client protocol of HRegionInterface to PB addendum (Revision 1327629)
Result = FAILURE
stack :
Files :
* /hbase/trunk/security/src/main/java/org/apache/hadoop/hbase/ipc/SecureRpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/RpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13249075#comment-13249075 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-06 23:41:33, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ipc/RegionProtocols.java, line 36
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98829#file98829line36>
bq. >
bq. > Where is RegionProtocols used? We put the two protocols together? Do we need to?
Ok, let me remove it.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6759
-----------------------------------------------------------
On 2012-04-03 23:32:10, 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-03 23:32:10)
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/HConstants.java 21ac4ba
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 aa30969
bq. src/main/java/org/apache/hadoop/hbase/client/HTable.java 3db0295
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/RegionProtocols.java PRE-CREATION
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/ProtobufUtil.java 2eb57de
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionClientProtocol.java PRE-CREATION
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/HBaseProtos.java 4026da0
bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 4f80999
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/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 d2b3060
bq. src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 91dce36
bq. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 227c5f2
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13251719#comment-13251719 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > 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.
Thanks for reviewing. As to one method to handle all actions, it is supported by the multi call. It is too complex to match the response.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java, line 691
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100870#file100870line691>
bq. >
bq. > I said it before but I like this code removal
It is not used so I removed it.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 533
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100872#file100872line533>
bq. >
bq. > What about the EOFE you added today? Does that need to go in here anywhere?
EOFE is an IOException. Good catch. I addressed it in HBASE-5621, which is in testing now.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 574
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100872#file100872line574>
bq. >
bq. > 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?
In this case, the Leases daemon will expire the scanner and clean it up.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 184
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100874#file100874line184>
bq. >
bq. > Rename to hbase.clientprotocol.class and CLIENT_PROTOCOL_CLASS. Seems more consistent?
Will fix.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnection.java, line 235
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100873#file100873line235>
bq. >
bq. > 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?
bq. >
bq. > Maybe this is a question for another client implementation that we do later? But would be interested if you had any ideas on this.
I agree a ClientProtocol should have an HConnection. We can hide the HConnection. Given a region location, we can create a ClientProtocol which manages its own HConnection.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 187
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100874#file100874line187>
bq. >
bq. > Rename too accordingly.
Will fix.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 565
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100874#file100874line565>
bq. >
bq. > This is pb VersionedProtocol?
So far, it can be any VersionedProtocol: ClientProtocol or HRegionInterface.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 621
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100874#file100874line621>
bq. >
bq. > Want to adjust this message?
Fixed.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 1440
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100874#file100874line1440>
bq. >
bq. > This set of protocols is covered by the synchronize held above? i.e. all access on protocols are under a block like this?
Good point. Will fix.
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 1449
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100874#file100874line1449>
bq. >
bq. > 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?
HServerAddress is deprecated. We are not going to get an ISA passed in. The protocol is cached so we don't need to create it many times.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6833
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Zhihong Yu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253802#comment-13253802 ]
Zhihong Yu commented on HBASE-5620:
-----------------------------------
Here is the command Jenkins used:
{code}
[trunk] $ /home/hudson/tools/maven/latest3/bin/mvn -e -X clean -Dmaven.test.redirectTestOutputToFile=true site install assembly:single -DskipITs -Prelease
{code}
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252072#comment-13252072 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6863
-----------------------------------------------------------
I'm halfway down on second page....
src/main/java/org/apache/hadoop/hbase/client/HTable.java
<https://reviews.apache.org/r/4629/#comment15282>
Is there an EOFE story here too? You are dealing w/ that elsewhere?
src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
<https://reviews.apache.org/r/4629/#comment15283>
Yeah, we were going to try and get the data up from rpc layer
src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
<https://reviews.apache.org/r/4629/#comment15284>
Does this call need to return a Result or do we need to add a new method to support being able to get results as part of the call to open a scanner?
src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java
<https://reviews.apache.org/r/4629/#comment15285>
Fix this text
src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java
<https://reviews.apache.org/r/4629/#comment15286>
This is the right comment?
src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
<https://reviews.apache.org/r/4629/#comment15287>
Do these have unit tests? Looks like it would be easy enough to add and could find some nasty bugs early.
- 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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253975#comment-13253975 ]
stack commented on HBASE-5620:
------------------------------
@Jimmy I think this is the biggest patch ever applied to HBase. Congrats!
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ 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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252613#comment-13252613 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-12 00:03:44, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java, line 187
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100889#file100889line187>
bq. >
bq. > Do these have unit tests? Looks like it would be easy enough to add and could find some nasty bugs early.
Not for now, I can add it in a separate patch if it is ok with you.
bq. On 2012-04-12 00:03:44, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java, line 28
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100885#file100885line28>
bq. >
bq. > This is the right comment?
Fixed.
bq. On 2012-04-12 00:03:44, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java, line 57
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100882#file100882line57>
bq. >
bq. > Fix this text
Fixed.
bq. On 2012-04-12 00:03:44, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java, line 190
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100876#file100876line190>
bq. >
bq. > Yeah, we were going to try and get the data up from rpc layer
Yes, after changed pb Result to use key value bytes, we should be able to get the data now.
bq. On 2012-04-12 00:03:44, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HTable.java, line 672
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100875#file100875line672>
bq. >
bq. > Is there an EOFE story here too? You are dealing w/ that elsewhere?
EOFE is also an IOE. It should be covered as before.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6863
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Zhihong Yu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13255554#comment-13255554 ]
Zhihong Yu commented on HBASE-5620:
-----------------------------------
@Stack:
Do you think the addendum is ready to be integrated ?
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253714#comment-13253714 ]
stack commented on HBASE-5620:
------------------------------
Mind opening new issues Jimmy to do outstanding work like unit tests?
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5620:
-------------------------------
Attachment: hbase-5620_v3.patch
> 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
>
> Attachments: hbase-5620_v3.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253377#comment-13253377 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
I will take a look at the test failures.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5620:
-------------------------------
Resolution: Fixed
Status: Resolved (was: Patch Available)
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13249041#comment-13249041 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6757
-----------------------------------------------------------
I missed this second page. Looks great. Some radical stuff going on in here. Can you explain some? Good on you Jimmy.
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
<https://reviews.apache.org/r/4629/#comment14844>
What does this do? Needs class comment
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
<https://reviews.apache.org/r/4629/#comment14845>
Spelling
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
<https://reviews.apache.org/r/4629/#comment14846>
We only do the latter if the flag is set, right?
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
<https://reviews.apache.org/r/4629/#comment14847>
Is this a get request builder? Name method getRequestBuilder?
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
<https://reviews.apache.org/r/4629/#comment14848>
Do you think that we should eventually just deprecate the current Get, Delete, and Put, etc. and move to the pb ones altogether? They are only in the way now?
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
<https://reviews.apache.org/r/4629/#comment14849>
buildMutateRequest?
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
<https://reviews.apache.org/r/4629/#comment14850>
Should this stuff be public? Its internal stuff? Should it be over under the client package? Then you could keep it encapsulated at least w/i the package? Is it used outside of the client package?
src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
<https://reviews.apache.org/r/4629/#comment14851>
Ditto. Should this be over in the client package? Is that the only place its used? Could we make it non-public?
src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
<https://reviews.apache.org/r/4629/#comment14852>
buildActionResult?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14864>
We don't need this anymore?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14863>
Why we don't need this anymore?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14862>
We don't need this anymore?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14861>
We don't need any more?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14860>
We don't need this anymore?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14859>
For sure these are no longer needed?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14858>
Not needed anymore?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14857>
Not used anymore?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14856>
Why this no more?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14855>
Why this no more?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14854>
No more of this?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14865>
Ain't this needed?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<https://reviews.apache.org/r/4629/#comment14866>
Some pretty radical removals in here. For sure we don't need the stuff being removed (Otherwise, I'm glad to see it go)
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
<https://reviews.apache.org/r/4629/#comment14867>
oh... I think I like what I'm seeing. Could this be an Interface at all?
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
<https://reviews.apache.org/r/4629/#comment14868>
What is the advantage of doing this?
src/main/protobuf/RegionClient.proto
<https://reviews.apache.org/r/4629/#comment14869>
We don't need these?
src/main/protobuf/hbase.proto
<https://reviews.apache.org/r/4629/#comment14870>
Good
- Michael
On 2012-04-03 23:32:10, 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-03 23:32:10)
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/HConstants.java 21ac4ba
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 aa30969
bq. src/main/java/org/apache/hadoop/hbase/client/HTable.java 3db0295
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/RegionProtocols.java PRE-CREATION
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/ProtobufUtil.java 2eb57de
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionClientProtocol.java PRE-CREATION
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/HBaseProtos.java 4026da0
bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 4f80999
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/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 d2b3060
bq. src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 91dce36
bq. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 227c5f2
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13254979#comment-13254979 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
I did some testing with YCSB (mostly inserts). It gave me better performance for the patch which was a surprise to me.
I will do some read-only testing with YCSB too.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
stack updated HBASE-5620:
-------------------------
Status: Open (was: Patch Available)
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13249338#comment-13249338 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/
-----------------------------------------------------------
(Updated 2012-04-07 21:30:32.906966)
Review request for hbase.
Changes
-------
Addressed Stack's comments. For KeyValue, I removed it from the pb Result.
Summary
-------
This is the client protocol part of region interface. The admin protocol part will be done in a different jira.
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
Filters, comparators and coprocessor parameters are still Writable. They will be addressed in different jiras.
The existing client interface is not changed so that we don't break existing clients.
This addresses bug HBASE-5620.
https://issues.apache.org/jira/browse/HBASE-5620
Diffs (updated)
-----
src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java 0129ee9
src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java 97293aa
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 16e4017
src/main/java/org/apache/hadoop/hbase/client/HConnection.java 5d43086
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 0b783ce
src/main/java/org/apache/hadoop/hbase/client/HTable.java aa7652f
src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java 9903df3
src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java ddcf9ad
src/main/java/org/apache/hadoop/hbase/filter/ParseConstants.java 1acbdab
src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489
src/main/java/org/apache/hadoop/hbase/io/TimeRange.java d135393
src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java 05ae717
src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0
src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 1316457
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 19ae18c
src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 2eb57de
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 4026da0
src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionAdminProtos.java 2169310
src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 8a61f7d
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 703e73d
src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java b520f3f
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java PRE-CREATION
src/main/protobuf/Admin.proto PRE-CREATION
src/main/protobuf/Client.proto PRE-CREATION
src/main/protobuf/RegionAdmin.proto c64d68b
src/main/protobuf/RegionClient.proto 358382b
src/main/protobuf/hbase.proto da78788
src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java c7284dc
src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java 0042468
src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java e34d8bc
src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3
src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 41616c8
src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 6ed4ba2
src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java b4dcb83
src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java ceca6f5
src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java cac2989
src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java a1bf73b
Diff: https://reviews.apache.org/r/4629/diff
Testing
-------
"mvn -PrunAllTests clean test" is green, except some flaky tests which need to run again.
Also tested it on a real cluster with ycsb and bigtop.
Thanks,
Jimmy
> 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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13262216#comment-13262216 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
You are right. I used just one thread. This can give us a clear picture on pb rpc overhead, right?
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13249077#comment-13249077 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java, line 89
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98835#file98835line89>
bq. >
bq. > What does this do? Needs class comment
Will add comment.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java, line 97
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98835#file98835line97>
bq. >
bq. > Spelling
Good catch, will fix.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java, line 98
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98835#file98835line98>
bq. >
bq. > We only do the latter if the flag is set, right?
Right.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java, line 107
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98835#file98835line107>
bq. >
bq. > Is this a get request builder? Name method getRequestBuilder?
It uses a request builder to build a get request.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java, line 146
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98835#file98835line146>
bq. >
bq. > Do you think that we should eventually just deprecate the current Get, Delete, and Put, etc. and move to the pb ones altogether? They are only in the way now?
I think so. We don't want to break the existing applications.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java, line 167
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98835#file98835line167>
bq. >
bq. > buildMutateRequest?
Sounds good to me.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java, line 646
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98835#file98835line646>
bq. >
bq. > Should this stuff be public? Its internal stuff? Should it be over under the client package? Then you could keep it encapsulated at least w/i the package? Is it used outside of the client package?
Let me check. I will try not to expose it.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java, line 56
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98836#file98836line56>
bq. >
bq. > Ditto. Should this be over in the client package? Is that the only place its used? Could we make it non-public?
Will do if it works.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java, line 101
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98836#file98836line101>
bq. >
bq. > buildActionResult?
Sure.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 185
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line185>
bq. >
bq. > We don't need this anymore?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 193
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line193>
bq. >
bq. > Why we don't need this anymore?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 218
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line218>
bq. >
bq. > We don't need this anymore?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 229
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line229>
bq. >
bq. > We don't need any more?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 271
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line271>
bq. >
bq. > We don't need this anymore?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 459
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line459>
bq. >
bq. > For sure these are no longer needed?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 1119
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line1119>
bq. >
bq. > Not needed anymore?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 1155
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line1155>
bq. >
bq. > Not used anymore?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 1181
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line1181>
bq. >
bq. > Why this no more?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 1199
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line1199>
bq. >
bq. > Why this no more?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 2518
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line2518>
bq. >
bq. > No more of this?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 3094
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line3094>
bq. >
bq. > Ain't this needed?
Moved to RegionServer, a new base class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 3195
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line3195>
bq. >
bq. > Some pretty radical removals in here. For sure we don't need the stuff being removed (Otherwise, I'm glad to see it go)
Moved to RegionServer, a new base class. I was think to remove class HRegionServer later on when HRegionInterface is not needed.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 98
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98842#file98842line98>
bq. >
bq. > oh... I think I like what I'm seeing. Could this be an Interface at all?
It is the new pb version of HRegionServer. So it is a class.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/protobuf/RegionClient.proto, line 150
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98843#file98843line150>
bq. >
bq. > We don't need these?
I added DeleteType to handle such usecases.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 1162
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98842#file98842line1162>
bq. >
bq. > What is the advantage of doing this?
So I don't mess HRegionInterface with the new pb. Later on, it will be easier to remove HRegionInterface.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6757
-----------------------------------------------------------
On 2012-04-03 23:32:10, 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-03 23:32:10)
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/HConstants.java 21ac4ba
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 aa30969
bq. src/main/java/org/apache/hadoop/hbase/client/HTable.java 3db0295
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/RegionProtocols.java PRE-CREATION
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/ProtobufUtil.java 2eb57de
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionClientProtocol.java PRE-CREATION
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/HBaseProtos.java 4026da0
bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 4f80999
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/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 d2b3060
bq. src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 91dce36
bq. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 227c5f2
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5620:
-------------------------------
Status: Open (was: Patch Available)
> 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
>
> Attachments: hbase-5620_v3.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253974#comment-13253974 ]
stack commented on HBASE-5620:
------------------------------
@Jimmy I think this is the biggest patch ever applied to HBase. Congrats!
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5620:
-------------------------------
Attachment: hbase-5620-sec.patch
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13249384#comment-13249384 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java, line 146
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98835#file98835line146>
bq. >
bq. > Do you think that we should eventually just deprecate the current Get, Delete, and Put, etc. and move to the pb ones altogether? They are only in the way now?
bq.
bq. Jimmy Xiang wrote:
bq. I think so. We don't want to break the existing applications.
No. As you suggest, parity first. Thinking on it, I'm thinking that when we write the new client, that it is the new client that just does pbs direct.
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 3195
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98839#file98839line3195>
bq. >
bq. > Some pretty radical removals in here. For sure we don't need the stuff being removed (Otherwise, I'm glad to see it go)
bq.
bq. Jimmy Xiang wrote:
bq. Moved to RegionServer, a new base class. I was think to remove class HRegionServer later on when HRegionInterface is not needed.
Ok. Would be really cool if there was a RegionServer Interface that the pb service implemented (We already have Server Interface and RegionServerServices Interface).
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 98
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98842#file98842line98>
bq. >
bq. > oh... I think I like what I'm seeing. Could this be an Interface at all?
bq.
bq. Jimmy Xiang wrote:
bq. It is the new pb version of HRegionServer. So it is a class.
Could it be an Interface that the pb Service implements? Is that over the top?
bq. On 2012-04-06 23:26:21, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 1162
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98842#file98842line1162>
bq. >
bq. > What is the advantage of doing this?
bq.
bq. Jimmy Xiang wrote:
bq. So I don't mess HRegionInterface with the new pb. Later on, it will be easier to remove HRegionInterface.
I like this tactic of yours
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6757
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Hadoop QA (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13254215#comment-13254215 ]
Hadoop QA commented on HBASE-5620:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12522687/hbase-5620-sec.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
-1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.io.hfile.TestForceCacheImportantBlocks
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1527//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1527//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1527//console
This message is automatically generated.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
stack updated HBASE-5620:
-------------------------
Status: Patch Available (was: Open)
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13262193#comment-13262193 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
Yes, the throughput is also degraded. Without the patch, it is 2941 ops/sec; with the patch it is 2725 ops/sec, about 7.4% degrade.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Todd Lipcon (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13262203#comment-13262203 ]
Todd Lipcon commented on HBASE-5620:
------------------------------------
That sounds like you only ran YCSB with 1 thread...? 1 sec / 2725 = 366us.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Reopened] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Zhihong Yu (Reopened) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Zhihong Yu reopened HBASE-5620:
-------------------------------
As you can see in https://builds.apache.org/view/G-L/view/HBase/job/HBase-TRUNK-security/171/, there were 12 more test failures.
To narrow the checkin that caused additional test failures, I locally backed out HBASE-5684 and HBASE-5747. TestAccessControlFilter#testQualifierAccess still failed with access permission error.
After backing out this JIRA, the test passed.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Zhihong Yu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13254258#comment-13254258 ]
Zhihong Yu commented on HBASE-5620:
-----------------------------------
Addendum makes sense.
TestAccessControlFilter passes with the addendum.
Whole security test suite should be run.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253712#comment-13253712 ]
stack commented on HBASE-5620:
------------------------------
I think src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java is in wrong package. Ditto for AdminProtocol. What you think Jimmy? Should we move them? Where should they go? At top level? Or into client package?
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13249006#comment-13249006 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > Good stuff Jimmy. A few comments below. Sorry I ramble at times. Read to the end before reacting because I change my mind as I work through the patch. You forgot to add RequestConverter?
Thanks for reviewing. RequestConverter is the 21st file. There are more files on page 2. Please review them too. Thanks.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 766
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98832#file98832line766>
bq. >
bq. > white space
Will remove.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java, line 37
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98833#file98833line37>
bq. >
bq. > Is this just regionadmin protocol? Or is it general admin?
Should I change it to AdminProtocol? There is no other admin.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 510
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98832#file98832line510>
bq. >
bq. > Its as though we should give clients a way to pass in the raw pbs rather than have them go via our versions of Get, Delete, etc.
I agree. We can add some new methods later. For now, I'd like to make the existing functions work at first.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 160
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98832#file98832line160>
bq. >
bq. > Ugh. KeyValue needs to be an Interface that both pb and our current KV implements.
bq. >
bq. > This will be a performance killer? Should Result let out the raw pb KV?
I was thinking to has some new methods which use raw pbs, which old clients should migrate to.
Let me change the current KeyValue class to an interface.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 149
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98832#file98832line149>
bq. >
bq. > Why not throw ServiceException?
I like to throw ServiceException too. Let me give it a try. If the existing interface is not affected, I will do it.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java, line 418
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98830#file98830line418>
bq. >
bq. > Is ServiceException a pb exception?
It is from pb. But it is not a pb message as I know.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ipc/RegionProtocols.java, line 36
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98829#file98829line36>
bq. >
bq. > What is this for?
Region server could implement multiple protocols such as RegionClientProtocol and RegionAdminProtocol.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ipc/RegionProtocols.java, line 27
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98829#file98829line27>
bq. >
bq. > Fix
Fixed.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java, line 363
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98825#file98825line363>
bq. >
bq. > Do you have to? This stuff is nasty enough w/o lettting it out of this package.
It is to support Filters and comparators which are still Writable. We can change it back once Filters and comparators are moved to pb too.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java, line 230
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98822#file98822line230>
bq. >
bq. > scannerid is internal or something now?
I think it is still public as it is defined in the pb messages. For ScannerCallable, it is internal though.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java, line 192
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98822#file98822line192>
bq. >
bq. > I suppose HBaseClient down in rpc knows it because it has to deserialize the pb?
You are right. We need to come up a way to pop this info up.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 1441
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98820#file98820line1441>
bq. >
bq. > Does this need to be synchronized at all?
I think so. It's better to be synchronized. The connection is shared.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 1428
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98820#file98820line1428>
bq. >
bq. > Is this good? Should we have servername? What if server restarts in between? Would we want to notice that we don't have a connection to new instantiation and go create one?
In this case, they need to close the existing connection, and get a new one. Otherwise, there will be memory leak.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 606
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98820#file98820line606>
bq. >
bq. > Move these defines into this class rather than up in HConstants?
Ok, will do.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 1395
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98820#file98820line1395>
bq. >
bq. > Whats happening here? We want to drop the methods that took hostname and port only and move to those that take servername?
Just did some refactory so that we can use the same method to get a proxy for either a RegionClientProtocol and a HRegionInterface.
The public interface is not touched.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnection.java, line 43
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98819#file98819line43>
bq. >
bq. > Why do we need to pollute HConnection w/ higher level notions such as RegionClientProtocol (or ClientProtocol?).... hmmm ClientProtocol would be ok but not RegionClientProtocol...
OK, will rename it to ClientProtocol.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 530
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line530>
bq. >
bq. > Whats the NULL_CONTROLLER about?
Pb rpc needs a RpcController. Let me use null directly.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 574
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line574>
bq. >
bq. > So we don't close scanners anymore?
That's right, if closeScanner is specified in the request.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 525
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line525>
bq. >
bq. > Why we need a converter?
Pb needs some work to build a message. A converter is some helper to share some logic. The ProtobufUtil will be too big soon. That's why I have this separate converter.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 518
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line518>
bq. >
bq. > this should be getClient? We're going to get a client for a table? Maybe should be getTableClient?
The region client protocol. How about just getClient?
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 517
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line517>
bq. >
bq. > RegionClientProtocol is a bad name, no? Should it be just ClientProtocol and AdminProtocol?
Ok, will change to ClientProtocol and AdminProtocol.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/HConstants.java, line 661
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98815#file98815line661>
bq. >
bq. > Should these be in here? Can they be elsewhere, as defines in RegionClassProtocol?
Will move to HConnectionManager.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnection.java, line 234
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98819#file98819line234>
bq. >
bq. > I wonder if this needs to be public and in the Interface? Could it be an internal thing? How uses this thing that comes out? Maybe it needs to be here but sure seems like an internal thing: i.e. we pass hostname and port of a particular regionserver but then internally as we go about the cluster the client will go to new regionservers and set up connections....
bq. >
bq. > Looking around though, it seems that while most uses of this method should be shut down: e.g. over in catalog package and replaced by calls that go via HTable, there are a few places we need this still probably .. as in the master sendRegionClose, etc. calls. So you can't get rid of it.
bq. >
bq. > But this looks like it should be called RegionServerProtocol
bq. > or ClientProtocol and not RegionClientProtocol. It connects to a RegionServer not a Region.....
bq. >
bq. > Or what you think Jimmy? When I look at the RegionClient proto file, is it true to say that in each message, we MUST pass a region since we are going against a particular region... which would mean this is indeed a RegionClientProtocol (you just have to specify coordinates in two steps; 1. pass servername setting up connection, then 2. for any call on the connection, need to specify the region.....)
bq. >
bq. >
bq. >
Agreed to rename it to ClientProtocol.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6751
-----------------------------------------------------------
On 2012-04-03 23:32:10, 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-03 23:32:10)
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/HConstants.java 21ac4ba
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 aa30969
bq. src/main/java/org/apache/hadoop/hbase/client/HTable.java 3db0295
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/RegionProtocols.java PRE-CREATION
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/ProtobufUtil.java 2eb57de
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionClientProtocol.java PRE-CREATION
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/HBaseProtos.java 4026da0
bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 4f80999
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/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 d2b3060
bq. src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 91dce36
bq. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 227c5f2
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253852#comment-13253852 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
@Stack, thanks a lot!
I moved them to top-level in HBase-5621 and posted a review request. Could you please review?
I am ok to move them to client package.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Zhihong Yu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13262742#comment-13262742 ]
Zhihong Yu commented on HBASE-5620:
-----------------------------------
Is there plan to adopt various measures to counter this 8% performance dip ?
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13263742#comment-13263742 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
Yes, I was thinking to eliminate the conversion layer if possible, probably starting from the server side.
As to I performance test I did, I think the 27us overhead is more meaningful than the percentage number.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
stack updated HBASE-5620:
-------------------------
Status: Open (was: Patch Available)
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5620:
-------------------------------
Status: Patch Available (was: Open)
Minor code change. Fixed a bug.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252623#comment-13252623 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/
-----------------------------------------------------------
(Updated 2012-04-12 17:25:51.153610)
Review request for hbase.
Changes
-------
Addressed Stack's comments.
Summary
-------
This is the client protocol part of region interface. The admin protocol part will be done in a different jira.
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
Filters, comparators and coprocessor parameters are still Writable. They will be addressed in different jiras.
The existing client interface is not changed so that we don't break existing clients.
This addresses bug HBASE-5620.
https://issues.apache.org/jira/browse/HBASE-5620
Diffs (updated)
-----
src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java 0129ee9
src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java 3167f23
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 16e4017
src/main/java/org/apache/hadoop/hbase/client/HConnection.java 5d43086
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 0b783ce
src/main/java/org/apache/hadoop/hbase/client/HTable.java aa7652f
src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java 9903df3
src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java ddcf9ad
src/main/java/org/apache/hadoop/hbase/filter/ParseConstants.java 1acbdab
src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489
src/main/java/org/apache/hadoop/hbase/io/TimeRange.java d135393
src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java 05ae717
src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0
src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 0573c68
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java b71ae66
src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 2eb57de
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 4026da0
src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionAdminProtos.java 2169310
src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 9b34e61
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 703e73d
src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java b520f3f
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java PRE-CREATION
src/main/protobuf/Admin.proto PRE-CREATION
src/main/protobuf/Client.proto PRE-CREATION
src/main/protobuf/RegionAdmin.proto c64d68b
src/main/protobuf/RegionClient.proto 358382b
src/main/protobuf/hbase.proto da78788
src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java c7284dc
src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java 0042468
src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java e34d8bc
src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3
src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java 41616c8
src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 6ed4ba2
src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java b4dcb83
src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java ceca6f5
src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java cac2989
src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java a1bf73b
Diff: https://reviews.apache.org/r/4629/diff
Testing
-------
"mvn -PrunAllTests clean test" is green, except some flaky tests which need to run again.
Also tested it on a real cluster with ycsb and bigtop.
Thanks,
Jimmy
> 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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
stack updated HBASE-5620:
-------------------------
Attachment: (was: hbase-5620_v4.patch)
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13249054#comment-13249054 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6759
-----------------------------------------------------------
src/main/java/org/apache/hadoop/hbase/ipc/RegionProtocols.java
<https://reviews.apache.org/r/4629/#comment14879>
Where is RegionProtocols used? We put the two protocols together? Do we need to?
- Michael
On 2012-04-03 23:32:10, 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-03 23:32:10)
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/HConstants.java 21ac4ba
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 aa30969
bq. src/main/java/org/apache/hadoop/hbase/client/HTable.java 3db0295
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/RegionProtocols.java PRE-CREATION
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/ProtobufUtil.java 2eb57de
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionClientProtocol.java PRE-CREATION
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/HBaseProtos.java 4026da0
bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 4f80999
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/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 d2b3060
bq. src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 91dce36
bq. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 227c5f2
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252005#comment-13252005 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > 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.
bq.
bq. Jimmy Xiang wrote:
bq. Thanks for reviewing. As to one method to handle all actions, it is supported by the multi call. It is too complex to match the response.
Sorry, I don't get the last part? The response is too hard to make when the query is lots of types?
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java, line 691
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100870#file100870line691>
bq. >
bq. > I said it before but I like this code removal
bq.
bq. Jimmy Xiang wrote:
bq. It is not used so I removed it.
Great
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnection.java, line 235
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100873#file100873line235>
bq. >
bq. > 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?
bq. >
bq. > Maybe this is a question for another client implementation that we do later? But would be interested if you had any ideas on this.
bq.
bq. Jimmy Xiang wrote:
bq. I agree a ClientProtocol should have an HConnection. We can hide the HConnection. Given a region location, we can create a ClientProtocol which manages its own HConnection.
Sounds better
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6833
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252035#comment-13252035 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-11 06:22:10, Michael Stack wrote:
bq. > 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.
bq.
bq. Jimmy Xiang wrote:
bq. Thanks for reviewing. As to one method to handle all actions, it is supported by the multi call. It is too complex to match the response.
bq.
bq. Michael Stack wrote:
bq. Sorry, I don't get the last part? The response is too hard to make when the query is lots of types?
Right, basically the code is not efficient.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6833
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13255778#comment-13255778 ]
stack commented on HBASE-5620:
------------------------------
I made HBASE-5810 to apply this Jimmy. Good stuff.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Zhihong Yu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253781#comment-13253781 ]
Zhihong Yu commented on HBASE-5620:
-----------------------------------
Looks like trunk build failed with:
{code}
[ERROR] Failed to execute goal org.apache.rat:apache-rat-plugin:0.8:check (default) on project hbase: Too many unapproved licenses: 1 -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.rat:apache-rat-plugin:0.8:check (default) on project hbase: Too many unapproved licenses: 1
{code}
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253840#comment-13253840 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
I ran Apache Rat check like mvn apache-rat:check, and it is ok.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5620:
-------------------------------
Status: Patch Available (was: Reopened)
hbase-5620-sec.patch is to fix the security unit test failures.
When I ran all tests, I forgot the security part, which has a different RPC engine.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5620:
-------------------------------
Hadoop Flags: Reviewed
Status: Patch Available (was: Open)
> 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
>
> Attachments: hbase-5620_v3.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
stack updated HBASE-5620:
-------------------------
Attachment: hbase-5620_v4.patch
Retrying Jimmy's patch
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Hadoop QA (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253513#comment-13253513 ]
Hadoop QA commented on HBASE-5620:
----------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12522579/hbase-5620_v4.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 30 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
-1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.mapreduce.TestWALPlayer
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1511//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1511//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1511//console
This message is automatically generated.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13249382#comment-13249382 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 517
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line517>
bq. >
bq. > RegionClientProtocol is a bad name, no? Should it be just ClientProtocol and AdminProtocol?
bq.
bq. Jimmy Xiang wrote:
bq. Ok, will change to ClientProtocol and AdminProtocol.
I think thats better.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 518
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line518>
bq. >
bq. > this should be getClient? We're going to get a client for a table? Maybe should be getTableClient?
bq.
bq. Jimmy Xiang wrote:
bq. The region client protocol. How about just getClient?
Yes. In actuality, isn't this really a client on a region so getTableClient wouldn't be correct either as I suggest above.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 525
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line525>
bq. >
bq. > Why we need a converter?
bq.
bq. Jimmy Xiang wrote:
bq. Pb needs some work to build a message. A converter is some helper to share some logic. The ProtobufUtil will be too big soon. That's why I have this separate converter.
Good. I saw that later on.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 530
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line530>
bq. >
bq. > Whats the NULL_CONTROLLER about?
bq.
bq. Jimmy Xiang wrote:
bq. Pb rpc needs a RpcController. Let me use null directly.
yeah...
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java, line 574
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98818#file98818line574>
bq. >
bq. > So we don't close scanners anymore?
bq.
bq. Jimmy Xiang wrote:
bq. That's right, if closeScanner is specified in the request.
oh, nice.
What about the case where no close is called. We'll hold on to the resources on the server side w/o cleanup?
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 1395
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98820#file98820line1395>
bq. >
bq. > Whats happening here? We want to drop the methods that took hostname and port only and move to those that take servername?
bq.
bq. Jimmy Xiang wrote:
bq. Just did some refactory so that we can use the same method to get a proxy for either a RegionClientProtocol and a HRegionInterface.
bq.
bq. The public interface is not touched.
ok. good.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line 1428
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98820#file98820line1428>
bq. >
bq. > Is this good? Should we have servername? What if server restarts in between? Would we want to notice that we don't have a connection to new instantiation and go create one?
bq.
bq. Jimmy Xiang wrote:
bq. In this case, they need to close the existing connection, and get a new one. Otherwise, there will be memory leak.
Otherwise, we will overwrite the old with the new?
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java, line 363
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98825#file98825line363>
bq. >
bq. > Do you have to? This stuff is nasty enough w/o lettting it out of this package.
bq.
bq. Jimmy Xiang wrote:
bq. It is to support Filters and comparators which are still Writable. We can change it back once Filters and comparators are moved to pb too.
ok
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ipc/RegionProtocols.java, line 36
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98829#file98829line36>
bq. >
bq. > What is this for?
bq.
bq. Jimmy Xiang wrote:
bq. Region server could implement multiple protocols such as RegionClientProtocol and RegionAdminProtocol.
Do we need to put them together? Who uses this RegionProtocols?
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 160
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98832#file98832line160>
bq. >
bq. > Ugh. KeyValue needs to be an Interface that both pb and our current KV implements.
bq. >
bq. > This will be a performance killer? Should Result let out the raw pb KV?
bq.
bq. Jimmy Xiang wrote:
bq. I was thinking to has some new methods which use raw pbs, which old clients should migrate to.
bq.
bq. Let me change the current KeyValue class to an interface.
That might be hard boss. But, could we implement a subset of KV as an Interface, enough of a subset that it'd make these pb stuff happy?
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 510
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98832#file98832line510>
bq. >
bq. > Its as though we should give clients a way to pass in the raw pbs rather than have them go via our versions of Get, Delete, etc.
bq.
bq. Jimmy Xiang wrote:
bq. I agree. We can add some new methods later. For now, I'd like to make the existing functions work at first.
Agreed. Looking at the later stuff I think Get, Delete, etc. should just go and we should have folks use the pb stuff.
bq. On 2012-04-06 21:02:20, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java, line 37
bq. > <https://reviews.apache.org/r/4629/diff/1/?file=98833#file98833line37>
bq. >
bq. > Is this just regionadmin protocol? Or is it general admin?
bq.
bq. Jimmy Xiang wrote:
bq. Should I change it to AdminProtocol? There is no other admin.
Yes
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6751
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253843#comment-13253843 ]
stack commented on HBASE-5620:
------------------------------
I did too and trunk is also no longer complaining. The rat test is a PITA. There was probably some deitrus laying around that it picked up. I modified the trunk build to keep the rat.txt report next time.
@Jimmy I think top-level is better than where it currently is. What other Protocols would go up to the top level? None I suppose. I suppose they should be in client package but its a little perverse having the the client stuff reaching into zk and util and protobuf...
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13254360#comment-13254360 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
Thanks for reviewing. Both regular test suite and security test suite are green for me. I mean all tests in the suite.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13255667#comment-13255667 ]
stack commented on HBASE-5620:
------------------------------
@Jimmy So every invocation will throw an exception?
{code}
+ // For protobuf protocols, ServiceException is expected
{code}
Whats the Set in Invocation doing? You add it but don't seem to access it?
I like the removal of a call method down through the rpc stack
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13254216#comment-13254216 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
TestForceCacheImportantBlocks is green for me.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13245888#comment-13245888 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/
-----------------------------------------------------------
Review request for hbase.
Summary
-------
This is the client protocol part of region interface. The admin protocol part will be done in a different jira.
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
Filters, comparators and coprocessor parameters are still Writable. They will be addressed in different jiras.
The existing client interface is not changed so that we don't break existing clients.
This addresses bug HBASE-5620.
https://issues.apache.org/jira/browse/HBASE-5620
Diffs
-----
src/main/java/org/apache/hadoop/hbase/HConstants.java 21ac4ba
src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java 0129ee9
src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java 97293aa
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java 16e4017
src/main/java/org/apache/hadoop/hbase/client/HConnection.java 5d43086
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java aa30969
src/main/java/org/apache/hadoop/hbase/client/HTable.java 3db0295
src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java 9903df3
src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java ddcf9ad
src/main/java/org/apache/hadoop/hbase/filter/ParseConstants.java 1acbdab
src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489
src/main/java/org/apache/hadoop/hbase/io/TimeRange.java d135393
src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java 05ae717
src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0
src/main/java/org/apache/hadoop/hbase/ipc/RegionProtocols.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 1316457
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 19ae18c
src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 2eb57de
src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/RegionClientProtocol.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java 4026da0
src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 4f80999
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 703e73d
src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java b520f3f
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java PRE-CREATION
src/main/protobuf/RegionClient.proto 358382b
src/main/protobuf/hbase.proto da78788
src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java c7284dc
src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java 0042468
src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java e34d8bc
src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3
src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java d2b3060
src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 91dce36
src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 227c5f2
src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java ceca6f5
src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java cac2989
src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java a1bf73b
Diff: https://reviews.apache.org/r/4629/diff
Testing
-------
"mvn -PrunAllTests clean test" is green, except some flaky tests which need to run again.
Also tested it on a real cluster with ycsb and bigtop.
Thanks,
Jimmy
> 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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13262017#comment-13262017 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
I did some testing with YCSB. I loaded 9000 rows to a table and read (Get) from it for 2000000 times.
The idea is to have less data so that all are in memory, so as to test the RPC overhead.
The performance number changes. On average, the average read latency is 335us without the patch, 362us with the patch.
It is about 27us (8%) degrade.
> 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
>
> Attachments: hbase-5620-sec.patch, hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
stack updated HBASE-5620:
-------------------------
Resolution: Fixed
Status: Resolved (was: Patch Available)
Committed to trunk. Thats some pretty heavy lifting you got going on in here Jimmy. Good stuff.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Hadoop QA (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252687#comment-13252687 ]
Hadoop QA commented on HBASE-5620:
----------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12522452/hbase-5620_v3.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 30 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1496//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1496//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1496//console
This message is automatically generated.
> 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
>
> Attachments: hbase-5620_v3.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13252622#comment-13252622 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
bq. On 2012-04-12 04:50:15, Michael Stack wrote:
bq. > What you want to do Jimmy? On next iteration, if all tests pass, should we check it in? I think its missing unit tests for the new protobuf utility. We should add those. I'd be fine adding them in a separate patch if that would suit you. This thing is already massive. We could commit to a branch too but that probably doesn't get us far; you have your own setup where you are to which you can commit, right?
bq.
bq. Jimmy Xiang wrote:
bq. Could you please check it in if all tests pass? It can save me some rebasing/merging efforts. :)
bq.
bq. I can add unit tests for the new protobuf utility in a separate patch. I do have my own setup to commit my changes. I am also working on HBASE-5621.
bq. I can resolve other issues in HBASE-5621, or separate patches.
bq.
I can. You going to do another version to address above feedback?
bq. On 2012-04-12 04:50:15, Michael Stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 143
bq. > <https://reviews.apache.org/r/4629/diff/2/?file=100898#file100898line143>
bq. >
bq. > Should this class implement Closeable, Stoppable and Abortable?
bq.
bq. Jimmy Xiang wrote:
bq. RegionServerServices defined Stoppable, Abortable. Now RegionServer implements RegionServerServices.
bq. Not sure about Closeable.
Good
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6868
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13248772#comment-13248772 ]
jiraposter@reviews.apache.org commented on HBASE-5620:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4629/#review6751
-----------------------------------------------------------
Good stuff Jimmy. A few comments below. Sorry I ramble at times. Read to the end before reacting because I change my mind as I work through the patch. You forgot to add RequestConverter?
src/main/java/org/apache/hadoop/hbase/HConstants.java
<https://reviews.apache.org/r/4629/#comment14787>
Should these be in here? Can they be elsewhere, as defines in RegionClassProtocol?
src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
<https://reviews.apache.org/r/4629/#comment14788>
I don't know how you get away with these removes but I'm glad to see them go -- anything that shrinks this catalog package is good by me
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/4629/#comment14789>
RegionClientProtocol is a bad name, no? Should it be just ClientProtocol and AdminProtocol?
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/4629/#comment14790>
this should be getClient? We're going to get a client for a table? Maybe should be getTableClient?
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/4629/#comment14791>
Why we need a converter?
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/4629/#comment14792>
Whats the NULL_CONTROLLER about?
src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<https://reviews.apache.org/r/4629/#comment14793>
So we don't close scanners anymore?
src/main/java/org/apache/hadoop/hbase/client/HConnection.java
<https://reviews.apache.org/r/4629/#comment14794>
Why do we need to pollute HConnection w/ higher level notions such as RegionClientProtocol (or ClientProtocol?).... hmmm ClientProtocol would be ok but not RegionClientProtocol...
src/main/java/org/apache/hadoop/hbase/client/HConnection.java
<https://reviews.apache.org/r/4629/#comment14795>
I wonder if this needs to be public and in the Interface? Could it be an internal thing? How uses this thing that comes out? Maybe it needs to be here but sure seems like an internal thing: i.e. we pass hostname and port of a particular regionserver but then internally as we go about the cluster the client will go to new regionservers and set up connections....
Looking around though, it seems that while most uses of this method should be shut down: e.g. over in catalog package and replaced by calls that go via HTable, there are a few places we need this still probably .. as in the master sendRegionClose, etc. calls. So you can't get rid of it.
But this looks like it should be called RegionServerProtocol
or ClientProtocol and not RegionClientProtocol. It connects to a RegionServer not a Region.....
Or what you think Jimmy? When I look at the RegionClient proto file, is it true to say that in each message, we MUST pass a region since we are going against a particular region... which would mean this is indeed a RegionClientProtocol (you just have to specify coordinates in two steps; 1. pass servername setting up connection, then 2. for any call on the connection, need to specify the region.....)
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment14796>
Move these defines into this class rather than up in HConstants?
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment14797>
Whats happening here? We want to drop the methods that took hostname and port only and move to those that take servername?
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment14798>
Is this good? Should we have servername? What if server restarts in between? Would we want to notice that we don't have a connection to new instantiation and go create one?
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<https://reviews.apache.org/r/4629/#comment14799>
Does this need to be synchronized at all?
src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
<https://reviews.apache.org/r/4629/#comment14800>
I suppose HBaseClient down in rpc knows it because it has to deserialize the pb?
src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
<https://reviews.apache.org/r/4629/#comment14801>
scannerid is internal or something now?
src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java
<https://reviews.apache.org/r/4629/#comment14802>
Not your fault but server is a bad variable name for what this is, huh..
src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java
<https://reviews.apache.org/r/4629/#comment14803>
Do you have to? This stuff is nasty enough w/o lettting it out of this package.
src/main/java/org/apache/hadoop/hbase/ipc/RegionProtocols.java
<https://reviews.apache.org/r/4629/#comment14804>
Fix
src/main/java/org/apache/hadoop/hbase/ipc/RegionProtocols.java
<https://reviews.apache.org/r/4629/#comment14805>
What is this for?
src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
<https://reviews.apache.org/r/4629/#comment14806>
Is ServiceException a pb exception?
src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
<https://reviews.apache.org/r/4629/#comment14807>
Why not throw ServiceException?
src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
<https://reviews.apache.org/r/4629/#comment14808>
Ugh. KeyValue needs to be an Interface that both pb and our current KV implements.
This will be a performance killer? Should Result let out the raw pb KV?
src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
<https://reviews.apache.org/r/4629/#comment14809>
Good one. I should use this in stuff I committed recently
src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
<https://reviews.apache.org/r/4629/#comment14810>
Its as though we should give clients a way to pass in the raw pbs rather than have them go via our versions of Get, Delete, etc.
src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
<https://reviews.apache.org/r/4629/#comment14811>
white space
src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java
<https://reviews.apache.org/r/4629/#comment14812>
Is this just regionadmin protocol? Or is it general admin?
src/main/java/org/apache/hadoop/hbase/protobuf/RegionClientProtocol.java
<https://reviews.apache.org/r/4629/#comment14813>
This is probably a good idea.
- Michael
On 2012-04-03 23:32:10, 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-03 23:32:10)
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/HConstants.java 21ac4ba
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 aa30969
bq. src/main/java/org/apache/hadoop/hbase/client/HTable.java 3db0295
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/RegionProtocols.java PRE-CREATION
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/ProtobufUtil.java 2eb57de
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionAdminProtocol.java PRE-CREATION
bq. src/main/java/org/apache/hadoop/hbase/protobuf/RegionClientProtocol.java PRE-CREATION
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/HBaseProtos.java 4026da0
bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java b36a9c0
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 4f80999
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/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 d2b3060
bq. src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 91dce36
bq. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 227c5f2
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
stack updated HBASE-5620:
-------------------------
Attachment: hbase-5620_v4.patch
Retry after backing out wal compression patch that was breaking a few tests. Will narrow the set of tests that need looking at.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Updated] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "stack (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
stack updated HBASE-5620:
-------------------------
Status: Patch Available (was: Open)
Retry against hadoopqa
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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
[jira] [Commented] (HBASE-5620) Convert the client protocol of
HRegionInterface to PB
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13254153#comment-13254153 ]
Jimmy Xiang commented on HBASE-5620:
------------------------------------
@Stack, thanks.
@Ted, I am looking into it now.
> 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
>
> Attachments: hbase-5620_v3.patch, hbase-5620_v4.patch, hbase-5620_v4.patch
>
>
--
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