[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13268152#comment-13268152 ] Hudson commented on HBASE-5444: --- Integrated in HBase-TRUNK-security #191 (See [https://builds.apache.org/job/HBase-TRUNK-security/191/]) HBASE-5444 Add PB-based calls to HMasterRegionInterface (Revision 119) Result = SUCCESS stack : Files : * /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon * /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ClusterStatus.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ServerLoad.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/RegionServerStatusProtocol.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MXBean.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.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/generated/HBaseProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java * /hbase/trunk/src/main/protobuf/RegionServerStatus.proto * /hbase/trunk/src/main/protobuf/hbase.proto * /hbase/trunk/src/main/resources/hbase-webapps/master/table.jsp * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Fix For: 0.96.0 Attachments: HBASE-5444-v10-trunk.patch, HBASE-5444-v6-trunk.patch, HBASE-5444-v9-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267217#comment-13267217 ] Hadoop QA commented on HBASE-5444: -- +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525392/HBASE-5444-v10-trunk.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 18 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/1740//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1740//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1740//console This message is automatically generated. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v10-trunk.patch, HBASE-5444-v6-trunk.patch, HBASE-5444-v9-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267284#comment-13267284 ] Hudson commented on HBASE-5444: --- Integrated in HBase-TRUNK #2842 (See [https://builds.apache.org/job/HBase-TRUNK/2842/]) HBASE-5444 Add PB-based calls to HMasterRegionInterface (Revision 119) Result = FAILURE stack : Files : * /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon * /hbase/trunk/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ClusterStatus.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ServerLoad.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/RegionServerStatusProtocol.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MXBean.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.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/generated/HBaseProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java * /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java * /hbase/trunk/src/main/protobuf/RegionServerStatus.proto * /hbase/trunk/src/main/protobuf/hbase.proto * /hbase/trunk/src/main/resources/hbase-webapps/master/table.jsp * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java * /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Fix For: 0.96.0 Attachments: HBASE-5444-v10-trunk.patch, HBASE-5444-v6-trunk.patch, HBASE-5444-v9-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267854#comment-13267854 ] Gregory Chanan commented on HBASE-5444: --- @Stack: I filed HBASE-5932 and HBASE-5933. If you think there are things I missed either let me know or file a JIRA and assign it to me. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Fix For: 0.96.0 Attachments: HBASE-5444-v10-trunk.patch, HBASE-5444-v6-trunk.patch, HBASE-5444-v9-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267856#comment-13267856 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- bq. On 2012-05-02 23:43:07, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 80 bq. https://reviews.apache.org/r/4463/diff/5/?file=106063#file106063line80 bq. bq. We need this? bq. bq. Gregory Chanan wrote: bq. I use it to convert the PB serverName that is passed into HMaster.regionServerReport into a ServerName that the ServerManager understands. Instead, we could have a ServerName static function that takes a PB ServerName and returns a ServerName. We already have a bunch of these parse* functions already, e.g. bq. bq. public static ServerName parseVersionedServerName(final byte [] versionedBytes) bq. bq. Michael Stack wrote: bq. Recently in trunk, we added a ServerName.parseFrom that should be able to make sense of any set of bytes parsed it whether pbs or old style versioned bytes. Could use this. bq. bq. Gregory Chanan wrote: bq. I don't think this applies. From reading ServerName.parseFrom it looks like it requires the PBMagicPrefix, which this case doesn't have nor need. Am I missing something? If no pb magic, it then goes on to try and parse the bytes otherwise. See other side of the pb check starting at line #332. - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review7492 --- On 2012-05-02 23:19:20, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-05-02 23:19:20) bq. bq. bq. Review request for hbase and Michael Stack. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b bq.src/main/java/org/apache/hadoop/hbase/ServerLoad.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b bq. src/main/java/org/apache/hadoop/hbase/ipc/RegionServerStatusProtocol.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 bq.src/main/protobuf/RegionServerStatus.proto PRE-CREATION bq.src/main/protobuf/hbase.proto 12e6053 bq.src/main/resources/hbase-webapps/master/table.jsp 3ef1190 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 bq.src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff: https://reviews.apache.org/r/4463/diff bq. bq. bq.
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267011#comment-13267011 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/ --- (Updated 2012-05-02 23:19:20.036448) Review request for hbase and Michael Stack. Changes --- Make changes based on Ted's and Stack's comments. - Fixed spacing - Moved RegionServerStatusProtocol from master to ipc (not o.a.h.h -- hopefully that's okay) - Created a ServerLoad object that encapsulated the PB ServerLoad object. Summary --- Adds PB-based calls replacing HMasterRegionInterface. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. This addresses bug HBASE-5444. https://issues.apache.org/jira/browse/HBASE-5444 Diffs (updated) - src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b src/main/java/org/apache/hadoop/hbase/ServerLoad.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b src/main/java/org/apache/hadoop/hbase/ipc/RegionServerStatusProtocol.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/protobuf/RegionServerStatus.proto PRE-CREATION src/main/protobuf/hbase.proto 12e6053 src/main/resources/hbase-webapps/master/table.jsp 3ef1190 src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 Diff: https://reviews.apache.org/r/4463/diff Testing --- Ran jenkins job, all unit tests passed. Thanks, Gregory Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267038#comment-13267038 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review7492 --- I'm +1 on this patch. I think this much cleaner than previous versions. I wanted more, of course, where users of the protocol would somehow be untouched or polluted by pbs but I realize that is asking for to much. Good stuff Gregory. src/main/java/org/apache/hadoop/hbase/ClusterStatus.java https://reviews.apache.org/r/4463/#comment16588 We need this import? Its for cp. Thats ok I'd say One day we can hide that too.. src/main/java/org/apache/hadoop/hbase/master/HMaster.java https://reviews.apache.org/r/4463/#comment16590 We can move the ipc protocol stuff to top level later... I was thinking that these classes shared by master and regionservers could be at o.a.h.h... but can do that later if it makes sense. Lets get this pb stuff in first. src/main/java/org/apache/hadoop/hbase/master/HMaster.java https://reviews.apache.org/r/4463/#comment16589 We need this? src/main/java/org/apache/hadoop/hbase/master/HMaster.java https://reviews.apache.org/r/4463/#comment16591 Yeah, I suppose you can't hide these from the class that is implementing the protocol... - Michael On 2012-05-02 23:19:20, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-05-02 23:19:20) bq. bq. bq. Review request for hbase and Michael Stack. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b bq.src/main/java/org/apache/hadoop/hbase/ServerLoad.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b bq. src/main/java/org/apache/hadoop/hbase/ipc/RegionServerStatusProtocol.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 bq.src/main/protobuf/RegionServerStatus.proto PRE-CREATION bq.src/main/protobuf/hbase.proto 12e6053 bq.src/main/resources/hbase-webapps/master/table.jsp 3ef1190 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 bq.src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff: https://reviews.apache.org/r/4463/diff bq. bq. bq. Testing bq. --- bq. bq. Ran jenkins job, all unit tests passed. bq. bq. bq. Thanks, bq. bq. Gregory bq.
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267040#comment-13267040 ] stack commented on HBASE-5444: -- Is v6 what is up in rb? And you've submitted it to hadoopqa? Thanks Gregory. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267053#comment-13267053 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- bq. On 2012-05-02 23:43:07, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 79 bq. https://reviews.apache.org/r/4463/diff/5/?file=106063#file106063line79 bq. bq. We can move the ipc protocol stuff to top level later... I was thinking that these classes shared by master and regionservers could be at o.a.h.h... but can do that later if it makes sense. Lets get this pb stuff in first. bq. bq. Sounds good. bq. On 2012-05-02 23:43:07, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 80 bq. https://reviews.apache.org/r/4463/diff/5/?file=106063#file106063line80 bq. bq. We need this? I use it to convert the PB serverName that is passed into HMaster.regionServerReport into a ServerName that the ServerManager understands. Instead, we could have a ServerName static function that takes a PB ServerName and returns a ServerName. We already have a bunch of these parse* functions already, e.g. public static ServerName parseVersionedServerName(final byte [] versionedBytes) bq. On 2012-05-02 23:43:07, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/ClusterStatus.java, line 36 bq. https://reviews.apache.org/r/4463/diff/5/?file=106055#file106055line36 bq. bq. We need this import? Its for cp. Thats ok I'd say One day we can hide that too.. I think we can get rid of this once I pb-ify the HMasterInterface. I'll put it on my list to check out. There's probably a few such cases. - Gregory --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review7492 --- On 2012-05-02 23:19:20, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-05-02 23:19:20) bq. bq. bq. Review request for hbase and Michael Stack. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b bq.src/main/java/org/apache/hadoop/hbase/ServerLoad.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b bq. src/main/java/org/apache/hadoop/hbase/ipc/RegionServerStatusProtocol.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 bq.src/main/protobuf/RegionServerStatus.proto PRE-CREATION bq.src/main/protobuf/hbase.proto 12e6053 bq.src/main/resources/hbase-webapps/master/table.jsp 3ef1190 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 bq.
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267062#comment-13267062 ] Hadoop QA commented on HBASE-5444: -- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525367/HBASE-5444-v9-trunk.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 18 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1733//console This message is automatically generated. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch, HBASE-5444-v9-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267155#comment-13267155 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- bq. On 2012-05-02 23:43:07, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 80 bq. https://reviews.apache.org/r/4463/diff/5/?file=106063#file106063line80 bq. bq. We need this? bq. bq. Gregory Chanan wrote: bq. I use it to convert the PB serverName that is passed into HMaster.regionServerReport into a ServerName that the ServerManager understands. Instead, we could have a ServerName static function that takes a PB ServerName and returns a ServerName. We already have a bunch of these parse* functions already, e.g. bq. bq. public static ServerName parseVersionedServerName(final byte [] versionedBytes) Recently in trunk, we added a ServerName.parseFrom that should be able to make sense of any set of bytes parsed it whether pbs or old style versioned bytes. Could use this. - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review7492 --- On 2012-05-02 23:19:20, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-05-02 23:19:20) bq. bq. bq. Review request for hbase and Michael Stack. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b bq.src/main/java/org/apache/hadoop/hbase/ServerLoad.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b bq. src/main/java/org/apache/hadoop/hbase/ipc/RegionServerStatusProtocol.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 bq.src/main/protobuf/RegionServerStatus.proto PRE-CREATION bq.src/main/protobuf/hbase.proto 12e6053 bq.src/main/resources/hbase-webapps/master/table.jsp 3ef1190 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 bq.src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff: https://reviews.apache.org/r/4463/diff bq. bq. bq. Testing bq. --- bq. bq. Ran jenkins job, all unit tests passed. bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267156#comment-13267156 ] stack commented on HBASE-5444: -- Sorry Gregory, I meant to ask, would you like me commit this and then in separate issues work on the outstanding stuff or do you want to update a new patch? Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch, HBASE-5444-v9-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267183#comment-13267183 ] Gregory Chanan commented on HBASE-5444: --- Updated version against newest trunk. I would like you to commit this and then I'll work on separate issues on the outstanding stuff. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v10-trunk.patch, HBASE-5444-v6-trunk.patch, HBASE-5444-v9-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13267189#comment-13267189 ] stack commented on HBASE-5444: -- @Gregory np Please file the other issues. Waiting on hadoopqa before committing. Good stuff. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v10-trunk.patch, HBASE-5444-v6-trunk.patch, HBASE-5444-v9-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13266009#comment-13266009 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/ --- (Updated 2012-05-01 19:53:51.307399) Review request for hbase and Michael Stack. Changes --- Update against newest trunk and followed Ted's suggestion about breaking out totalRequestsCount computation into own function. Summary --- Adds PB-based calls replacing HMasterRegionInterface. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. This addresses bug HBASE-5444. https://issues.apache.org/jira/browse/HBASE-5444 Diffs (updated) - src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 src/main/java/org/apache/hadoop/hbase/master/RegionServerStatusProtocol.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/protobuf/RegionServerStatus.proto PRE-CREATION src/main/protobuf/hbase.proto 12e6053 src/main/resources/hbase-webapps/master/table.jsp 3ef1190 src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 Diff: https://reviews.apache.org/r/4463/diff Testing --- Ran jenkins job, all unit tests passed. Thanks, Gregory Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13266028#comment-13266028 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review7441 --- src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java https://reviews.apache.org/r/4463/#comment16369 Insert a space between if and (. src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java https://reviews.apache.org/r/4463/#comment16372 You can return list.toArray() directly, similar to line 1179. - Ted On 2012-05-01 19:53:51, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-05-01 19:53:51) bq. bq. bq. Review request for hbase and Michael Stack. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq. src/main/java/org/apache/hadoop/hbase/master/RegionServerStatusProtocol.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 bq.src/main/protobuf/RegionServerStatus.proto PRE-CREATION bq.src/main/protobuf/hbase.proto 12e6053 bq.src/main/resources/hbase-webapps/master/table.jsp 3ef1190 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 bq.src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff: https://reviews.apache.org/r/4463/diff bq. bq. bq. Testing bq. --- bq. bq. Ran jenkins job, all unit tests passed. bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13266033#comment-13266033 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review7442 --- A few comments below the thrust of which are about encapsulating pb if possible rather than have it spread around in classes.See what you think. It would not be hard to get me commit this as is. src/main/java/org/apache/hadoop/hbase/HConstants.java https://reviews.apache.org/r/4463/#comment16370 Were you going to move this down to where its used G? Does it need to be up here? src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java https://reviews.apache.org/r/4463/#comment16371 Hurray! src/main/java/org/apache/hadoop/hbase/master/MXBean.java https://reviews.apache.org/r/4463/#comment16373 All of these classes are importing generated pb classes. Would it be better to have a high-level ServerLoad class that hid inside it the pb stuff instead? Less pb generated class pollution. src/main/java/org/apache/hadoop/hbase/master/ServerManager.java https://reviews.apache.org/r/4463/#comment16374 Yeah, its kinda ugly having this package reach over into pb package. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java https://reviews.apache.org/r/4463/#comment16375 We should not be reaching over into the master package. Put this protocol class at the top level since shared by master and regionserver? - Michael On 2012-05-01 19:53:51, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-05-01 19:53:51) bq. bq. bq. Review request for hbase and Michael Stack. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq. src/main/java/org/apache/hadoop/hbase/master/RegionServerStatusProtocol.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 bq.src/main/protobuf/RegionServerStatus.proto PRE-CREATION bq.src/main/protobuf/hbase.proto 12e6053 bq.src/main/resources/hbase-webapps/master/table.jsp 3ef1190 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 bq.src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff:
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13266168#comment-13266168 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- bq. On 2012-05-01 20:16:06, Ted Yu wrote: bq. src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 1128 bq. https://reviews.apache.org/r/4463/diff/4/?file=105814#file105814line1128 bq. bq. Insert a space between if and (. Will do. bq. On 2012-05-01 20:16:06, Ted Yu wrote: bq. src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java, line 1145 bq. https://reviews.apache.org/r/4463/diff/4/?file=105814#file105814line1145 bq. bq. You can return list.toArray() directly, similar to line 1179. I still need to convert from Coprocessor to String (via getName) if I call toArray(). Am I missing something? - Gregory --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review7441 --- On 2012-05-01 19:53:51, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-05-01 19:53:51) bq. bq. bq. Review request for hbase and Michael Stack. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq. src/main/java/org/apache/hadoop/hbase/master/RegionServerStatusProtocol.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 bq.src/main/protobuf/RegionServerStatus.proto PRE-CREATION bq.src/main/protobuf/hbase.proto 12e6053 bq.src/main/resources/hbase-webapps/master/table.jsp 3ef1190 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 bq.src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff: https://reviews.apache.org/r/4463/diff bq. bq. bq. Testing bq. --- bq. bq. Ran jenkins job, all unit tests passed. bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13266167#comment-13266167 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- bq. On 2012-05-01 20:20:09, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/HConstants.java, line 679 bq. https://reviews.apache.org/r/4463/diff/4/?file=105802#file105802line679 bq. bq. Were you going to move this down to where its used G? Does it need to be up here? I can just get rid of this and use null everywhere. That's what it looks like Jimmy did. bq. On 2012-05-01 20:20:09, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 173 bq. https://reviews.apache.org/r/4463/diff/4/?file=105817#file105817line173 bq. bq. We should not be reaching over into the master package. Put this protocol class at the top level since shared by master and regionserver? Will do. bq. On 2012-05-01 20:20:09, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/master/MXBean.java, line 23 bq. https://reviews.apache.org/r/4463/diff/4/?file=105809#file105809line23 bq. bq. All of these classes are importing generated pb classes. Would it be better to have a high-level ServerLoad class that hid inside it the pb stuff instead? Less pb generated class pollution. I think that's a good idea. I was considering doing that, but wasn't sure if I should do it for all the generated pb types. Since ServerLoad is sprinkled everywhere, it seems like that is at least a good place to start. - Gregory --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review7442 --- On 2012-05-01 19:53:51, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-05-01 19:53:51) bq. bq. bq. Review request for hbase and Michael Stack. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq. src/main/java/org/apache/hadoop/hbase/master/RegionServerStatusProtocol.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 bq.src/main/protobuf/RegionServerStatus.proto PRE-CREATION bq.src/main/protobuf/hbase.proto 12e6053 bq.src/main/resources/hbase-webapps/master/table.jsp 3ef1190 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 bq.src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba bq.
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13265585#comment-13265585 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/ --- (Updated 2012-05-01 01:29:14.986766) Review request for hbase and Michael Stack. Summary --- Adds PB-based calls replacing HMasterRegionInterface. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. This addresses bug HBASE-5444. https://issues.apache.org/jira/browse/HBASE-5444 Diffs - src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 src/main/java/org/apache/hadoop/hbase/master/RegionServerStatusProtocol.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/protobuf/RegionServerStatus.proto PRE-CREATION src/main/protobuf/hbase.proto 12e6053 src/main/resources/hbase-webapps/master/table.jsp 3ef1190 src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 Diff: https://reviews.apache.org/r/4463/diff Testing --- Ran jenkins job, all unit tests passed. Thanks, Gregory Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13265604#comment-13265604 ] Hadoop QA commented on HBASE-5444: -- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525149/HBASE-5444-v6-trunk.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 18 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 appears to introduce 3 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.client.TestShell org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1702//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1702//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1702//console This message is automatically generated. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13265615#comment-13265615 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review7411 --- src/main/resources/hbase-webapps/master/table.jsp https://reviews.apache.org/r/4463/#comment16343 Can this computation be moved somewhere so that it can be reused ? - Ted On 2012-05-01 01:29:14, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-05-01 01:29:14) bq. bq. bq. Review request for hbase and Michael Stack. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b bq.src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq. src/main/java/org/apache/hadoop/hbase/master/RegionServerStatusProtocol.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 bq.src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d bq. src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 bq.src/main/protobuf/RegionServerStatus.proto PRE-CREATION bq.src/main/protobuf/hbase.proto 12e6053 bq.src/main/resources/hbase-webapps/master/table.jsp 3ef1190 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 bq.src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff: https://reviews.apache.org/r/4463/diff bq. bq. bq. Testing bq. --- bq. bq. Ran jenkins job, all unit tests passed. bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan Attachments: HBASE-5444-v6-trunk.patch -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13263248#comment-13263248 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/ --- (Updated 2012-04-26 23:27:07.073810) Review request for hbase. Changes --- Updated for Stack's comments. Passes all unit tests. Summary --- Adds PB-based calls replacing HMasterRegionInterface. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. This addresses bug HBASE-5444. https://issues.apache.org/jira/browse/HBASE-5444 Diffs (updated) - src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon 3c7c091 src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 5d7f07b src/main/java/org/apache/hadoop/hbase/HConstants.java a9d80a0 src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 973c7cb src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java bb6ab3b src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java f56127d src/main/java/org/apache/hadoop/hbase/master/HMaster.java 81e9023 src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 src/main/java/org/apache/hadoop/hbase/master/RegionServerStatusProtocol.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 80271b1 src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java efcf74d src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/protobuf/RegionServerStatus.proto PRE-CREATION src/main/protobuf/hbase.proto 12e6053 src/main/resources/hbase-webapps/master/table.jsp 3ef1190 src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 72554cb src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java d039be3 src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java bd5fa90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 Diff: https://reviews.apache.org/r/4463/diff Testing --- Ran jenkins job, all unit tests passed. Thanks, Gregory Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- 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-5444) Add PB-based calls to HMasterRegionInterface
[
https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13259959#comment-13259959
]
jirapos...@reviews.apache.org commented on HBASE-5444:
--
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. Looks great. On commit will everything be broke?
Should be able to commit next version; this version passed all the tests
previously (things have changed since then).
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. pom.xml, line 797
bq. https://reviews.apache.org/r/4463/diff/2/?file=95085#file95085line797
bq.
bq. We don't need this anymore now we are checking in generated files.
Correct.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. src/main/java/org/apache/hadoop/hbase/HConstants.java, line 33
bq. https://reviews.apache.org/r/4463/diff/2/?file=95089#file95089line33
bq.
bq. Should this top level class have references down into protobufs?
Maybe the empty server load should be in ServerLoad or at least under
o.a.h.h.protobuf
I'll put it under ProtobufUtil. Don't want to put it under ServerLoad because
that is generated (I know we are checking in the generated files, but it would
be a pain to maintain).
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 946
bq. https://reviews.apache.org/r/4463/diff/2/?file=95096#file95096line946
bq.
bq. Should it return the Response builder or the Response (don't they
have same underlying 'Interface'? Return that?)
bq.
bq. Oh, I think I understand... must be a Builder in case we add config
later?
Yes to the second question.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java, line 30
bq. https://reviews.apache.org/r/4463/diff/2/?file=95102#file95102line30
bq.
bq. Something called ProtobufUtil.java was added since you posted your
patch. Maybe this stuff belongs in there?
Correct.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java, line 56
bq. https://reviews.apache.org/r/4463/diff/2/?file=95102#file95102line56
bq.
bq. Why not have this as static in ServerLoad?
Because ServerLoad is generated; would be a pain if we need to change it in the
future.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java, line 79
bq. https://reviews.apache.org/r/4463/diff/2/?file=95102#file95102line79
bq.
bq. ditto
Same as above.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. src/main/proto/hbase.proto, line 22
bq. https://reviews.apache.org/r/4463/diff/2/?file=95105#file95105line22
bq.
bq. A file of this name has gone in already. Need to rejigger your
patch?
Yes.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. src/main/proto/hbase.proto, line 30
bq. https://reviews.apache.org/r/4463/diff/2/?file=95105#file95105line30
bq.
bq. Use Jimmy's regionspec?
Good call.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. src/main/proto/MasterRegionProtocol.proto, line 50
bq. https://reviews.apache.org/r/4463/diff/2/?file=95104#file95104line50
bq.
bq. What is this again? Should this be ServerName from hbase.proto?
Will do.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq. src/main/proto/MasterRegionProtocol.proto, line 61
bq. https://reviews.apache.org/r/4463/diff/2/?file=95104#file95104line61
bq.
bq. ditto
Will do.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq.
src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java, line
39
bq. https://reviews.apache.org/r/4463/diff/2/?file=95101#file95101line39
bq.
bq. You think this package right place for this? How about in master
package or up above at o.a.h.h?
bq.
bq. Is MasterRegionInterface a good name for this Interface even? The
name was made long long time ago. Didn't make sense then. Still doesn't.
Should it be called RegionServerInterface or RegionServer in the master package
-- its the Interface RegionServers invoke on the master... whats a good name
for that? RegionServerService or RegionServersInterface
Master package sounds like a good place for it.
Agreed that the current name isn't good. When Jimmy split the interfaces to
the region, he broke them along functionality lines, i.e. Client vs Admin.
Maybe it makes sense to add something into the name that describes what the
interface does functionally, rather than just the endpoints. How about
RegionServerStatus{Service,Interface,Protocol}? (I'll just copy the endings
that Jimmy used). Every call in the interface basically describes the status,
e.g. startup, report, reportOnError.
bq. On 2012-04-03 06:07:58, Michael Stack wrote:
bq.
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13245013#comment-13245013 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review6651 --- Looks great. On commit will everything be broke? pom.xml https://reviews.apache.org/r/4463/#comment14348 We don't need this anymore now we are checking in generated files. src/main/java/org/apache/hadoop/hbase/HConstants.java https://reviews.apache.org/r/4463/#comment14349 Should this top level class have references down into protobufs? Maybe the empty server load should be in ServerLoad or at least under o.a.h.h.protobuf src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java https://reviews.apache.org/r/4463/#comment14350 Radical! Jimmy didn't remove his interface. Maybe he should have? src/main/java/org/apache/hadoop/hbase/master/HMaster.java https://reviews.apache.org/r/4463/#comment14351 Should it return the Response builder or the Response (don't they have same underlying 'Interface'? Return that?) Oh, I think I understand... must be a Builder in case we add config later? src/main/java/org/apache/hadoop/hbase/master/HMaster.java https://reviews.apache.org/r/4463/#comment14352 ok, yeah, here is the actual instance, not the builder src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java https://reviews.apache.org/r/4463/#comment14354 You think this package right place for this? How about in master package or up above at o.a.h.h? Is MasterRegionInterface a good name for this Interface even? The name was made long long time ago. Didn't make sense then. Still doesn't. Should it be called RegionServerInterface or RegionServer in the master package -- its the Interface RegionServers invoke on the master... whats a good name for that? RegionServerService or RegionServersInterface src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java https://reviews.apache.org/r/4463/#comment14353 Something called ProtobufUtil.java was added since you posted your patch. Maybe this stuff belongs in there? src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java https://reviews.apache.org/r/4463/#comment14355 Why not have this as static in ServerLoad? src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java https://reviews.apache.org/r/4463/#comment14356 ditto src/main/proto/MasterRegionProtocol.proto https://reviews.apache.org/r/4463/#comment14357 MasterRegionProtocol doesn't seem like good name for this file? MasterRegionServer or RegionServerToMasterProtocol or Interface? Jimmy left of the 'Protocol' in his patch. src/main/proto/MasterRegionProtocol.proto https://reviews.apache.org/r/4463/#comment14358 Seems like a bad name. No Region on Master? src/main/proto/MasterRegionProtocol.proto https://reviews.apache.org/r/4463/#comment14359 What is this again? Should this be ServerName from hbase.proto? src/main/proto/MasterRegionProtocol.proto https://reviews.apache.org/r/4463/#comment14360 ditto src/main/proto/MasterRegionProtocol.proto https://reviews.apache.org/r/4463/#comment14361 RegionServerService? src/main/proto/hbase.proto https://reviews.apache.org/r/4463/#comment14362 A file of this name has gone in already. Need to rejigger your patch? src/main/proto/hbase.proto https://reviews.apache.org/r/4463/#comment14363 Use Jimmy's regionspec? src/main/proto/hbase.proto https://reviews.apache.org/r/4463/#comment14364 I think jimmy made one of these already. Coordinate. I like the name of yours... - Michael On 2012-03-23 19:55:28, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-03-23 19:55:28) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.pom.xml 10b13ef bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq.
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13245463#comment-13245463 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- bq. On 2012-04-03 06:07:58, Michael Stack wrote: bq. src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java, line 78 bq. https://reviews.apache.org/r/4463/diff/2/?file=95093#file95093line78 bq. bq. Radical! bq. bq. Jimmy didn't remove his interface. Maybe he should have? I haven't removed HRegionInterface yet. I can remove it after the admin part is also completed. bq. On 2012-04-03 06:07:58, Michael Stack wrote: bq. src/main/proto/hbase.proto, line 116 bq. https://reviews.apache.org/r/4463/diff/2/?file=95105#file95105line116 bq. bq. I think jimmy made one of these already. Coordinate. I like the name of yours... Sure, I can rename mine. - Jimmy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review6651 --- On 2012-03-23 19:55:28, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-03-23 19:55:28) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.pom.xml 10b13ef bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon ae76204 bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 9df4c10 bq.src/main/java/org/apache/hadoop/hbase/HConstants.java 347 bq.src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 2916d68 bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0 bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java d47ef10 bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java cd1755f bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java cbd55f7 bq. src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java e0af8fb bq.src/main/proto/MasterRegionProtocol.proto PRE-CREATION bq.src/main/proto/hbase.proto PRE-CREATION bq.src/main/resources/hbase-webapps/master/table.jsp 811df46 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 6af9188 bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java 368a0e5 bq.src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 841649a bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java 379f70c bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff: https://reviews.apache.org/r/4463/diff bq. bq. bq. Testing bq. --- bq. bq. Ran jenkins job, all unit tests passed. bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13244995#comment-13244995 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- bq. On 2012-03-23 17:01:13, Jimmy Xiang wrote: bq. src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java, line 34 bq. https://reviews.apache.org/r/4463/diff/1/?file=94838#file94838line34 bq. bq. Should it be Private? Should we call it MasterRegionProtocol? bq. bq. Gregory Chanan wrote: bq. I'll make it private. bq. bq. I'll leave it named Interface for now, just because it is replacing HMasterRegionInterface. I agree we should be consistent. Perhaps someone else can chime in with an opinion on Interface vs Protocol. IMO, Protocol would be better. Interface is way overloaded. So is protocol but less so and this seems like usage seems particularly apt. Just my twosense (This was an old comment I never published) - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review6286 --- On 2012-03-23 19:55:28, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-03-23 19:55:28) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.pom.xml 10b13ef bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon ae76204 bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 9df4c10 bq.src/main/java/org/apache/hadoop/hbase/HConstants.java 347 bq.src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 2916d68 bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0 bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java d47ef10 bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java cd1755f bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java cbd55f7 bq. src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java e0af8fb bq.src/main/proto/MasterRegionProtocol.proto PRE-CREATION bq.src/main/proto/hbase.proto PRE-CREATION bq.src/main/resources/hbase-webapps/master/table.jsp 811df46 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 6af9188 bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java 368a0e5 bq.src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 841649a bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java 379f70c bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff: https://reviews.apache.org/r/4463/diff bq. bq. bq. Testing bq. --- bq. bq. Ran jenkins job, all unit tests passed. bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- This message is automatically generated
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236785#comment-13236785 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review6286 --- pom.xml https://reviews.apache.org/r/4463/#comment13630 I have some enhancement to the pom. Please check my patch on https://issues.apache.org/jira/browse/HBASE-5619 src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon https://reviews.apache.org/r/4463/#comment13632 I called it HBaseProtos instead of CommonProtocol since it is not a protocol itself. src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java https://reviews.apache.org/r/4463/#comment13633 Should it be Private? Should we call it MasterRegionProtocol? src/main/proto/MasterRegionProtocol.proto https://reviews.apache.org/r/4463/#comment13634 This looks like a property. I have similar message for RegionProtocols. Can we put this in hbase.proto and call it Property so that we can share it? src/main/proto/hbase.proto https://reviews.apache.org/r/4463/#comment13642 Do we have column family information? - Jimmy On 2012-03-23 04:58:51, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-03-23 04:58:51) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.pom.xml 10b13ef bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon ae76204 bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 9df4c10 bq.src/main/java/org/apache/hadoop/hbase/HConstants.java 347 bq.src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 2916d68 bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0 bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java d47ef10 bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java cd1755f bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java cbd55f7 bq. src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java e0af8fb bq.src/main/proto/MasterRegionProtocol.proto PRE-CREATION bq.src/main/proto/hbase.proto PRE-CREATION bq.src/main/resources/hbase-webapps/master/table.jsp 811df46 bq.src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 6af9188 bq.src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java 368a0e5 bq.src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3 bq.src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 841649a bq.src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java 379f70c bq. src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 bq. bq. Diff: https://reviews.apache.org/r/4463/diff bq. bq. bq. Testing bq. --- bq. bq. Ran jenkins job, all unit tests passed. bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236971#comment-13236971 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- bq. On 2012-03-23 17:01:13, Jimmy Xiang wrote: bq. src/main/proto/hbase.proto, line 33 bq. https://reviews.apache.org/r/4463/diff/1/?file=94842#file94842line33 bq. bq. Do we have column family information? I don't know what you mean by this question. I don't actually need this for HBASE-5444; I'll remove it and bring it back in HBASE-5445. bq. On 2012-03-23 17:01:13, Jimmy Xiang wrote: bq. pom.xml, line 768 bq. https://reviews.apache.org/r/4463/diff/1/?file=94822#file94822line768 bq. bq. I have some enhancement to the pom. Please check my patch on bq. bq. https://issues.apache.org/jira/browse/HBASE-5619 Will do. bq. On 2012-03-23 17:01:13, Jimmy Xiang wrote: bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon, line 40 bq. https://reviews.apache.org/r/4463/diff/1/?file=94823#file94823line40 bq. bq. I called it HBaseProtos instead of CommonProtocol since it is not a protocol itself. Sounds good, will do. bq. On 2012-03-23 17:01:13, Jimmy Xiang wrote: bq. src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java, line 34 bq. https://reviews.apache.org/r/4463/diff/1/?file=94838#file94838line34 bq. bq. Should it be Private? Should we call it MasterRegionProtocol? I'll make it private. I'll leave it named Interface for now, just because it is replacing HMasterRegionInterface. I agree we should be consistent. Perhaps someone else can chime in with an opinion on Interface vs Protocol. bq. On 2012-03-23 17:01:13, Jimmy Xiang wrote: bq. src/main/proto/MasterRegionProtocol.proto, line 40 bq. https://reviews.apache.org/r/4463/diff/1/?file=94841#file94841line40 bq. bq. This looks like a property. I have similar message for RegionProtocols. Can we put this in hbase.proto and call it Property so that we can share it? Property is kind of a non-descriptive name. How about NameValuePair? - Gregory --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/#review6286 --- On 2012-03-23 04:58:51, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4463/ bq. --- bq. bq. (Updated 2012-03-23 04:58:51) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Adds PB-based calls replacing HMasterRegionInterface. bq. bq. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.pom.xml 10b13ef bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 bq. src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon ae76204 bq.src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 9df4c10 bq.src/main/java/org/apache/hadoop/hbase/HConstants.java 347 bq.src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 bq.src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 2916d68 bq.src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 bq.src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0 bq.src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java d47ef10 bq.src/main/java/org/apache/hadoop/hbase/master/HMaster.java cd1755f bq.src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 bq.src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 bq.src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 bq.src/main/java/org/apache/hadoop/hbase/master/ServerManager.java cbd55f7 bq. src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java PRE-CREATION bq.src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java e0af8fb bq.src/main/proto/MasterRegionProtocol.proto PRE-CREATION bq.src/main/proto/hbase.proto PRE-CREATION bq.src/main/resources/hbase-webapps/master/table.jsp 811df46
[jira] [Commented] (HBASE-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236999#comment-13236999 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/ --- (Updated 2012-03-23 19:55:28.089163) Review request for hbase. Changes --- Update for Jimmy's review. Summary --- Adds PB-based calls replacing HMasterRegionInterface. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. This addresses bug HBASE-5444. https://issues.apache.org/jira/browse/HBASE-5444 Diffs (updated) - pom.xml 10b13ef src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon ae76204 src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 9df4c10 src/main/java/org/apache/hadoop/hbase/HConstants.java 347 src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489 src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 2916d68 src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0 src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java d47ef10 src/main/java/org/apache/hadoop/hbase/master/HMaster.java cd1755f src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 src/main/java/org/apache/hadoop/hbase/master/ServerManager.java cbd55f7 src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java e0af8fb src/main/proto/MasterRegionProtocol.proto PRE-CREATION src/main/proto/hbase.proto PRE-CREATION src/main/resources/hbase-webapps/master/table.jsp 811df46 src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 6af9188 src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java 368a0e5 src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3 src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 841649a src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java 379f70c src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 Diff: https://reviews.apache.org/r/4463/diff Testing --- Ran jenkins job, all unit tests passed. Thanks, Gregory Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- 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-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236327#comment-13236327 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4463/ --- Review request for hbase. Summary --- Adds PB-based calls replacing HMasterRegionInterface. There are some temporary hacks, e.g. converting PB-based ServerLoad to existing HServerLoad so I didn't need to convert ClusterStatus (which brings in a lot of other changes). That will be cleaned up in HBASE-5445. This addresses bug HBASE-5444. https://issues.apache.org/jira/browse/HBASE-5444 Diffs - pom.xml 10b13ef src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon 69434f7 src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RSStatusTmpl.jamon ae76204 src/main/java/org/apache/hadoop/hbase/ClusterStatus.java 9df4c10 src/main/java/org/apache/hadoop/hbase/HConstants.java 347 src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java cbfa489 src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java 0db2760 src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 2916d68 src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java fd97830 src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java f1f06b0 src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java d47ef10 src/main/java/org/apache/hadoop/hbase/master/HMaster.java cd1755f src/main/java/org/apache/hadoop/hbase/master/MXBean.java 7f44dc2 src/main/java/org/apache/hadoop/hbase/master/MXBeanImpl.java 45b8fe7 src/main/java/org/apache/hadoop/hbase/master/MasterDumpServlet.java be63838 src/main/java/org/apache/hadoop/hbase/master/ServerManager.java cbd55f7 src/main/java/org/apache/hadoop/hbase/protobuf/MasterRegionInterface.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/protobuf/PBHelper.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java e0af8fb src/main/proto/MasterRegionProtocol.proto PRE-CREATION src/main/proto/hbase.proto PRE-CREATION src/main/resources/hbase-webapps/master/table.jsp 811df46 src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 6af9188 src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java 368a0e5 src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java f2f8ee3 src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 841649a src/test/java/org/apache/hadoop/hbase/master/TestMXBean.java 379f70c src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e99d251 Diff: https://reviews.apache.org/r/4463/diff Testing --- Ran jenkins job, all unit tests passed. Thanks, Gregory Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- 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-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13221165#comment-13221165 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4149/#review5570 --- Please check Benoit's comments on https://reviews.apache.org/r/4054/ about shorten the names. I updated https://reviews.apache.org/r/4054/ with a new diff. It has ServeName and other shared protos. src/main/proto/HMasterRegionProtocol.proto https://reviews.apache.org/r/4149/#comment12061 Please put generated at the end. - Jimmy On 2012-03-02 01:30:36, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4149/ bq. --- bq. bq. (Updated 2012-03-02 01:30:36) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Protobuf work for HMasterRegionInterface. bq. bq. No need to comment on the pom.xml changes: I just copied those from HBASE-5443 (https://reviews.apache.org/r/4054/). bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.pom.xml 0f0aa9a bq.src/main/proto/HMasterRegionProtocol.proto PRE-CREATION bq.src/main/proto/hbase.proto PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/4149/diff bq. bq. bq. Testing bq. --- bq. bq. mvn -DskipTests package successful and files generated successfully bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- 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-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13221192#comment-13221192 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4149/#review5574 --- src/main/proto/HMasterRegionProtocol.proto https://reviews.apache.org/r/4149/#comment12072 Can we doc whats in here better? src/main/proto/HMasterRegionProtocol.proto https://reviews.apache.org/r/4149/#comment12073 Is this one k/v only? Isn't it possible to pass more than just the one k/v? An actual Map? src/main/proto/HMasterRegionProtocol.proto https://reviews.apache.org/r/4149/#comment12075 Doesn't exising interface have doc. Could we port that over? - Michael On 2012-03-02 01:30:36, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4149/ bq. --- bq. bq. (Updated 2012-03-02 01:30:36) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Protobuf work for HMasterRegionInterface. bq. bq. No need to comment on the pom.xml changes: I just copied those from HBASE-5443 (https://reviews.apache.org/r/4054/). bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.pom.xml 0f0aa9a bq.src/main/proto/HMasterRegionProtocol.proto PRE-CREATION bq.src/main/proto/hbase.proto PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/4149/diff bq. bq. bq. Testing bq. --- bq. bq. mvn -DskipTests package successful and files generated successfully bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- 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-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13221209#comment-13221209 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4149/#review5576 --- src/main/proto/HMasterRegionProtocol.proto https://reviews.apache.org/r/4149/#comment12076 will do. src/main/proto/HMasterRegionProtocol.proto https://reviews.apache.org/r/4149/#comment12077 You can, the name here sucks -- this is really a MapEntry. If you look at RegionServerStartupResponseProto, it contains a list of these entries. src/main/proto/HMasterRegionProtocol.proto https://reviews.apache.org/r/4149/#comment12084 Good idea. Not sure what the best form for comments is -- I don't see any way to have protobufs generate javadoc comments into the generated code. I'll figure something out. - Gregory On 2012-03-02 01:30:36, Gregory Chanan wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4149/ bq. --- bq. bq. (Updated 2012-03-02 01:30:36) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. Protobuf work for HMasterRegionInterface. bq. bq. No need to comment on the pom.xml changes: I just copied those from HBASE-5443 (https://reviews.apache.org/r/4054/). bq. bq. bq. This addresses bug HBASE-5444. bq. https://issues.apache.org/jira/browse/HBASE-5444 bq. bq. bq. Diffs bq. - bq. bq.pom.xml 0f0aa9a bq.src/main/proto/HMasterRegionProtocol.proto PRE-CREATION bq.src/main/proto/hbase.proto PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/4149/diff bq. bq. bq. Testing bq. --- bq. bq. mvn -DskipTests package successful and files generated successfully bq. bq. bq. Thanks, bq. bq. Gregory bq. bq. Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- 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-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13221229#comment-13221229 ] Zhihong Yu commented on HBASE-5444: --- I found this issue w.r.t. javadoc in protobufs generated code: http://code.google.com/p/protobuf/issues/detail?id=148 Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- 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-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13221304#comment-13221304 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4149/ --- (Updated 2012-03-02 22:18:47.273090) Review request for hbase. Summary --- Protobuf work for HMasterRegionInterface. No need to comment on the pom.xml changes: I just copied those from HBASE-5443 (https://reviews.apache.org/r/4054/). This addresses bug HBASE-5444. https://issues.apache.org/jira/browse/HBASE-5444 Diffs (updated) - pom.xml 0f0aa9a src/main/proto/HMasterRegionProtocol.proto PRE-CREATION src/main/proto/hbase.proto PRE-CREATION Diff: https://reviews.apache.org/r/4149/diff Testing --- mvn -DskipTests package successful and files generated successfully Thanks, Gregory Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- 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-5444) Add PB-based calls to HMasterRegionInterface
[ https://issues.apache.org/jira/browse/HBASE-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13220578#comment-13220578 ] jirapos...@reviews.apache.org commented on HBASE-5444: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4149/ --- Review request for hbase. Summary --- Protobuf work for HMasterRegionInterface. No need to comment on the pom.xml changes: I just copied those from HBASE-5443 (https://reviews.apache.org/r/4054/). This addresses bug HBASE-5444. https://issues.apache.org/jira/browse/HBASE-5444 Diffs - pom.xml 0f0aa9a src/main/proto/HMasterRegionProtocol.proto PRE-CREATION src/main/proto/hbase.proto PRE-CREATION Diff: https://reviews.apache.org/r/4149/diff Testing --- mvn -DskipTests package successful and files generated successfully Thanks, Gregory Add PB-based calls to HMasterRegionInterface Key: HBASE-5444 URL: https://issues.apache.org/jira/browse/HBASE-5444 Project: HBase Issue Type: Sub-task Components: ipc, master, migration, regionserver Reporter: Todd Lipcon Assignee: Gregory Chanan -- 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
