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