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