[
https://issues.apache.org/jira/browse/HBASE-5620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13249041#comment-13249041
]
[email protected] 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
