[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268907#comment-13268907 ] Hudson commented on HBASE-5889: --- Integrated in HBase-TRUNK-security #192 (See [https://builds.apache.org/job/HBase-TRUNK-security/192/]) HBASE-5889 Remove HRegionInterface (Revision 1334314) Result = SUCCESS stack : Files : * /hbase/trunk/conf/hbase-policy.xml * /hbase/trunk/security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java * /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java * /hbase/trunk/src/main/protobuf/Admin.proto * /hbase/trunk/src/main/protobuf/RPC.proto * /hbase/trunk/src/main/resources/hbase-default.xml * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestRollingRestart.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestRSStatusServlet.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch, hbase_5889_v4.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- This message is automatically generated by JIRA. If you think it was sent incorrectly,
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268911#comment-13268911 ] Hudson commented on HBASE-5889: --- Integrated in HBase-TRUNK #2850 (See [https://builds.apache.org/job/HBase-TRUNK/2850/]) HBASE-5889 Remove HRegionInterface (Revision 1334314) Result = FAILURE stack : Files : * /hbase/trunk/conf/hbase-policy.xml * /hbase/trunk/security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java * /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java * /hbase/trunk/src/main/protobuf/Admin.proto * /hbase/trunk/src/main/protobuf/RPC.proto * /hbase/trunk/src/main/resources/hbase-default.xml * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestRollingRestart.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestRSStatusServlet.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch, hbase_5889_v4.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268526#comment-13268526 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- bq. On 2012-05-04 01:08:06, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 1295 bq. https://reviews.apache.org/r/4993/diff/1/?file=106388#file106388line1295 bq. bq. HRegionServer util might be better place for this yes. Or could they be put into a new class, RegionServerUtil in the regionserver package? Maybe they don't have to be public methods if done this way? bq. bq. Either sounds good to me boss. It is good to have a new class RegionServerUtil. Will do. - Jimmy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/#review7539 --- On 2012-05-03 17:27:50, Jimmy Xiang wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4993/ bq. --- bq. bq. (Updated 2012-05-03 17:27:50) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Removed HRegionInterface, and cleaned up the HRegionServer, moved pb code from RegionServer back to HRegionServer. bq. bq. The goal is to avoid two copies of region server code to maintain, and make it possible to avoid data type conversion in the sever side. bq. bq. Fixed some unit tests. Now all region server unit tests test the new pb functions. bq. bq. Enhanced getServerInfo so that it returns the webui port too. bq. bq. bq. This addresses bug HBASE-5889. bq. https://issues.apache.org/jira/browse/HBASE-5889 bq. bq. bq. Diffs bq. - bq. bq.conf/hbase-policy.xml e45f23c bq. security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java 0c4b4cb bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 87f04f4 bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java e3912c2 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java fc9176d bq.src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 757f98e bq.src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java cd9b528 bq.src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 79d5fdd bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 212ee3e bq.src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java d1e0993 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 81603af bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java fbf0127 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java db1333b bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java ae2094d bq.src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java 8b45f03 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 827fb23 bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 96ac8bd bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 4cb070e bq.src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java c2c89ea bq.src/main/protobuf/Admin.proto 2ad6fb0 bq.src/main/protobuf/RPC.proto 105fb3f bq.src/main/resources/hbase-default.xml f54b345 bq.src/main/resources/hbase-webapps/master/table.jsp ca7310c bq.src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java a1992c3 bq.src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java ad77e0a bq.src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 5574b7f bq.src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 3dfc94e bq. src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java 42092b7 bq.src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java c270e28 bq.src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java c36272f bq. src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java bdec3ee bq. src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java 7dbba66 bq. src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java 3acb988 bq. src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java eb546a5 bq.
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268600#comment-13268600 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/ --- (Updated 2012-05-04 18:16:37.788666) Review request for hbase. Changes --- Addressed Stack's comments. Summary --- Removed HRegionInterface, and cleaned up the HRegionServer, moved pb code from RegionServer back to HRegionServer. The goal is to avoid two copies of region server code to maintain, and make it possible to avoid data type conversion in the sever side. Fixed some unit tests. Now all region server unit tests test the new pb functions. Enhanced getServerInfo so that it returns the webui port too. This addresses bug HBASE-5889. https://issues.apache.org/jira/browse/HBASE-5889 Diffs (updated) - conf/hbase-policy.xml e45f23c security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java fda40cc src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 87f04f4 src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java 8a383e4 src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java e3912c2 src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 11d8bf9 src/main/java/org/apache/hadoop/hbase/client/HTable.java b8290e4 src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java 578b2b2 src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java fc9176d src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 757f98e src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 9e4ada9 src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java cd9b528 src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 79d5fdd src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 6ba8ab0 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 212ee3e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java d1e0993 src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 81603af src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java fbf0127 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java db1333b src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java ae2094d src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java 8b45f03 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 827fb23 src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 8c8381b src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 4cb070e src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java c2c89ea src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerUtil.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java 5050df0 src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 7b4f4a2 src/main/java/org/apache/hadoop/hbase/util/HBaseFsckRepair.java 9c3c9ef src/main/protobuf/Admin.proto 2ad6fb0 src/main/protobuf/RPC.proto 105fb3f src/main/resources/hbase-default.xml f54b345 src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java a1992c3 src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java ad77e0a src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 5574b7f src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 3dfc94e src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java 42092b7 src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java c270e28 src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java 0079b13 src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java c36272f src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java bdec3ee src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java 7dbba66 src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java 3acb988 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java eb546a5 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java ceba5cd src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java ec08b17 src/test/java/org/apache/hadoop/hbase/master/TestRollingRestart.java 30c6cf1 src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java 8c3f67e
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268628#comment-13268628 ] Hadoop QA commented on HBASE-5889: -- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525658/hbase-5889_v3.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 63 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +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 failed these unit tests: org.apache.hadoop.hbase.replication.TestReplication Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1769//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1769//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1769//console This message is automatically generated. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268666#comment-13268666 ] stack commented on HBASE-5889: -- Is this good to go in Jimmy? What comments did you address? Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268670#comment-13268670 ] Jimmy Xiang commented on HBASE-5889: Yes, I think it is good to go. I moved those utils like openRegion from ProtobufUtil to RegionServerUtil. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268732#comment-13268732 ] stack commented on HBASE-5889: -- The util is for serverside though? If not, if for client side, then yes, belongs in client package. Good on you Jimmy. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268781#comment-13268781 ] Jimmy Xiang commented on HBASE-5889: After a second thought, I used protobuf util instead since it is for the client side. For the sever side case you referred to (openRegion returns a OpenRegionState), I moved it out of the util. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch, hbase_5889_v4.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268785#comment-13268785 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/ --- (Updated 2012-05-04 22:54:46.398933) Review request for hbase. Summary --- Removed HRegionInterface, and cleaned up the HRegionServer, moved pb code from RegionServer back to HRegionServer. The goal is to avoid two copies of region server code to maintain, and make it possible to avoid data type conversion in the sever side. Fixed some unit tests. Now all region server unit tests test the new pb functions. Enhanced getServerInfo so that it returns the webui port too. This addresses bug HBASE-5889. https://issues.apache.org/jira/browse/HBASE-5889 Diffs (updated) - conf/hbase-policy.xml e45f23c security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java fda40cc src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 87f04f4 src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java e3912c2 src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java fc9176d src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 757f98e src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java cd9b528 src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 79d5fdd src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 6ba8ab0 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 212ee3e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java d1e0993 src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 81603af src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java fbf0127 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java db1333b src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java ae2094d src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java 8b45f03 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 827fb23 src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 8c8381b src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 4cb070e src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java c2c89ea src/main/protobuf/Admin.proto 2ad6fb0 src/main/protobuf/RPC.proto 105fb3f src/main/resources/hbase-default.xml f54b345 src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java a1992c3 src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java ad77e0a src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 5574b7f src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 3dfc94e src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java 42092b7 src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java c270e28 src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java c36272f src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java bdec3ee src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java 7dbba66 src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java 3acb988 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java eb546a5 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java ceba5cd src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java ec08b17 src/test/java/org/apache/hadoop/hbase/master/TestRollingRestart.java 30c6cf1 src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java 8c3f67e src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java 7bfe4cd src/test/java/org/apache/hadoop/hbase/regionserver/TestRSStatusServlet.java ffce7e8 src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java aa5ca37 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 65fa948 src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java fc4a0a7 Diff: https://reviews.apache.org/r/4993/diff Testing --- All regular and security profile tests are green before I rebased to the latest today. Thanks, Jimmy Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type:
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268805#comment-13268805 ] Hadoop QA commented on HBASE-5889: -- +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525693/hbase_5889_v4.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 60 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +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/1777//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1777//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1777//console This message is automatically generated. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch, hbase_5889_v4.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268806#comment-13268806 ] stack commented on HBASE-5889: -- I can't tell highlevel what you've done Jimmy? I see RegionServerUtil is missing. Where do you spread its content? I don't see obvious new util classes. Thanks boss. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch, hbase_5889_v4.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268816#comment-13268816 ] Jimmy Xiang commented on HBASE-5889: :) I thought to create new util class for client side. I used protobuf util instead since it is for the client side. For the sever side case you referred to (openRegion returns a OpenRegionState), I moved it out of the util to the original place since it's not shared. I changed the shared one so that it doesn't refer to region server directly. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch, hbase_5889_v4.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268895#comment-13268895 ] stack commented on HBASE-5889: -- I think this not the best soln. but I'm not going to argue for a better patch in here when this one is already fat and its gotten a +1 from hadoopqa. Lets get this in and work on polish in other issues. Good stuff Jimmy. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase-5889_v3.patch, hbase_5889.patch, hbase_5889_v2.patch, hbase_5889_v4.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267613#comment-13267613 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/ --- Review request for hbase. Summary --- Removed HRegionInterface, and cleaned up the HRegionServer, moved pb code from RegionServer back to HRegionServer. The goal is to avoid two copies of region server code to maintain, and make it possible to avoid data type conversion in the sever side. Fixed some unit tests. Now all region server unit tests test the new pb functions. Enhanced getServerInfo so that it returns the webui port too. This addresses bug HBASE-5889. https://issues.apache.org/jira/browse/HBASE-5889 Diffs - conf/hbase-policy.xml e45f23c security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java 0c4b4cb src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 87f04f4 src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java e3912c2 src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java fc9176d src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 757f98e src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java cd9b528 src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 79d5fdd src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 212ee3e src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java d1e0993 src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 81603af src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java fbf0127 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java db1333b src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java ae2094d src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java 8b45f03 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 827fb23 src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 96ac8bd src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 4cb070e src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java c2c89ea src/main/protobuf/Admin.proto 2ad6fb0 src/main/protobuf/RPC.proto 105fb3f src/main/resources/hbase-default.xml f54b345 src/main/resources/hbase-webapps/master/table.jsp ca7310c src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java a1992c3 src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java ad77e0a src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 5574b7f src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 3dfc94e src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java 42092b7 src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java c270e28 src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java c36272f src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java bdec3ee src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java 7dbba66 src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java 3acb988 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java eb546a5 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java ceba5cd src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java ec08b17 src/test/java/org/apache/hadoop/hbase/master/TestRollingRestart.java 30c6cf1 src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java 8c3f67e src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java 7bfe4cd src/test/java/org/apache/hadoop/hbase/regionserver/TestRSStatusServlet.java ffce7e8 src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java aa5ca37 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 65fa948 src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java fc4a0a7 Diff: https://reviews.apache.org/r/4993/diff Testing --- All regular and security profile tests are green before I rebased to the latest today. Thanks, Jimmy Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[
https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267645#comment-13267645
]
Zhihong Yu commented on HBASE-5889:
---
HRegionInterface is used by asynchbase:
{code}
writeHBaseString(buf, org.apache.hadoop.hbase.ipc.HRegionInterface);
final String klass = org.apache.hadoop.hbase.ipc.HRegionInterface;
./src/RegionClient.java
{code}
Should we start a discussion on dev@hbase to get wider feedback about the
roadmap for non-bundled (third-party) HBase client(s) ?
Remove HRegionInterface
---
Key: HBASE-5889
URL: https://issues.apache.org/jira/browse/HBASE-5889
Project: HBase
Issue Type: Improvement
Components: client, ipc, regionserver
Affects Versions: 0.96.0
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
Fix For: 0.96.0
Attachments: hbase_5889.patch
As a step to move internals to PB, so as to avoid the conversion for
performance reason, we should remove the HRegionInterface.
Therefore region server only supports ClientProtocol and AdminProtocol.
Later on, HRegion can work with PB messages directly.
--
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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267654#comment-13267654 ] Hadoop QA commented on HBASE-5889: -- +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525477/hbase_5889.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 60 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +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/1747//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1747//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1747//console This message is automatically generated. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase_5889.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267684#comment-13267684 ] Jimmy Xiang commented on HBASE-5889: @Ted, I posted a message to dev@hbase as suggested. I think it is to their benefits to migrate as well. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase_5889.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267881#comment-13267881 ] stack commented on HBASE-5889: -- Benoît knows about the coming pb conversion and is good w/ it. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase_5889.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267900#comment-13267900 ] Jimmy Xiang commented on HBASE-5889: @Stack, thanks, good to know. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase_5889.patch, hbase_5889_v2.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267917#comment-13267917 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/#review7528 --- Excellent Jimmy. High-level, too much has been moved to protobufutils IMO. Below I highlight where we have gone too far. What you think? Otherwise, these is not much of substance to my comments below. I'd be up for committing this patch before it rots and addressing issues raised in a new jira if thats what you'd prefer. security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java https://reviews.apache.org/r/4993/#comment16658 Anyone working on removal of HMasterInterface? Devaraj will need to pick up these changes if your patch goes in before his. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon https://reviews.apache.org/r/4993/#comment16659 Is this right after Gregory's commit last night? He hides pb Serverload inside a ServerLoad. Perhaps have to go to pb RegionLoad still even after his commit? src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon https://reviews.apache.org/r/4993/#comment16660 ok src/main/java/org/apache/hadoop/hbase/HConstants.java https://reviews.apache.org/r/4993/#comment16662 These are just not used? We used have a mechanism where you could subclass regionserver and then configure it so your subclass was started out on the cluster. You think that still possible? src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java https://reviews.apache.org/r/4993/#comment16663 Hurray! src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java https://reviews.apache.org/r/4993/#comment16664 Is it going too far adding methods like this into protobufutils? These methods seem pretty core hbase facility, too core to be out in a util method? You have them here because there is a bunch of pb'ing going on? I know I suggested that we move some of the transforms out here... but maybe this is taking it a bit far? What you reckon? src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java https://reviews.apache.org/r/4993/#comment16665 I think this kinda builder is a the right thing to have over in this util class. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java https://reviews.apache.org/r/4993/#comment1 Like I said to Gregory last night, its kinda hard hiding this pb stuff when you are in the class that is slinging them. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java https://reviews.apache.org/r/4993/#comment16667 Is this new or moved code? src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java https://reviews.apache.org/r/4993/#comment16668 This was no longer a good idea? src/main/protobuf/Admin.proto https://reviews.apache.org/r/4993/#comment16669 good src/main/resources/hbase-default.xml https://reviews.apache.org/r/4993/#comment16670 good src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java https://reviews.apache.org/r/4993/#comment16671 It is unexpected going to ProtobufUtils to get online regions. src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java https://reviews.apache.org/r/4993/#comment16672 ditto - Michael On 2012-05-03 17:27:50, Jimmy Xiang wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4993/ bq. --- bq. bq. (Updated 2012-05-03 17:27:50) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Removed HRegionInterface, and cleaned up the HRegionServer, moved pb code from RegionServer back to HRegionServer. bq. bq. The goal is to avoid two copies of region server code to maintain, and make it possible to avoid data type conversion in the sever side. bq. bq. Fixed some unit tests. Now all region server unit tests test the new pb functions. bq. bq. Enhanced getServerInfo so that it returns the webui port too. bq. bq. bq. This addresses bug HBASE-5889. bq. https://issues.apache.org/jira/browse/HBASE-5889 bq. bq. bq. Diffs bq. - bq. bq.conf/hbase-policy.xml e45f23c bq. security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java 0c4b4cb bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 87f04f4 bq.
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267928#comment-13267928 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java, line 38 bq. https://reviews.apache.org/r/4993/diff/1/?file=106380#file106380line38 bq. bq. Anyone working on removal of HMasterInterface? bq. bq. Devaraj will need to pick up these changes if your patch goes in before his. I'm working on the HMasterInterface. See HBASE-5445. bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon, line 41 bq. https://reviews.apache.org/r/4993/diff/1/?file=106381#file106381line41 bq. bq. Is this right after Gregory's commit last night? He hides pb Serverload inside a ServerLoad. Perhaps have to go to pb RegionLoad still even after his commit? There isn't currently a RegionLoad that hides pb RegionLoad in the way you've described with ServerLoad. I actually wrote one up, but didn't include it because there is an existing RegionLoad that is needed until the HMasterInterface is done. I felt it made more sense to wait rather than spend a bunch of time renaming stuff. If this patch is committed as is, let's file a JIRA and assign it to me? - Gregory --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/#review7528 --- On 2012-05-03 17:27:50, Jimmy Xiang wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4993/ bq. --- bq. bq. (Updated 2012-05-03 17:27:50) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Removed HRegionInterface, and cleaned up the HRegionServer, moved pb code from RegionServer back to HRegionServer. bq. bq. The goal is to avoid two copies of region server code to maintain, and make it possible to avoid data type conversion in the sever side. bq. bq. Fixed some unit tests. Now all region server unit tests test the new pb functions. bq. bq. Enhanced getServerInfo so that it returns the webui port too. bq. bq. bq. This addresses bug HBASE-5889. bq. https://issues.apache.org/jira/browse/HBASE-5889 bq. bq. bq. Diffs bq. - bq. bq.conf/hbase-policy.xml e45f23c bq. security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java 0c4b4cb bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 87f04f4 bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java e3912c2 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java fc9176d bq.src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 757f98e bq.src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java cd9b528 bq.src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 79d5fdd bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 212ee3e bq.src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java d1e0993 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 81603af bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java fbf0127 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java db1333b bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java ae2094d bq.src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java 8b45f03 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 827fb23 bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 96ac8bd bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 4cb070e bq.src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java c2c89ea bq.src/main/protobuf/Admin.proto 2ad6fb0 bq.src/main/protobuf/RPC.proto 105fb3f bq.src/main/resources/hbase-default.xml f54b345 bq.src/main/resources/hbase-webapps/master/table.jsp ca7310c bq.src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java a1992c3 bq.src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java ad77e0a bq.src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 5574b7f bq.src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267931#comment-13267931 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon, line 41 bq. https://reviews.apache.org/r/4993/diff/1/?file=106381#file106381line41 bq. bq. Is this right after Gregory's commit last night? He hides pb Serverload inside a ServerLoad. Perhaps have to go to pb RegionLoad still even after his commit? bq. bq. Gregory Chanan wrote: bq. There isn't currently a RegionLoad that hides pb RegionLoad in the way you've described with ServerLoad. I actually wrote one up, but didn't include it because there is an existing RegionLoad that is needed until the HMasterInterface is done. I felt it made more sense to wait rather than spend a bunch of time renaming stuff. bq. bq. If this patch is committed as is, let's file a JIRA and assign it to me? Grand! - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/#review7528 --- On 2012-05-03 17:27:50, Jimmy Xiang wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4993/ bq. --- bq. bq. (Updated 2012-05-03 17:27:50) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Removed HRegionInterface, and cleaned up the HRegionServer, moved pb code from RegionServer back to HRegionServer. bq. bq. The goal is to avoid two copies of region server code to maintain, and make it possible to avoid data type conversion in the sever side. bq. bq. Fixed some unit tests. Now all region server unit tests test the new pb functions. bq. bq. Enhanced getServerInfo so that it returns the webui port too. bq. bq. bq. This addresses bug HBASE-5889. bq. https://issues.apache.org/jira/browse/HBASE-5889 bq. bq. bq. Diffs bq. - bq. bq.conf/hbase-policy.xml e45f23c bq. security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java 0c4b4cb bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 87f04f4 bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java e3912c2 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java fc9176d bq.src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 757f98e bq.src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java cd9b528 bq.src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 79d5fdd bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 212ee3e bq.src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java d1e0993 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 81603af bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java fbf0127 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java db1333b bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java ae2094d bq.src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java 8b45f03 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 827fb23 bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 96ac8bd bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 4cb070e bq.src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java c2c89ea bq.src/main/protobuf/Admin.proto 2ad6fb0 bq.src/main/protobuf/RPC.proto 105fb3f bq.src/main/resources/hbase-default.xml f54b345 bq.src/main/resources/hbase-webapps/master/table.jsp ca7310c bq.src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java a1992c3 bq.src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java ad77e0a bq.src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 5574b7f bq.src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 3dfc94e bq. src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java 42092b7 bq.src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java c270e28 bq.src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java c36272f bq. src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java bdec3ee bq.
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267939#comment-13267939 ] Hadoop QA commented on HBASE-5889: -- +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525521/hbase_5889_v2.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 60 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +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/1753//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1753//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1753//console This message is automatically generated. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 Attachments: hbase_5889.patch, hbase_5889_v2.patch As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267955#comment-13267955 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon, line 41 bq. https://reviews.apache.org/r/4993/diff/1/?file=106381#file106381line41 bq. bq. Is this right after Gregory's commit last night? He hides pb Serverload inside a ServerLoad. Perhaps have to go to pb RegionLoad still even after his commit? bq. bq. Gregory Chanan wrote: bq. There isn't currently a RegionLoad that hides pb RegionLoad in the way you've described with ServerLoad. I actually wrote one up, but didn't include it because there is an existing RegionLoad that is needed until the HMasterInterface is done. I felt it made more sense to wait rather than spend a bunch of time renaming stuff. bq. bq. If this patch is committed as is, let's file a JIRA and assign it to me? bq. bq. Michael Stack wrote: bq. Grand! I rebased to trunk and got Gregory's patch. bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/HConstants.java, line 195 bq. https://reviews.apache.org/r/4993/diff/1/?file=106382#file106382line195 bq. bq. These are just not used? bq. bq. We used have a mechanism where you could subclass regionserver and then configure it so your subclass was started out on the cluster. You think that still possible? It is still possible. Now, if we want to subclass regionserver, we need to use HConstants.REGION_SERVER_IMPL. The HRegionInterface is not there any more. bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java, line 185 bq. https://reviews.apache.org/r/4993/diff/1/?file=106390#file106390line185 bq. bq. I think this kinda builder is a the right thing to have over in this util class. You are right. I am concerned with the size of the util class. So I'd like to have several special util classes. bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 248 bq. https://reviews.apache.org/r/4993/diff/1/?file=106396#file106396line248 bq. bq. Is this new or moved code? It is moved from RegionServer, which is originally fro HRegionServer. Sorry for the confusion. bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 1657 bq. https://reviews.apache.org/r/4993/diff/1/?file=106398#file106398line1657 bq. bq. This was no longer a good idea? It is hard to maintain two copies of implementation, especially one is not unit tested any more. bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java, line 688 bq. https://reviews.apache.org/r/4993/diff/1/?file=106410#file106410line688 bq. bq. It is unexpected going to ProtobufUtils to get online regions. This method is used in many places. Should I put the util in HRegionServer as a static helper method? bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java, line 315 bq. https://reviews.apache.org/r/4993/diff/1/?file=106414#file106414line315 bq. bq. ditto How about put it in HRegionServer as a static helper util? bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 1295 bq. https://reviews.apache.org/r/4993/diff/1/?file=106388#file106388line1295 bq. bq. Is it going too far adding methods like this into protobufutils? These methods seem pretty core hbase facility, too core to be out in a util method? You have them here because there is a bunch of pb'ing going on? bq. bq. I know I suggested that we move some of the transforms out here... but maybe this is taking it a bit far? bq. bq. What you reckon? openRegion is used in many test classes. That's why I have this in the util. Should I put in HRegionServer as a static helper util? - Jimmy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/#review7528 --- On 2012-05-03 17:27:50, Jimmy Xiang wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4993/ bq. --- bq. bq. (Updated 2012-05-03
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268001#comment-13268001 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/#review7539 --- src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java https://reviews.apache.org/r/4993/#comment16698 HRegionServer util might be better place for this yes. Or could they be put into a new class, RegionServerUtil in the regionserver package? Maybe they don't have to be public methods if done this way? Either sounds good to me boss. - Michael On 2012-05-03 17:27:50, Jimmy Xiang wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4993/ bq. --- bq. bq. (Updated 2012-05-03 17:27:50) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Removed HRegionInterface, and cleaned up the HRegionServer, moved pb code from RegionServer back to HRegionServer. bq. bq. The goal is to avoid two copies of region server code to maintain, and make it possible to avoid data type conversion in the sever side. bq. bq. Fixed some unit tests. Now all region server unit tests test the new pb functions. bq. bq. Enhanced getServerInfo so that it returns the webui port too. bq. bq. bq. This addresses bug HBASE-5889. bq. https://issues.apache.org/jira/browse/HBASE-5889 bq. bq. bq. Diffs bq. - bq. bq.conf/hbase-policy.xml e45f23c bq. security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java 0c4b4cb bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 87f04f4 bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java e3912c2 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java fc9176d bq.src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 757f98e bq.src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java cd9b528 bq.src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 79d5fdd bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 212ee3e bq.src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java d1e0993 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 81603af bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java fbf0127 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java db1333b bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java ae2094d bq.src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java 8b45f03 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 827fb23 bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 96ac8bd bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 4cb070e bq.src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java c2c89ea bq.src/main/protobuf/Admin.proto 2ad6fb0 bq.src/main/protobuf/RPC.proto 105fb3f bq.src/main/resources/hbase-default.xml f54b345 bq.src/main/resources/hbase-webapps/master/table.jsp ca7310c bq.src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java a1992c3 bq.src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java ad77e0a bq.src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 5574b7f bq.src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 3dfc94e bq. src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java 42092b7 bq.src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java c270e28 bq.src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java c36272f bq. src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java bdec3ee bq. src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java 7dbba66 bq. src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java 3acb988 bq. src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java eb546a5 bq.src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java ceba5cd bq. src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java ec08b17 bq.
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268002#comment-13268002 ] jirapos...@reviews.apache.org commented on HBASE-5889: -- bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java, line 1657 bq. https://reviews.apache.org/r/4993/diff/1/?file=106398#file106398line1657 bq. bq. This was no longer a good idea? bq. bq. Jimmy Xiang wrote: bq. It is hard to maintain two copies of implementation, especially one is not unit tested any more. Ok. bq. On 2012-05-03 22:57:57, Michael Stack wrote: bq. src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java, line 688 bq. https://reviews.apache.org/r/4993/diff/1/?file=106410#file106410line688 bq. bq. It is unexpected going to ProtobufUtils to get online regions. bq. bq. Jimmy Xiang wrote: bq. This method is used in many places. Should I put the util in HRegionServer as a static helper method? See above. In fact isn't onlineRegions a class of its own? If so, could be a static method in there? - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4993/#review7528 --- On 2012-05-03 17:27:50, Jimmy Xiang wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4993/ bq. --- bq. bq. (Updated 2012-05-03 17:27:50) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Removed HRegionInterface, and cleaned up the HRegionServer, moved pb code from RegionServer back to HRegionServer. bq. bq. The goal is to avoid two copies of region server code to maintain, and make it possible to avoid data type conversion in the sever side. bq. bq. Fixed some unit tests. Now all region server unit tests test the new pb functions. bq. bq. Enhanced getServerInfo so that it returns the webui port too. bq. bq. bq. This addresses bug HBASE-5889. bq. https://issues.apache.org/jira/browse/HBASE-5889 bq. bq. bq. Diffs bq. - bq. bq.conf/hbase-policy.xml e45f23c bq. security/src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java 0c4b4cb bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 87f04f4 bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java e3912c2 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java fc9176d bq.src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 757f98e bq.src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java cd9b528 bq.src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java 79d5fdd bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 212ee3e bq.src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java d1e0993 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java 81603af bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java fbf0127 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java db1333b bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java ae2094d bq.src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCProtos.java 8b45f03 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 827fb23 bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 96ac8bd bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java 4cb070e bq.src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java c2c89ea bq.src/main/protobuf/Admin.proto 2ad6fb0 bq.src/main/protobuf/RPC.proto 105fb3f bq.src/main/resources/hbase-default.xml f54b345 bq.src/main/resources/hbase-webapps/master/table.jsp ca7310c bq.src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java a1992c3 bq.src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java ad77e0a bq.src/test/java/org/apache/hadoop/hbase/TestRegionRebalancing.java 5574b7f bq.src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java 3dfc94e bq. src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java 42092b7 bq.src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java c270e28 bq.src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java c36272f bq.
[jira] [Commented] (HBASE-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13265000#comment-13265000 ] stack commented on HBASE-5889: -- bq. I'm just not sure this is actually the best bang for the buck, and might make layering less clean. Because the HRegion APIs would all take pbs rather than the Get/Put/Delete, etc.? And doing this conversion would be a bunch of work that would be better spent doing other stuff? Serverside, going from pb into Get/Delete/Put just to get the data into and out of regions seems gratuitous and crud we should purge. Your profiling though would seem to make this a minor issue, one I would have thought prviously critical to address. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13264417#comment-13264417 ] stack commented on HBASE-5889: -- @Jimmy You are trying to get rid of the conversion we currently do where if the server receives a pb Get, we have to change the http://hbase.apache.org/apidocs/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.GetOrBuilder.html into a http://hbase.apache.org/apidocs/org/apache/hadoop/hbase/client/Get.html so you can call http://hbase.apache.org/apidocs/org/apache/hadoop/hbase/regionserver/HRegionServer.html#get(byte[], org.apache.hadoop.hbase.client.Get) ? Sounds good to me. Is there a prob. doing this Todd that you see? Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13264450#comment-13264450 ] Todd Lipcon commented on HBASE-5889: I'm just not sure this is actually the best bang for the buck, and might make layering less clean. I'm looking at RS CPU usage right now and the protobuf conversion doesn't show up at all in oprofile results. Finding a number of other hotspots for this kind of workload, though (see other JIRAs I'm filing) Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13263805#comment-13263805 ] Todd Lipcon commented on HBASE-5889: Can you explain which layer of conversion you're trying to remove? I think we still need our own interface for clients -- clients shouldn't directly speak protobuf. Rather than doing a big change like this, can we get some good profiling on a benchmark and see where our time is going? We can probably make up the lost time in other ways. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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-5889) Remove HRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13263818#comment-13263818 ] Jimmy Xiang commented on HBASE-5889: The existing functions in the client side will not be touched for now. Probably we will add new methods to use the new types later. I was thinking to remove the conversion in server side, for example, HRegion. But for now, I'd like to remove HRegionInterface at first so as to clean up the references to it. This is necessary since I found some test codes are still using the old interface. It will be good for us to test the new interface. Remove HRegionInterface --- Key: HBASE-5889 URL: https://issues.apache.org/jira/browse/HBASE-5889 Project: HBase Issue Type: Improvement Components: client, ipc, regionserver Affects Versions: 0.96.0 Reporter: Jimmy Xiang Assignee: Jimmy Xiang Fix For: 0.96.0 As a step to move internals to PB, so as to avoid the conversion for performance reason, we should remove the HRegionInterface. Therefore region server only supports ClientProtocol and AdminProtocol. Later on, HRegion can work with PB messages directly. -- 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
