HBase
  1. HBase
  2. HBASE-5869

Move SplitLogManager splitlog taskstate and AssignmentManager RegionTransitionData znode datas to pb

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.95.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    1. 5869v7.txt
      282 kB
      stack
    2. 5869v8.txt
      291 kB
      stack
    3. 5869v9.txt
      297 kB
      stack
    4. firstcut.txt
      102 kB
      stack
    5. secondcut.txt
      139 kB
      stack
    6. v10.txt
      312 kB
      stack
    7. v11.txt
      327 kB
      stack
    8. v12.txt
      328 kB
      stack
    9. v13.txt
      329 kB
      stack
    10. v13.txt
      329 kB
      stack
    11. v4.txt
      231 kB
      stack
    12. v5.txt
      282 kB
      stack
    13. v6.txt
      281 kB
      stack

      Issue Links

        Activity

        Hide
        stack added a comment -

        Marking closed.

        Show
        stack added a comment - Marking closed.
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-security #190 (See https://builds.apache.org/job/HBase-TRUNK-security/190/)
        HBASE-5919 Add fixes for Ted's review comments from HBASE-5869 (Revision 1333304)
        HBASE-5869 Move SplitLogManager splitlog taskstate and AssignmentManager RegionTransitionData znode datas to pb (Revision 1333099)

        Result = SUCCESS
        tedyu :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java

        stack :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/DeserializationException.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HBaseException.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/RegionTransition.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ServerName.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/SplitLogTask.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.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/MasterFileSystem.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
        • /hbase/trunk/src/main/protobuf/ZooKeeper.proto
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestSerialization.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/Mocking.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK-security #190 (See https://builds.apache.org/job/HBase-TRUNK-security/190/ ) HBASE-5919 Add fixes for Ted's review comments from HBASE-5869 (Revision 1333304) HBASE-5869 Move SplitLogManager splitlog taskstate and AssignmentManager RegionTransitionData znode datas to pb (Revision 1333099) Result = SUCCESS tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java stack : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/DeserializationException.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HBaseException.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/RegionTransition.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ServerName.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/SplitLogTask.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.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/MasterFileSystem.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java /hbase/trunk/src/main/protobuf/ZooKeeper.proto /hbase/trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestSerialization.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/Mocking.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2841 (See https://builds.apache.org/job/HBase-TRUNK/2841/)
        HBASE-5919 Add fixes for Ted's review comments from HBASE-5869 (Revision 1333304)

        Result = SUCCESS
        tedyu :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2841 (See https://builds.apache.org/job/HBase-TRUNK/2841/ ) HBASE-5919 Add fixes for Ted's review comments from HBASE-5869 (Revision 1333304) Result = SUCCESS tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java
        Hide
        stack added a comment -

        No. I committed the patch that passed hadoopqa. Will do new issue to address your comments.

        Show
        stack added a comment - No. I committed the patch that passed hadoopqa. Will do new issue to address your comments.
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2836 (See https://builds.apache.org/job/HBase-TRUNK/2836/)
        HBASE-5869 Move SplitLogManager splitlog taskstate and AssignmentManager RegionTransitionData znode datas to pb (Revision 1333099)

        Result = SUCCESS
        stack :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/DeserializationException.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HBaseException.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/RegionTransition.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ServerName.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/SplitLogTask.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.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/MasterFileSystem.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
        • /hbase/trunk/src/main/protobuf/ZooKeeper.proto
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestSerialization.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/Mocking.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2836 (See https://builds.apache.org/job/HBase-TRUNK/2836/ ) HBASE-5869 Move SplitLogManager splitlog taskstate and AssignmentManager RegionTransitionData znode datas to pb (Revision 1333099) Result = SUCCESS stack : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/DeserializationException.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HBaseException.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/RegionTransition.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ServerName.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/SplitLogTask.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.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/MasterFileSystem.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java /hbase/trunk/src/main/protobuf/ZooKeeper.proto /hbase/trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestSerialization.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/Mocking.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
        Hide
        Ted Yu added a comment -

        Let me fix toStringBinary so it deals w/ case where data is < 64 bytes.

        Is the above done ?
        According to http://docs.oracle.com/javase/6/docs/api/java/lang/String.html#String%28byte[],%20int,%20int,%20java.lang.String%29 :

        IndexOutOfBoundsException - If the offset and length arguments index characters outside the bounds of the bytes array
        

        > Should e1 be included in the log ?

        Will add it.

        Is the above done ?

        Show
        Ted Yu added a comment - Let me fix toStringBinary so it deals w/ case where data is < 64 bytes. Is the above done ? According to http://docs.oracle.com/javase/6/docs/api/java/lang/String.html#String%28byte[],%20int,%20int,%20java.lang.String%29 : IndexOutOfBoundsException - If the offset and length arguments index characters outside the bounds of the bytes array > Should e1 be included in the log ? Will add it. Is the above done ?
        Hide
        stack added a comment -

        Committed trunk after second clean hadoopqa build. Thanks for reviews Ted.

        Show
        stack added a comment - Committed trunk after second clean hadoopqa build. Thanks for reviews Ted.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525301/v13.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 50 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/1723//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1723//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1723//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525301/v13.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 50 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/1723//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1723//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1723//console This message is automatically generated.
        Hide
        stack added a comment -

        If a DE comes out, I don't think it a good idea that we cut the stack trace off at the knees (I'm new to pb; would like to see some of these exceptions first before I start making calls on how we might massage them)

        Show
        stack added a comment - If a DE comes out, I don't think it a good idea that we cut the stack trace off at the knees (I'm new to pb; would like to see some of these exceptions first before I start making calls on how we might massage them)
        Hide
        Ted Yu added a comment -

        Searching through the patch, I don't see InvalidProtocolBufferException being parsed/unwrapped out of DeserializationException.
        DeserializationException provides isolation to clients w.r.t. various exceptions such as InvalidProtocolBufferException.
        So there is no loss not keeping the stack of InvalidProtocolBufferException.

        Show
        Ted Yu added a comment - Searching through the patch, I don't see InvalidProtocolBufferException being parsed/unwrapped out of DeserializationException. DeserializationException provides isolation to clients w.r.t. various exceptions such as InvalidProtocolBufferException. So there is no loss not keeping the stack of InvalidProtocolBufferException.
        Hide
        stack added a comment -

        Retry to see if hadoopqa will pick it up.

        Show
        stack added a comment - Retry to see if hadoopqa will pick it up.
        Hide
        stack added a comment -

        I don't follow. Please give example illustrating the difference you are trying to bring out.

        As is, will show IPBE as cause and if a message, that'll show as part of the cause. Yours would look to squash the exception stack to just the message... if there is one at all.

        Show
        stack added a comment - I don't follow. Please give example illustrating the difference you are trying to bring out. As is, will show IPBE as cause and if a message, that'll show as part of the cause. Yours would look to squash the exception stack to just the message... if there is one at all.
        Hide
        Ted Yu added a comment -

        According to https://developers.google.com/protocol-buffers/docs/reference/java/com/google/protobuf/InvalidProtocolBufferException, InvalidProtocolBufferException doesn't provide its own method for revealing what went wrong during parsing.

        In the patch, I see the following construct:

            } catch (InvalidProtocolBufferException e) {
              throw new DeserializationException(e);
        

        I think we don't expect client to interpret InvalidProtocolBufferException. I would suggest changing the above to:

            } catch (InvalidProtocolBufferException e) {
              throw new DeserializationException(e.getMessage());
        
        Show
        Ted Yu added a comment - According to https://developers.google.com/protocol-buffers/docs/reference/java/com/google/protobuf/InvalidProtocolBufferException , InvalidProtocolBufferException doesn't provide its own method for revealing what went wrong during parsing. In the patch, I see the following construct: } catch (InvalidProtocolBufferException e) { throw new DeserializationException(e); I think we don't expect client to interpret InvalidProtocolBufferException. I would suggest changing the above to: } catch (InvalidProtocolBufferException e) { throw new DeserializationException(e.getMessage());
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/#review7452
        -----------------------------------------------------------

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
        <https://reviews.apache.org/r/4926/#comment16434>

        The port numbers don't match.

        • Ted

        On 2012-05-01 20:42:36, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4926/

        -----------------------------------------------------------

        (Updated 2012-05-01 20:42:36)

        Review request for hbase and Jimmy Xiang.

        Summary

        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.

        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into

        the classes themselves so can encapsulate how serialization is done into one place

        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports

        that are across package (zookeeper into protobuf and/or into regionserver and/or

        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java

        New generic deserialization exception.

        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java

        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java

        Moved under zookeeper package.

        A src/main/java/org/apache/hadoop/hbase/HBaseException.java

        New base hbase exception as suggested by hbase-5796. New DeserializationException

        inherits from this.

        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java

        State of a region in transition. Top-level because used by a

        few top-level packages. Encapsulates pb serialization/deserialization.

        M src/main/java/org/apache/hadoop/hbase/ServerName.java

        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java

        Counters used by distributed log splitting.

        A SplitLogTask

        Class that encapsulates log splitting state. Also encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java

        Implement code for state. Added functions to go from code to state and vice

        versa. Used serializing.

        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java

        Remove unused imports.

        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java

        Removed. Replaced by RegionTransition moved to package top-level.

        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

        Use new DeserializationException. Move to using new RegionTransition

        from RegionTransitionData class. Pass deserialized class rather than

        byte array. Remove duplicated code.

        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java

        Use new ServerName parse method rather than ZKUtil one.

        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java

        Redo to use new SplitLogTask and SplitLogCounter classes.

        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

        expectPBMagicPrefix added

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        Use new RegionTransition in place of RegionTransitionData.

        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java

        Define moved from ZKSplitLog to SplitLogManager.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

        Changed method name from getZNodeData to toByteArray to match how we've

        named it elsewhere. Use new DeserializationException

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

        Use new RegionTransion class

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java

        Moved stuff that was in here up into SplitLogManager where better

        belongs. Also moved serialization/deserialization up into the

        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java

        Moved deserialization of ServerName out of here and up into ServerName.

        M src/main/protobuf/ZooKeeper.proto

        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.

        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2

        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508

        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377

        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6

        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/MasterFileSystem.java 692f194

        src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f

        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76

        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc

        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 914b0d3

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c

        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af

        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde

        src/main/protobuf/ZooKeeper.proto 961ab65

        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889

        src/test/java/org/apache/hadoop/hbase/TestSerialization.java 50cb9d4

        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48

        src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java 1105ec9

        src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java 36dd289

        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb

        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8

        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876

        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90

        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba

        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a

        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 01dff76

        Diff: https://reviews.apache.org/r/4926/diff

        Testing

        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/#review7452 ----------------------------------------------------------- src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java < https://reviews.apache.org/r/4926/#comment16434 > The port numbers don't match. Ted On 2012-05-01 20:42:36, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-05-01 20:42:36) Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs ----- src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/MasterFileSystem.java 692f194 src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 914b0d3 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestSerialization.java 50cb9d4 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java 1105ec9 src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java 36dd289 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 01dff76 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-05-01 21:59:01, Ted Yu wrote:

        > src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java, line 99

        > <https://reviews.apache.org/r/4926/diff/3/?file=105878#file105878line99>

        >

        > What if an AtomicInteger counter is added in the future ?

        Open new JIRA. This is just a move of existing code.

        On 2012-05-01 21:59:01, Ted Yu wrote:

        > src/main/java/org/apache/hadoop/hbase/SplitLogTask.java, line 155

        > <https://reviews.apache.org/r/4926/diff/3/?file=105879#file105879line155>

        >

        > What would the first 64 bytes of data represent ?

        > Do we know that data.length >= 64 ?

        Let me fix toStringBinary so it deals w/ case where data is < 64 bytes.

        Regards what this represents, it could be anything. Just saving our logs from being filled w/ binary.

        On 2012-05-01 21:59:01, Ted Yu wrote:

        > src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java, line 567

        > <https://reviews.apache.org/r/4926/diff/3/?file=105887#file105887line567>

        >

        > Should e1 be included in the log ?

        > 're-' before 'resubmit' is not necessary.

        Will add it.

        re- makes sense because this is retry inside exception handling.

        • Michael

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/#review7446
        -----------------------------------------------------------

        On 2012-05-01 20:42:36, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4926/

        -----------------------------------------------------------

        (Updated 2012-05-01 20:42:36)

        Review request for hbase and Jimmy Xiang.

        Summary

        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.

        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into

        the classes themselves so can encapsulate how serialization is done into one place

        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports

        that are across package (zookeeper into protobuf and/or into regionserver and/or

        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java

        New generic deserialization exception.

        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java

        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java

        Moved under zookeeper package.

        A src/main/java/org/apache/hadoop/hbase/HBaseException.java

        New base hbase exception as suggested by hbase-5796. New DeserializationException

        inherits from this.

        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java

        State of a region in transition. Top-level because used by a

        few top-level packages. Encapsulates pb serialization/deserialization.

        M src/main/java/org/apache/hadoop/hbase/ServerName.java

        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java

        Counters used by distributed log splitting.

        A SplitLogTask

        Class that encapsulates log splitting state. Also encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java

        Implement code for state. Added functions to go from code to state and vice

        versa. Used serializing.

        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java

        Remove unused imports.

        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java

        Removed. Replaced by RegionTransition moved to package top-level.

        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

        Use new DeserializationException. Move to using new RegionTransition

        from RegionTransitionData class. Pass deserialized class rather than

        byte array. Remove duplicated code.

        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java

        Use new ServerName parse method rather than ZKUtil one.

        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java

        Redo to use new SplitLogTask and SplitLogCounter classes.

        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

        expectPBMagicPrefix added

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        Use new RegionTransition in place of RegionTransitionData.

        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java

        Define moved from ZKSplitLog to SplitLogManager.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

        Changed method name from getZNodeData to toByteArray to match how we've

        named it elsewhere. Use new DeserializationException

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

        Use new RegionTransion class

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java

        Moved stuff that was in here up into SplitLogManager where better

        belongs. Also moved serialization/deserialization up into the

        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java

        Moved deserialization of ServerName out of here and up into ServerName.

        M src/main/protobuf/ZooKeeper.proto

        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.

        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2

        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508

        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377

        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6

        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/MasterFileSystem.java 692f194

        src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f

        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76

        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc

        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 914b0d3

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c

        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af

        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde

        src/main/protobuf/ZooKeeper.proto 961ab65

        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889

        src/test/java/org/apache/hadoop/hbase/TestSerialization.java 50cb9d4

        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48

        src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java 1105ec9

        src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java 36dd289

        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb

        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8

        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876

        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90

        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba

        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a

        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 01dff76

        Diff: https://reviews.apache.org/r/4926/diff

        Testing

        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-05-01 21:59:01, Ted Yu wrote: > src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java, line 99 > < https://reviews.apache.org/r/4926/diff/3/?file=105878#file105878line99 > > > What if an AtomicInteger counter is added in the future ? Open new JIRA. This is just a move of existing code. On 2012-05-01 21:59:01, Ted Yu wrote: > src/main/java/org/apache/hadoop/hbase/SplitLogTask.java, line 155 > < https://reviews.apache.org/r/4926/diff/3/?file=105879#file105879line155 > > > What would the first 64 bytes of data represent ? > Do we know that data.length >= 64 ? Let me fix toStringBinary so it deals w/ case where data is < 64 bytes. Regards what this represents, it could be anything. Just saving our logs from being filled w/ binary. On 2012-05-01 21:59:01, Ted Yu wrote: > src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java, line 567 > < https://reviews.apache.org/r/4926/diff/3/?file=105887#file105887line567 > > > Should e1 be included in the log ? > 're-' before 'resubmit' is not necessary. Will add it. re- makes sense because this is retry inside exception handling. Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/#review7446 ----------------------------------------------------------- On 2012-05-01 20:42:36, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-05-01 20:42:36) Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs ----- src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/MasterFileSystem.java 692f194 src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 914b0d3 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestSerialization.java 50cb9d4 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java 1105ec9 src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java 36dd289 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 01dff76 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/#review7446
        -----------------------------------------------------------

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java
        <https://reviews.apache.org/r/4926/#comment16378>

        Usually this line is not the first in a file.

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java
        <https://reviews.apache.org/r/4926/#comment16379>

        'log' seems redundant here.

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java
        <https://reviews.apache.org/r/4926/#comment16377>

        What if an AtomicInteger counter is added in the future ?

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java
        <https://reviews.apache.org/r/4926/#comment16381>

        'date' -> 'data'

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java
        <https://reviews.apache.org/r/4926/#comment16382>

        'An' -> 'A'

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java
        <https://reviews.apache.org/r/4926/#comment16383>

        What would the first 64 bytes of data represent ?
        Do we know that data.length >= 64 ?

        src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        <https://reviews.apache.org/r/4926/#comment16408>

        Should e1 be included in the log ?
        're-' before 'resubmit' is not necessary.

        • Ted

        On 2012-05-01 20:42:36, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4926/

        -----------------------------------------------------------

        (Updated 2012-05-01 20:42:36)

        Review request for hbase and Jimmy Xiang.

        Summary

        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.

        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into

        the classes themselves so can encapsulate how serialization is done into one place

        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports

        that are across package (zookeeper into protobuf and/or into regionserver and/or

        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java

        New generic deserialization exception.

        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java

        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java

        Moved under zookeeper package.

        A src/main/java/org/apache/hadoop/hbase/HBaseException.java

        New base hbase exception as suggested by hbase-5796. New DeserializationException

        inherits from this.

        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java

        State of a region in transition. Top-level because used by a

        few top-level packages. Encapsulates pb serialization/deserialization.

        M src/main/java/org/apache/hadoop/hbase/ServerName.java

        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java

        Counters used by distributed log splitting.

        A SplitLogTask

        Class that encapsulates log splitting state. Also encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java

        Implement code for state. Added functions to go from code to state and vice

        versa. Used serializing.

        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java

        Remove unused imports.

        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java

        Removed. Replaced by RegionTransition moved to package top-level.

        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

        Use new DeserializationException. Move to using new RegionTransition

        from RegionTransitionData class. Pass deserialized class rather than

        byte array. Remove duplicated code.

        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java

        Use new ServerName parse method rather than ZKUtil one.

        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java

        Redo to use new SplitLogTask and SplitLogCounter classes.

        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

        expectPBMagicPrefix added

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        Use new RegionTransition in place of RegionTransitionData.

        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java

        Define moved from ZKSplitLog to SplitLogManager.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

        Changed method name from getZNodeData to toByteArray to match how we've

        named it elsewhere. Use new DeserializationException

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

        Use new RegionTransion class

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java

        Moved stuff that was in here up into SplitLogManager where better

        belongs. Also moved serialization/deserialization up into the

        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java

        Moved deserialization of ServerName out of here and up into ServerName.

        M src/main/protobuf/ZooKeeper.proto

        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.

        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2

        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508

        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377

        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6

        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/MasterFileSystem.java 692f194

        src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f

        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76

        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc

        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 914b0d3

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c

        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af

        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde

        src/main/protobuf/ZooKeeper.proto 961ab65

        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889

        src/test/java/org/apache/hadoop/hbase/TestSerialization.java 50cb9d4

        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48

        src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java 1105ec9

        src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java 36dd289

        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb

        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8

        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876

        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90

        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba

        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a

        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 01dff76

        Diff: https://reviews.apache.org/r/4926/diff

        Testing

        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/#review7446 ----------------------------------------------------------- src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java < https://reviews.apache.org/r/4926/#comment16378 > Usually this line is not the first in a file. src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java < https://reviews.apache.org/r/4926/#comment16379 > 'log' seems redundant here. src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java < https://reviews.apache.org/r/4926/#comment16377 > What if an AtomicInteger counter is added in the future ? src/main/java/org/apache/hadoop/hbase/SplitLogTask.java < https://reviews.apache.org/r/4926/#comment16381 > 'date' -> 'data' src/main/java/org/apache/hadoop/hbase/SplitLogTask.java < https://reviews.apache.org/r/4926/#comment16382 > 'An' -> 'A' src/main/java/org/apache/hadoop/hbase/SplitLogTask.java < https://reviews.apache.org/r/4926/#comment16383 > What would the first 64 bytes of data represent ? Do we know that data.length >= 64 ? src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java < https://reviews.apache.org/r/4926/#comment16408 > Should e1 be included in the log ? 're-' before 'resubmit' is not necessary. Ted On 2012-05-01 20:42:36, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-05-01 20:42:36) Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs ----- src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/MasterFileSystem.java 692f194 src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 914b0d3 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestSerialization.java 50cb9d4 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java 1105ec9 src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java 36dd289 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 01dff76 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525216/v13.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 50 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/1711//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1711//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1711//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525216/v13.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 50 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/1711//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1711//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1711//console This message is automatically generated.
        Hide
        stack added a comment -

        What I applied to rb and what I want to commit. Same as v12 only removes needless region creation over in testwalobserver tests... was causing testwalobserver take time going down.

        Show
        stack added a comment - What I applied to rb and what I want to commit. Same as v12 only removes needless region creation over in testwalobserver tests... was causing testwalobserver take time going down.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/
        -----------------------------------------------------------

        (Updated 2012-05-01 20:42:36.337375)

        Review request for hbase and Jimmy Xiang.

        Changes
        -------

        Same as original w/ a few fixes for tests that failed:

        1. In distributed log tests, was failing to pick up the recovered.edits file because string passed included state of the split log task when what was wanted was servername only

        Summary
        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.
        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into
        the classes themselves so can encapsulate how serialization is done into one place
        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports
        that are across package (zookeeper into protobuf and/or into regionserver and/or
        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java
        New generic deserialization exception.
        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java
        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java
        Moved under zookeeper package.
        A src/main/java/org/apache/hadoop/hbase/HBaseException.java
        New base hbase exception as suggested by hbase-5796. New DeserializationException
        inherits from this.
        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java
        State of a region in transition. Top-level because used by a
        few top-level packages. Encapsulates pb serialization/deserialization.
        M src/main/java/org/apache/hadoop/hbase/ServerName.java
        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.
        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java
        Counters used by distributed log splitting.
        A SplitLogTask
        Class that encapsulates log splitting state. Also encapsulates pb'ing.
        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
        Implement code for state. Added functions to go from code to state and vice
        versa. Used serializing.
        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
        Remove unused imports.
        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java
        Removed. Replaced by RegionTransition moved to package top-level.
        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java
        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
        Use new DeserializationException. Move to using new RegionTransition
        from RegionTransitionData class. Pass deserialized class rather than
        byte array. Remove duplicated code.
        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        Use new ServerName parse method rather than ZKUtil one.
        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java
        Redo to use new SplitLogTask and SplitLogCounter classes.
        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        expectPBMagicPrefix added
        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        Use new RegionTransition in place of RegionTransitionData.
        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java
        Define moved from ZKSplitLog to SplitLogManager.
        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java
        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java
        Changed method name from getZNodeData to toByteArray to match how we've
        named it elsewhere. Use new DeserializationException
        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
        Use new RegionTransion class
        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java
        Moved stuff that was in here up into SplitLogManager where better
        belongs. Also moved serialization/deserialization up into the
        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.
        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
        Moved deserialization of ServerName out of here and up into ServerName.
        M src/main/protobuf/ZooKeeper.proto
        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.
        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs (updated)


        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2
        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624
        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508
        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377
        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70
        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6
        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/MasterFileSystem.java 692f194
        src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f
        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76
        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc
        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6
        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f
        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4
        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 914b0d3
        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c
        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af
        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80
        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672
        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9
        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde
        src/main/protobuf/ZooKeeper.proto 961ab65
        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889
        src/test/java/org/apache/hadoop/hbase/TestSerialization.java 50cb9d4
        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48
        src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java 1105ec9
        src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java 36dd289
        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb
        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8
        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876
        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90
        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba
        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e
        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865
        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea
        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4
        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a
        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 01dff76

        Diff: https://reviews.apache.org/r/4926/diff

        Testing
        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-05-01 20:42:36.337375) Review request for hbase and Jimmy Xiang. Changes ------- Same as original w/ a few fixes for tests that failed: 1. In distributed log tests, was failing to pick up the recovered.edits file because string passed included state of the split log task when what was wanted was servername only Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs (updated) src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/MasterFileSystem.java 692f194 src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 914b0d3 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestSerialization.java 50cb9d4 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditor.java 1105ec9 src/test/java/org/apache/hadoop/hbase/coprocessor/TestWALObserver.java 36dd289 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 01dff76 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        stack added a comment -

        Whoopdeee!

        Let me post latest up on rb in case anyone wants take a look. Will apply tomorrow unless objection. Let me do another test run too to be sure.

        Show
        stack added a comment - Whoopdeee! Let me post latest up on rb in case anyone wants take a look. Will apply tomorrow unless objection. Let me do another test run too to be sure.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525202/v12.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 50 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/1710//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1710//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1710//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525202/v12.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 50 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/1710//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1710//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1710//console This message is automatically generated.
        Hide
        stack added a comment -

        These three tests hung. If I run them local, they are fine. Retrying.

        Show
        stack added a comment - These three tests hung. If I run them local, they are fine. Retrying.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525189/v11.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 47 new or modified tests.

        +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1707//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1707//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1707//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525189/v11.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 47 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1707//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1707//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1707//console This message is automatically generated.
        Hide
        stack added a comment -

        Hopefully got all the test failures with this version of patch

        Show
        stack added a comment - Hopefully got all the test failures with this version of patch
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525158/v10.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 47 new or modified tests.

        +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol
        org.apache.hadoop.hbase.client.TestShell
        org.apache.hadoop.hbase.master.TestMasterFailover

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1703//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1703//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1703//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525158/v10.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 47 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol org.apache.hadoop.hbase.client.TestShell org.apache.hadoop.hbase.master.TestMasterFailover Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1703//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1703//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1703//console This message is automatically generated.
        Hide
        stack added a comment -

        File path to recovered edits prob. fixed

        Show
        stack added a comment - File path to recovered edits prob. fixed
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525109/5869v9.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 47 new or modified tests.

        +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.client.TestScannerTimeout
        org.apache.hadoop.hbase.client.TestMultiParallel
        org.apache.hadoop.hbase.master.TestMasterFailover
        org.apache.hadoop.hbase.TestFullLogReconstruction
        org.apache.hadoop.hbase.regionserver.TestRSKilledWhenMasterInitializing
        org.apache.hadoop.hbase.regionserver.TestHRegionOnCluster
        org.apache.hadoop.hbase.TestDrainingServer
        org.apache.hadoop.hbase.catalog.TestMetaReaderEditor
        org.apache.hadoop.hbase.master.TestDistributedLogSplitting

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1694//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1694//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1694//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525109/5869v9.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 47 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.client.TestScannerTimeout org.apache.hadoop.hbase.client.TestMultiParallel org.apache.hadoop.hbase.master.TestMasterFailover org.apache.hadoop.hbase.TestFullLogReconstruction org.apache.hadoop.hbase.regionserver.TestRSKilledWhenMasterInitializing org.apache.hadoop.hbase.regionserver.TestHRegionOnCluster org.apache.hadoop.hbase.TestDrainingServer org.apache.hadoop.hbase.catalog.TestMetaReaderEditor org.apache.hadoop.hbase.master.TestDistributedLogSplitting Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1694//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1694//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1694//console This message is automatically generated.
        Hide
        stack added a comment -

        I was returning early in AssignmentManager if null data inside isCarryingRegion when I should have carried on to trip over the get of region location from the AM memory. Seems to fix some of the failing tests.

        Show
        stack added a comment - I was returning early in AssignmentManager if null data inside isCarryingRegion when I should have carried on to trip over the get of region location from the AM memory. Seems to fix some of the failing tests.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/#review7379
        -----------------------------------------------------------

        Looks good to me.

        • Jimmy

        On 2012-04-28 23:42:52, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4926/

        -----------------------------------------------------------

        (Updated 2012-04-28 23:42:52)

        Review request for hbase and Jimmy Xiang.

        Summary

        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.

        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into

        the classes themselves so can encapsulate how serialization is done into one place

        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports

        that are across package (zookeeper into protobuf and/or into regionserver and/or

        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java

        New generic deserialization exception.

        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java

        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java

        Moved under zookeeper package.

        A src/main/java/org/apache/hadoop/hbase/HBaseException.java

        New base hbase exception as suggested by hbase-5796. New DeserializationException

        inherits from this.

        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java

        State of a region in transition. Top-level because used by a

        few top-level packages. Encapsulates pb serialization/deserialization.

        M src/main/java/org/apache/hadoop/hbase/ServerName.java

        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java

        Counters used by distributed log splitting.

        A SplitLogTask

        Class that encapsulates log splitting state. Also encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java

        Implement code for state. Added functions to go from code to state and vice

        versa. Used serializing.

        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java

        Remove unused imports.

        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java

        Removed. Replaced by RegionTransition moved to package top-level.

        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

        Use new DeserializationException. Move to using new RegionTransition

        from RegionTransitionData class. Pass deserialized class rather than

        byte array. Remove duplicated code.

        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java

        Use new ServerName parse method rather than ZKUtil one.

        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java

        Redo to use new SplitLogTask and SplitLogCounter classes.

        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

        expectPBMagicPrefix added

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        Use new RegionTransition in place of RegionTransitionData.

        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java

        Define moved from ZKSplitLog to SplitLogManager.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

        Changed method name from getZNodeData to toByteArray to match how we've

        named it elsewhere. Use new DeserializationException

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

        Use new RegionTransion class

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java

        Moved stuff that was in here up into SplitLogManager where better

        belongs. Also moved serialization/deserialization up into the

        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java

        Moved deserialization of ServerName out of here and up into ServerName.

        M src/main/protobuf/ZooKeeper.proto

        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.

        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2

        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508

        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377

        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6

        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/MasterFileSystem.java 692f194

        src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f

        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76

        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc

        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c

        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af

        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde

        src/main/protobuf/ZooKeeper.proto 961ab65

        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889

        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48

        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb

        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8

        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876

        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90

        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba

        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a

        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572

        Diff: https://reviews.apache.org/r/4926/diff

        Testing

        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/#review7379 ----------------------------------------------------------- Looks good to me. Jimmy On 2012-04-28 23:42:52, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-04-28 23:42:52) Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs ----- src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/MasterFileSystem.java 692f194 src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-04-28 22:14:23, Jimmy Xiang wrote:

        > src/main/protobuf/ZooKeeper.proto, line 82

        > <https://reviews.apache.org/r/4926/diff/1/?file=105372#file105372line82>

        >

        > A task is a path, this is more like a task state, isn't it?

        Michael Stack wrote:

        I can change this np.

        Currently I have the pb class named same as the class that wraps it. Should I change this? Add a pb prefix or something? Problem w/ that is that no other of the pb classes have the pb prefix. They are in the generated package which is probably sufficient to distingush them? My hope is to make it so the pbs do not leak outside of the class that serializes to them; e.g. this SplitLogTask class.

        Jimmy Xiang wrote:

        I got your point. I prefer to have the pb class named the same as the wrapper class, if there is one. Should we create a separate task state wrapper class if needed?

        Michael Stack wrote:

        I just tried changing the name of this class from SplitLogTask to SplitLogTaskState and it don't seem right since you can do a 'getState' call on this class – the class has State AND the origin of the task. I'm going to leave the name as is.

        Ok on keeping names the same. It should be fine if we can keep the pb stuff bottled up under the pb package or internal only to the class that uses the pb (except where pb comes out on server..)

        Thanks Jimmy

        Ok, that's fine with me.

        On 2012-04-28 22:14:23, Jimmy Xiang wrote:

        > src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java, line 182

        > <https://reviews.apache.org/r/4926/diff/1/?file=105357#file105357line182>

        >

        > Should we abort? Under what scenario the parsing can fail, other than a conflict data format?

        Michael Stack wrote:

        I thought I was just redoing what was there previous. We could abort but maybe next time through the deserialization works because its been updated by another? Or, we spew this error all over the logs and drive someone crazy? Will look at it again.

        Michael Stack wrote:

        Yeah, I'll leave this as is after looking at it. Hopefully will be good on next go around.

        Ok

        • Jimmy

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/#review7360
        -----------------------------------------------------------

        On 2012-04-28 23:42:52, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4926/

        -----------------------------------------------------------

        (Updated 2012-04-28 23:42:52)

        Review request for hbase and Jimmy Xiang.

        Summary

        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.

        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into

        the classes themselves so can encapsulate how serialization is done into one place

        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports

        that are across package (zookeeper into protobuf and/or into regionserver and/or

        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java

        New generic deserialization exception.

        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java

        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java

        Moved under zookeeper package.

        A src/main/java/org/apache/hadoop/hbase/HBaseException.java

        New base hbase exception as suggested by hbase-5796. New DeserializationException

        inherits from this.

        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java

        State of a region in transition. Top-level because used by a

        few top-level packages. Encapsulates pb serialization/deserialization.

        M src/main/java/org/apache/hadoop/hbase/ServerName.java

        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java

        Counters used by distributed log splitting.

        A SplitLogTask

        Class that encapsulates log splitting state. Also encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java

        Implement code for state. Added functions to go from code to state and vice

        versa. Used serializing.

        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java

        Remove unused imports.

        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java

        Removed. Replaced by RegionTransition moved to package top-level.

        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

        Use new DeserializationException. Move to using new RegionTransition

        from RegionTransitionData class. Pass deserialized class rather than

        byte array. Remove duplicated code.

        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java

        Use new ServerName parse method rather than ZKUtil one.

        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java

        Redo to use new SplitLogTask and SplitLogCounter classes.

        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

        expectPBMagicPrefix added

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        Use new RegionTransition in place of RegionTransitionData.

        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java

        Define moved from ZKSplitLog to SplitLogManager.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

        Changed method name from getZNodeData to toByteArray to match how we've

        named it elsewhere. Use new DeserializationException

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

        Use new RegionTransion class

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java

        Moved stuff that was in here up into SplitLogManager where better

        belongs. Also moved serialization/deserialization up into the

        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java

        Moved deserialization of ServerName out of here and up into ServerName.

        M src/main/protobuf/ZooKeeper.proto

        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.

        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2

        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508

        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377

        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6

        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/MasterFileSystem.java 692f194

        src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f

        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76

        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc

        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c

        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af

        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde

        src/main/protobuf/ZooKeeper.proto 961ab65

        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889

        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48

        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb

        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8

        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876

        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90

        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba

        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a

        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572

        Diff: https://reviews.apache.org/r/4926/diff

        Testing

        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-04-28 22:14:23, Jimmy Xiang wrote: > src/main/protobuf/ZooKeeper.proto, line 82 > < https://reviews.apache.org/r/4926/diff/1/?file=105372#file105372line82 > > > A task is a path, this is more like a task state, isn't it? Michael Stack wrote: I can change this np. Currently I have the pb class named same as the class that wraps it. Should I change this? Add a pb prefix or something? Problem w/ that is that no other of the pb classes have the pb prefix. They are in the generated package which is probably sufficient to distingush them? My hope is to make it so the pbs do not leak outside of the class that serializes to them; e.g. this SplitLogTask class. Jimmy Xiang wrote: I got your point. I prefer to have the pb class named the same as the wrapper class, if there is one. Should we create a separate task state wrapper class if needed? Michael Stack wrote: I just tried changing the name of this class from SplitLogTask to SplitLogTaskState and it don't seem right since you can do a 'getState' call on this class – the class has State AND the origin of the task. I'm going to leave the name as is. Ok on keeping names the same. It should be fine if we can keep the pb stuff bottled up under the pb package or internal only to the class that uses the pb (except where pb comes out on server..) Thanks Jimmy Ok, that's fine with me. On 2012-04-28 22:14:23, Jimmy Xiang wrote: > src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java, line 182 > < https://reviews.apache.org/r/4926/diff/1/?file=105357#file105357line182 > > > Should we abort? Under what scenario the parsing can fail, other than a conflict data format? Michael Stack wrote: I thought I was just redoing what was there previous. We could abort but maybe next time through the deserialization works because its been updated by another? Or, we spew this error all over the logs and drive someone crazy? Will look at it again. Michael Stack wrote: Yeah, I'll leave this as is after looking at it. Hopefully will be good on next go around. Ok Jimmy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/#review7360 ----------------------------------------------------------- On 2012-04-28 23:42:52, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-04-28 23:42:52) Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs ----- src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/MasterFileSystem.java 692f194 src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12525070/5869v8.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 47 new or modified tests.

        +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.master.TestRollingRestart
        org.apache.hadoop.hbase.regionserver.TestHRegionOnCluster
        org.apache.hadoop.hbase.regionserver.wal.TestLogRollingNoCluster
        org.apache.hadoop.hbase.client.TestScannerTimeout
        org.apache.hadoop.hbase.master.TestDistributedLogSplitting
        org.apache.hadoop.hbase.TestDrainingServer
        org.apache.hadoop.hbase.regionserver.TestRSKilledWhenMasterInitializing
        org.apache.hadoop.hbase.TestFullLogReconstruction
        org.apache.hadoop.hbase.master.TestMasterFailover
        org.apache.hadoop.hbase.master.TestSplitLogManager
        org.apache.hadoop.hbase.TestZooKeeper

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1690//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1690//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1690//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12525070/5869v8.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 47 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.master.TestRollingRestart org.apache.hadoop.hbase.regionserver.TestHRegionOnCluster org.apache.hadoop.hbase.regionserver.wal.TestLogRollingNoCluster org.apache.hadoop.hbase.client.TestScannerTimeout org.apache.hadoop.hbase.master.TestDistributedLogSplitting org.apache.hadoop.hbase.TestDrainingServer org.apache.hadoop.hbase.regionserver.TestRSKilledWhenMasterInitializing org.apache.hadoop.hbase.TestFullLogReconstruction org.apache.hadoop.hbase.master.TestMasterFailover org.apache.hadoop.hbase.master.TestSplitLogManager org.apache.hadoop.hbase.TestZooKeeper Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1690//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1690//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1690//console This message is automatically generated.
        Hide
        stack added a comment -

        Fixes for a few of the failing tests.

        Show
        stack added a comment - Fixes for a few of the failing tests.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12524999/5869v7.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 41 new or modified tests.

        +1 hadoop23. The patch compiles against the hadoop 0.23.x profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.master.TestRollingRestart
        org.apache.hadoop.hbase.client.TestMultiParallel
        org.apache.hadoop.hbase.regionserver.TestHRegionOnCluster
        org.apache.hadoop.hbase.client.TestScannerTimeout
        org.apache.hadoop.hbase.master.TestDistributedLogSplitting
        org.apache.hadoop.hbase.replication.TestReplication
        org.apache.hadoop.hbase.catalog.TestMetaReaderEditor
        org.apache.hadoop.hbase.regionserver.TestSplitLogWorker
        org.apache.hadoop.hbase.regionserver.TestRSKilledWhenMasterInitializing
        org.apache.hadoop.hbase.master.TestAssignmentManager
        org.apache.hadoop.hbase.TestFullLogReconstruction
        org.apache.hadoop.hbase.master.TestMasterFailover
        org.apache.hadoop.hbase.TestZooKeeper

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1680//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1680//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1680//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12524999/5869v7.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 41 new or modified tests. +1 hadoop23. The patch compiles against the hadoop 0.23.x profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.master.TestRollingRestart org.apache.hadoop.hbase.client.TestMultiParallel org.apache.hadoop.hbase.regionserver.TestHRegionOnCluster org.apache.hadoop.hbase.client.TestScannerTimeout org.apache.hadoop.hbase.master.TestDistributedLogSplitting org.apache.hadoop.hbase.replication.TestReplication org.apache.hadoop.hbase.catalog.TestMetaReaderEditor org.apache.hadoop.hbase.regionserver.TestSplitLogWorker org.apache.hadoop.hbase.regionserver.TestRSKilledWhenMasterInitializing org.apache.hadoop.hbase.master.TestAssignmentManager org.apache.hadoop.hbase.TestFullLogReconstruction org.apache.hadoop.hbase.master.TestMasterFailover org.apache.hadoop.hbase.TestZooKeeper Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1680//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1680//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1680//console This message is automatically generated.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/
        -----------------------------------------------------------

        (Updated 2012-04-28 23:42:52.841896)

        Review request for hbase and Jimmy Xiang.

        Changes
        -------

        Address Jimmy's comments

        Summary
        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.
        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into
        the classes themselves so can encapsulate how serialization is done into one place
        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports
        that are across package (zookeeper into protobuf and/or into regionserver and/or
        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java
        New generic deserialization exception.
        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java
        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java
        Moved under zookeeper package.
        A src/main/java/org/apache/hadoop/hbase/HBaseException.java
        New base hbase exception as suggested by hbase-5796. New DeserializationException
        inherits from this.
        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java
        State of a region in transition. Top-level because used by a
        few top-level packages. Encapsulates pb serialization/deserialization.
        M src/main/java/org/apache/hadoop/hbase/ServerName.java
        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.
        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java
        Counters used by distributed log splitting.
        A SplitLogTask
        Class that encapsulates log splitting state. Also encapsulates pb'ing.
        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
        Implement code for state. Added functions to go from code to state and vice
        versa. Used serializing.
        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
        Remove unused imports.
        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java
        Removed. Replaced by RegionTransition moved to package top-level.
        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java
        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
        Use new DeserializationException. Move to using new RegionTransition
        from RegionTransitionData class. Pass deserialized class rather than
        byte array. Remove duplicated code.
        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        Use new ServerName parse method rather than ZKUtil one.
        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java
        Redo to use new SplitLogTask and SplitLogCounter classes.
        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        expectPBMagicPrefix added
        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        Use new RegionTransition in place of RegionTransitionData.
        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java
        Define moved from ZKSplitLog to SplitLogManager.
        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java
        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java
        Changed method name from getZNodeData to toByteArray to match how we've
        named it elsewhere. Use new DeserializationException
        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
        Use new RegionTransion class
        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java
        Moved stuff that was in here up into SplitLogManager where better
        belongs. Also moved serialization/deserialization up into the
        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.
        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
        Moved deserialization of ServerName out of here and up into ServerName.
        M src/main/protobuf/ZooKeeper.proto
        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.
        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs (updated)


        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2
        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624
        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508
        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377
        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70
        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6
        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/MasterFileSystem.java 692f194
        src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f
        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76
        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc
        src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6
        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f
        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4
        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c
        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af
        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80
        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672
        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9
        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde
        src/main/protobuf/ZooKeeper.proto 961ab65
        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889
        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48
        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb
        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8
        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876
        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90
        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba
        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e
        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865
        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea
        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4
        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a
        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572

        Diff: https://reviews.apache.org/r/4926/diff

        Testing
        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-04-28 23:42:52.841896) Review request for hbase and Jimmy Xiang. Changes ------- Address Jimmy's comments Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs (updated) src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/MasterFileSystem.java 692f194 src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ebffad6 src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        stack added a comment -

        Address Jimmy's comments.

        Show
        stack added a comment - Address Jimmy's comments.
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-04-28 22:14:23, Jimmy Xiang wrote:

        > src/main/protobuf/ZooKeeper.proto, line 82

        > <https://reviews.apache.org/r/4926/diff/1/?file=105372#file105372line82>

        >

        > A task is a path, this is more like a task state, isn't it?

        Michael Stack wrote:

        I can change this np.

        Currently I have the pb class named same as the class that wraps it. Should I change this? Add a pb prefix or something? Problem w/ that is that no other of the pb classes have the pb prefix. They are in the generated package which is probably sufficient to distingush them? My hope is to make it so the pbs do not leak outside of the class that serializes to them; e.g. this SplitLogTask class.

        Jimmy Xiang wrote:

        I got your point. I prefer to have the pb class named the same as the wrapper class, if there is one. Should we create a separate task state wrapper class if needed?

        I just tried changing the name of this class from SplitLogTask to SplitLogTaskState and it don't seem right since you can do a 'getState' call on this class – the class has State AND the origin of the task. I'm going to leave the name as is.

        Ok on keeping names the same. It should be fine if we can keep the pb stuff bottled up under the pb package or internal only to the class that uses the pb (except where pb comes out on server..)

        Thanks Jimmy

        On 2012-04-28 22:14:23, Jimmy Xiang wrote:

        > src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java, line 182

        > <https://reviews.apache.org/r/4926/diff/1/?file=105357#file105357line182>

        >

        > Should we abort? Under what scenario the parsing can fail, other than a conflict data format?

        Michael Stack wrote:

        I thought I was just redoing what was there previous. We could abort but maybe next time through the deserialization works because its been updated by another? Or, we spew this error all over the logs and drive someone crazy? Will look at it again.

        Yeah, I'll leave this as is after looking at it. Hopefully will be good on next go around.

        • Michael

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/#review7360
        -----------------------------------------------------------

        On 2012-04-28 18:10:24, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4926/

        -----------------------------------------------------------

        (Updated 2012-04-28 18:10:24)

        Review request for hbase and Jimmy Xiang.

        Summary

        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.

        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into

        the classes themselves so can encapsulate how serialization is done into one place

        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports

        that are across package (zookeeper into protobuf and/or into regionserver and/or

        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java

        New generic deserialization exception.

        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java

        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java

        Moved under zookeeper package.

        A src/main/java/org/apache/hadoop/hbase/HBaseException.java

        New base hbase exception as suggested by hbase-5796. New DeserializationException

        inherits from this.

        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java

        State of a region in transition. Top-level because used by a

        few top-level packages. Encapsulates pb serialization/deserialization.

        M src/main/java/org/apache/hadoop/hbase/ServerName.java

        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java

        Counters used by distributed log splitting.

        A SplitLogTask

        Class that encapsulates log splitting state. Also encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java

        Implement code for state. Added functions to go from code to state and vice

        versa. Used serializing.

        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java

        Remove unused imports.

        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java

        Removed. Replaced by RegionTransition moved to package top-level.

        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

        Use new DeserializationException. Move to using new RegionTransition

        from RegionTransitionData class. Pass deserialized class rather than

        byte array. Remove duplicated code.

        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java

        Use new ServerName parse method rather than ZKUtil one.

        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java

        Redo to use new SplitLogTask and SplitLogCounter classes.

        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

        expectPBMagicPrefix added

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        Use new RegionTransition in place of RegionTransitionData.

        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java

        Define moved from ZKSplitLog to SplitLogManager.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

        Changed method name from getZNodeData to toByteArray to match how we've

        named it elsewhere. Use new DeserializationException

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

        Use new RegionTransion class

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java

        Moved stuff that was in here up into SplitLogManager where better

        belongs. Also moved serialization/deserialization up into the

        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java

        Moved deserialization of ServerName out of here and up into ServerName.

        M src/main/protobuf/ZooKeeper.proto

        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.

        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2

        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508

        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377

        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6

        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/SplitLogManager.java 919c65f

        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76

        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c

        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af

        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde

        src/main/protobuf/ZooKeeper.proto 961ab65

        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889

        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48

        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb

        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8

        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876

        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90

        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba

        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a

        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572

        Diff: https://reviews.apache.org/r/4926/diff

        Testing

        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-04-28 22:14:23, Jimmy Xiang wrote: > src/main/protobuf/ZooKeeper.proto, line 82 > < https://reviews.apache.org/r/4926/diff/1/?file=105372#file105372line82 > > > A task is a path, this is more like a task state, isn't it? Michael Stack wrote: I can change this np. Currently I have the pb class named same as the class that wraps it. Should I change this? Add a pb prefix or something? Problem w/ that is that no other of the pb classes have the pb prefix. They are in the generated package which is probably sufficient to distingush them? My hope is to make it so the pbs do not leak outside of the class that serializes to them; e.g. this SplitLogTask class. Jimmy Xiang wrote: I got your point. I prefer to have the pb class named the same as the wrapper class, if there is one. Should we create a separate task state wrapper class if needed? I just tried changing the name of this class from SplitLogTask to SplitLogTaskState and it don't seem right since you can do a 'getState' call on this class – the class has State AND the origin of the task. I'm going to leave the name as is. Ok on keeping names the same. It should be fine if we can keep the pb stuff bottled up under the pb package or internal only to the class that uses the pb (except where pb comes out on server..) Thanks Jimmy On 2012-04-28 22:14:23, Jimmy Xiang wrote: > src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java, line 182 > < https://reviews.apache.org/r/4926/diff/1/?file=105357#file105357line182 > > > Should we abort? Under what scenario the parsing can fail, other than a conflict data format? Michael Stack wrote: I thought I was just redoing what was there previous. We could abort but maybe next time through the deserialization works because its been updated by another? Or, we spew this error all over the logs and drive someone crazy? Will look at it again. Yeah, I'll leave this as is after looking at it. Hopefully will be good on next go around. Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/#review7360 ----------------------------------------------------------- On 2012-04-28 18:10:24, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-04-28 18:10:24) Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs ----- src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-04-28 22:14:23, Jimmy Xiang wrote:

        > src/main/protobuf/ZooKeeper.proto, line 82

        > <https://reviews.apache.org/r/4926/diff/1/?file=105372#file105372line82>

        >

        > A task is a path, this is more like a task state, isn't it?

        Michael Stack wrote:

        I can change this np.

        Currently I have the pb class named same as the class that wraps it. Should I change this? Add a pb prefix or something? Problem w/ that is that no other of the pb classes have the pb prefix. They are in the generated package which is probably sufficient to distingush them? My hope is to make it so the pbs do not leak outside of the class that serializes to them; e.g. this SplitLogTask class.

        I got your point. I prefer to have the pb class named the same as the wrapper class, if there is one. Should we create a separate task state wrapper class if needed?

        • Jimmy

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/#review7360
        -----------------------------------------------------------

        On 2012-04-28 18:10:24, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4926/

        -----------------------------------------------------------

        (Updated 2012-04-28 18:10:24)

        Review request for hbase and Jimmy Xiang.

        Summary

        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.

        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into

        the classes themselves so can encapsulate how serialization is done into one place

        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports

        that are across package (zookeeper into protobuf and/or into regionserver and/or

        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java

        New generic deserialization exception.

        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java

        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java

        Moved under zookeeper package.

        A src/main/java/org/apache/hadoop/hbase/HBaseException.java

        New base hbase exception as suggested by hbase-5796. New DeserializationException

        inherits from this.

        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java

        State of a region in transition. Top-level because used by a

        few top-level packages. Encapsulates pb serialization/deserialization.

        M src/main/java/org/apache/hadoop/hbase/ServerName.java

        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java

        Counters used by distributed log splitting.

        A SplitLogTask

        Class that encapsulates log splitting state. Also encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java

        Implement code for state. Added functions to go from code to state and vice

        versa. Used serializing.

        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java

        Remove unused imports.

        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java

        Removed. Replaced by RegionTransition moved to package top-level.

        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

        Use new DeserializationException. Move to using new RegionTransition

        from RegionTransitionData class. Pass deserialized class rather than

        byte array. Remove duplicated code.

        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java

        Use new ServerName parse method rather than ZKUtil one.

        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java

        Redo to use new SplitLogTask and SplitLogCounter classes.

        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

        expectPBMagicPrefix added

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        Use new RegionTransition in place of RegionTransitionData.

        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java

        Define moved from ZKSplitLog to SplitLogManager.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

        Changed method name from getZNodeData to toByteArray to match how we've

        named it elsewhere. Use new DeserializationException

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

        Use new RegionTransion class

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java

        Moved stuff that was in here up into SplitLogManager where better

        belongs. Also moved serialization/deserialization up into the

        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java

        Moved deserialization of ServerName out of here and up into ServerName.

        M src/main/protobuf/ZooKeeper.proto

        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.

        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2

        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508

        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377

        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6

        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/SplitLogManager.java 919c65f

        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76

        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c

        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af

        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde

        src/main/protobuf/ZooKeeper.proto 961ab65

        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889

        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48

        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb

        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8

        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876

        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90

        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba

        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a

        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572

        Diff: https://reviews.apache.org/r/4926/diff

        Testing

        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-04-28 22:14:23, Jimmy Xiang wrote: > src/main/protobuf/ZooKeeper.proto, line 82 > < https://reviews.apache.org/r/4926/diff/1/?file=105372#file105372line82 > > > A task is a path, this is more like a task state, isn't it? Michael Stack wrote: I can change this np. Currently I have the pb class named same as the class that wraps it. Should I change this? Add a pb prefix or something? Problem w/ that is that no other of the pb classes have the pb prefix. They are in the generated package which is probably sufficient to distingush them? My hope is to make it so the pbs do not leak outside of the class that serializes to them; e.g. this SplitLogTask class. I got your point. I prefer to have the pb class named the same as the wrapper class, if there is one. Should we create a separate task state wrapper class if needed? Jimmy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/#review7360 ----------------------------------------------------------- On 2012-04-28 18:10:24, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-04-28 18:10:24) Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs ----- src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-04-28 22:14:23, Jimmy Xiang wrote:

        > src/main/protobuf/ZooKeeper.proto, line 82

        > <https://reviews.apache.org/r/4926/diff/1/?file=105372#file105372line82>

        >

        > A task is a path, this is more like a task state, isn't it?

        I can change this np.

        Currently I have the pb class named same as the class that wraps it. Should I change this? Add a pb prefix or something? Problem w/ that is that no other of the pb classes have the pb prefix. They are in the generated package which is probably sufficient to distingush them? My hope is to make it so the pbs do not leak outside of the class that serializes to them; e.g. this SplitLogTask class.

        On 2012-04-28 22:14:23, Jimmy Xiang wrote:

        > src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java, line 798

        > <https://reviews.apache.org/r/4926/diff/1/?file=105369#file105369line798>

        >

        > I see this many places. Can we put the whole block in a util method? Probably will be easier to read?

        Will do.

        On 2012-04-28 22:14:23, Jimmy Xiang wrote:

        > src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java, line 182

        > <https://reviews.apache.org/r/4926/diff/1/?file=105357#file105357line182>

        >

        > Should we abort? Under what scenario the parsing can fail, other than a conflict data format?

        I thought I was just redoing what was there previous. We could abort but maybe next time through the deserialization works because its been updated by another? Or, we spew this error all over the logs and drive someone crazy? Will look at it again.

        • Michael

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/#review7360
        -----------------------------------------------------------

        On 2012-04-28 18:10:24, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4926/

        -----------------------------------------------------------

        (Updated 2012-04-28 18:10:24)

        Review request for hbase and Jimmy Xiang.

        Summary

        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.

        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into

        the classes themselves so can encapsulate how serialization is done into one place

        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports

        that are across package (zookeeper into protobuf and/or into regionserver and/or

        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java

        New generic deserialization exception.

        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java

        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java

        Moved under zookeeper package.

        A src/main/java/org/apache/hadoop/hbase/HBaseException.java

        New base hbase exception as suggested by hbase-5796. New DeserializationException

        inherits from this.

        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java

        State of a region in transition. Top-level because used by a

        few top-level packages. Encapsulates pb serialization/deserialization.

        M src/main/java/org/apache/hadoop/hbase/ServerName.java

        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java

        Counters used by distributed log splitting.

        A SplitLogTask

        Class that encapsulates log splitting state. Also encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java

        Implement code for state. Added functions to go from code to state and vice

        versa. Used serializing.

        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java

        Remove unused imports.

        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java

        Removed. Replaced by RegionTransition moved to package top-level.

        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

        Use new DeserializationException. Move to using new RegionTransition

        from RegionTransitionData class. Pass deserialized class rather than

        byte array. Remove duplicated code.

        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java

        Use new ServerName parse method rather than ZKUtil one.

        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java

        Redo to use new SplitLogTask and SplitLogCounter classes.

        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

        expectPBMagicPrefix added

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        Use new RegionTransition in place of RegionTransitionData.

        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java

        Define moved from ZKSplitLog to SplitLogManager.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

        Changed method name from getZNodeData to toByteArray to match how we've

        named it elsewhere. Use new DeserializationException

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

        Use new RegionTransion class

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java

        Moved stuff that was in here up into SplitLogManager where better

        belongs. Also moved serialization/deserialization up into the

        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java

        Moved deserialization of ServerName out of here and up into ServerName.

        M src/main/protobuf/ZooKeeper.proto

        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.

        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2

        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508

        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377

        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6

        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/SplitLogManager.java 919c65f

        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76

        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c

        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af

        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde

        src/main/protobuf/ZooKeeper.proto 961ab65

        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889

        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48

        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb

        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8

        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876

        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90

        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba

        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a

        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572

        Diff: https://reviews.apache.org/r/4926/diff

        Testing

        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-04-28 22:14:23, Jimmy Xiang wrote: > src/main/protobuf/ZooKeeper.proto, line 82 > < https://reviews.apache.org/r/4926/diff/1/?file=105372#file105372line82 > > > A task is a path, this is more like a task state, isn't it? I can change this np. Currently I have the pb class named same as the class that wraps it. Should I change this? Add a pb prefix or something? Problem w/ that is that no other of the pb classes have the pb prefix. They are in the generated package which is probably sufficient to distingush them? My hope is to make it so the pbs do not leak outside of the class that serializes to them; e.g. this SplitLogTask class. On 2012-04-28 22:14:23, Jimmy Xiang wrote: > src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java, line 798 > < https://reviews.apache.org/r/4926/diff/1/?file=105369#file105369line798 > > > I see this many places. Can we put the whole block in a util method? Probably will be easier to read? Will do. On 2012-04-28 22:14:23, Jimmy Xiang wrote: > src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java, line 182 > < https://reviews.apache.org/r/4926/diff/1/?file=105357#file105357line182 > > > Should we abort? Under what scenario the parsing can fail, other than a conflict data format? I thought I was just redoing what was there previous. We could abort but maybe next time through the deserialization works because its been updated by another? Or, we spew this error all over the logs and drive someone crazy? Will look at it again. Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/#review7360 ----------------------------------------------------------- On 2012-04-28 18:10:24, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-04-28 18:10:24) Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs ----- src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/#review7360
        -----------------------------------------------------------

        Looks good. Great stuff.

        src/main/java/org/apache/hadoop/hbase/HBaseException.java
        <https://reviews.apache.org/r/4926/#comment16239>

        That's great.

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java
        <https://reviews.apache.org/r/4926/#comment16240>

        Should we abort? Under what scenario the parsing can fail, other than a conflict data format?

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
        <https://reviews.apache.org/r/4926/#comment16241>

        I see this many places. Can we put the whole block in a util method? Probably will be easier to read?

        src/main/protobuf/ZooKeeper.proto
        <https://reviews.apache.org/r/4926/#comment16242>

        A task is a path, this is more like a task state, isn't it?

        • Jimmy

        On 2012-04-28 18:10:24, Michael Stack wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4926/

        -----------------------------------------------------------

        (Updated 2012-04-28 18:10:24)

        Review request for hbase and Jimmy Xiang.

        Summary

        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.

        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into

        the classes themselves so can encapsulate how serialization is done into one place

        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports

        that are across package (zookeeper into protobuf and/or into regionserver and/or

        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java

        New generic deserialization exception.

        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java

        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java

        Moved under zookeeper package.

        A src/main/java/org/apache/hadoop/hbase/HBaseException.java

        New base hbase exception as suggested by hbase-5796. New DeserializationException

        inherits from this.

        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java

        State of a region in transition. Top-level because used by a

        few top-level packages. Encapsulates pb serialization/deserialization.

        M src/main/java/org/apache/hadoop/hbase/ServerName.java

        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java

        Counters used by distributed log splitting.

        A SplitLogTask

        Class that encapsulates log splitting state. Also encapsulates pb'ing.

        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java

        Implement code for state. Added functions to go from code to state and vice

        versa. Used serializing.

        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java

        Remove unused imports.

        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java

        Removed. Replaced by RegionTransition moved to package top-level.

        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

        Use new DeserializationException. Move to using new RegionTransition

        from RegionTransitionData class. Pass deserialized class rather than

        byte array. Remove duplicated code.

        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java

        Use new ServerName parse method rather than ZKUtil one.

        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java

        Redo to use new SplitLogTask and SplitLogCounter classes.

        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

        expectPBMagicPrefix added

        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java

        Use new RegionTransition in place of RegionTransitionData.

        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java

        Define moved from ZKSplitLog to SplitLogManager.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

        Changed method name from getZNodeData to toByteArray to match how we've

        named it elsewhere. Use new DeserializationException

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java

        Use new RegionTransion class

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java

        Moved stuff that was in here up into SplitLogManager where better

        belongs. Also moved serialization/deserialization up into the

        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.

        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java

        Moved deserialization of ServerName out of here and up into ServerName.

        M src/main/protobuf/ZooKeeper.proto

        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.

        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs

        -----

        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2

        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624

        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508

        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377

        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70

        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6

        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/SplitLogManager.java 919c65f

        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76

        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f

        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4

        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c

        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION

        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af

        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9

        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde

        src/main/protobuf/ZooKeeper.proto 961ab65

        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889

        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48

        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb

        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8

        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876

        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90

        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba

        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865

        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4

        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a

        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572

        Diff: https://reviews.apache.org/r/4926/diff

        Testing

        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/#review7360 ----------------------------------------------------------- Looks good. Great stuff. src/main/java/org/apache/hadoop/hbase/HBaseException.java < https://reviews.apache.org/r/4926/#comment16239 > That's great. src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java < https://reviews.apache.org/r/4926/#comment16240 > Should we abort? Under what scenario the parsing can fail, other than a conflict data format? src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java < https://reviews.apache.org/r/4926/#comment16241 > I see this many places. Can we put the whole block in a util method? Probably will be easier to read? src/main/protobuf/ZooKeeper.proto < https://reviews.apache.org/r/4926/#comment16242 > A task is a path, this is more like a task state, isn't it? Jimmy On 2012-04-28 18:10:24, Michael Stack wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- (Updated 2012-04-28 18:10:24) Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs ----- src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        stack added a comment -

        Trying against hadoopqa to see whats broke.

        Show
        stack added a comment - Trying against hadoopqa to see whats broke.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4926/
        -----------------------------------------------------------

        Review request for hbase and Jimmy Xiang.

        Summary
        -------

        Convert two zk users to pb: distributed log splitting and regions in transition.

        Refactored distributed log splitting so we only serialize/deserialize in one location.
        Less changes needed to do same for regions in transition.

        Moves serialization/deserialization out of the ZKAssign, ZKSplit and into
        the classes themselves so can encapsulate how serialization is done into one place
        (try to make the ZK* classes just deal in bytes – about 90% done).

        Moved classes used by various packages up to top level to minimize imports
        that are across package (zookeeper into protobuf and/or into regionserver and/or
        master packages, etc).

        A src/main/java/org/apache/hadoop/hbase/DeserializationException.java
        New generic deserialization exception.
        A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java
        D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java
        Moved under zookeeper package.
        A src/main/java/org/apache/hadoop/hbase/HBaseException.java
        New base hbase exception as suggested by hbase-5796. New DeserializationException
        inherits from this.
        A src/main/java/org/apache/hadoop/hbase/RegionTransition.java
        State of a region in transition. Top-level because used by a
        few top-level packages. Encapsulates pb serialization/deserialization.
        M src/main/java/org/apache/hadoop/hbase/ServerName.java
        Add method to deserialize a ServeName, etc. Encapsulates pb'ing.
        M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java
        Counters used by distributed log splitting.
        A SplitLogTask
        Class that encapsulates log splitting state. Also encapsulates pb'ing.
        M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
        Implement code for state. Added functions to go from code to state and vice
        versa. Used serializing.
        M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
        Remove unused imports.
        D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java
        Removed. Replaced by RegionTransition moved to package top-level.
        M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java
        M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
        Use new DeserializationException. Move to using new RegionTransition
        from RegionTransitionData class. Pass deserialized class rather than
        byte array. Remove duplicated code.
        M src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        Use new ServerName parse method rather than ZKUtil one.
        M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java
        Redo to use new SplitLogTask and SplitLogCounter classes.
        M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
        expectPBMagicPrefix added
        M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
        Use new RegionTransition in place of RegionTransitionData.
        M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java
        Define moved from ZKSplitLog to SplitLogManager.
        M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java
        M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java
        Changed method name from getZNodeData to toByteArray to match how we've
        named it elsewhere. Use new DeserializationException
        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
        Use new RegionTransion class
        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java
        Moved stuff that was in here up into SplitLogManager where better
        belongs. Also moved serialization/deserialization up into the
        class itself: SplitLogTask. Moved counters out to SplitLogCounter class.
        M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
        Moved deserialization of ServerName out of here and up into ServerName.
        M src/main/protobuf/ZooKeeper.proto
        Add two new classes, RegionTransition and SplitLogTask.

        This addresses bug HBASE-5869.
        https://issues.apache.org/jira/browse/HBASE-5869

        Diffs


        src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2
        src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624
        src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508
        src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377
        src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70
        src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6
        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/SplitLogManager.java 919c65f
        src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76
        src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc
        src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f
        src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4
        src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c
        src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION
        src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af
        src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80
        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672
        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9
        src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde
        src/main/protobuf/ZooKeeper.proto 961ab65
        src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889
        src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48
        src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb
        src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8
        src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876
        src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90
        src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba
        src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e
        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865
        src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea
        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4
        src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a
        src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572

        Diff: https://reviews.apache.org/r/4926/diff

        Testing
        -------

        Thanks,

        Michael

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4926/ ----------------------------------------------------------- Review request for hbase and Jimmy Xiang. Summary ------- Convert two zk users to pb: distributed log splitting and regions in transition. Refactored distributed log splitting so we only serialize/deserialize in one location. Less changes needed to do same for regions in transition. Moves serialization/deserialization out of the ZKAssign, ZKSplit and into the classes themselves so can encapsulate how serialization is done into one place (try to make the ZK* classes just deal in bytes – about 90% done). Moved classes used by various packages up to top level to minimize imports that are across package (zookeeper into protobuf and/or into regionserver and/or master packages, etc). A src/main/java/org/apache/hadoop/hbase/DeserializationException.java New generic deserialization exception. A src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java D src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java Moved under zookeeper package. A src/main/java/org/apache/hadoop/hbase/HBaseException.java New base hbase exception as suggested by hbase-5796. New DeserializationException inherits from this. A src/main/java/org/apache/hadoop/hbase/RegionTransition.java State of a region in transition. Top-level because used by a few top-level packages. Encapsulates pb serialization/deserialization. M src/main/java/org/apache/hadoop/hbase/ServerName.java Add method to deserialize a ServeName, etc. Encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java Counters used by distributed log splitting. A SplitLogTask Class that encapsulates log splitting state. Also encapsulates pb'ing. M src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java Implement code for state. Added functions to go from code to state and vice versa. Used serializing. M src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java Remove unused imports. D src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java Removed. Replaced by RegionTransition moved to package top-level. M src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Use new DeserializationException. Move to using new RegionTransition from RegionTransitionData class. Pass deserialized class rather than byte array. Remove duplicated code. M src/main/java/org/apache/hadoop/hbase/master/HMaster.java Use new ServerName parse method rather than ZKUtil one. M src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java M src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java Redo to use new SplitLogTask and SplitLogCounter classes. M src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java expectPBMagicPrefix added M src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java Use new RegionTransition in place of RegionTransitionData. M src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java Define moved from ZKSplitLog to SplitLogManager. M src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java Changed method name from getZNodeData to toByteArray to match how we've named it elsewhere. Use new DeserializationException M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Use new RegionTransion class M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java Moved stuff that was in here up into SplitLogManager where better belongs. Also moved serialization/deserialization up into the class itself: SplitLogTask. Moved counters out to SplitLogCounter class. M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Moved deserialization of ServerName out of here and up into ServerName. M src/main/protobuf/ZooKeeper.proto Add two new classes, RegionTransition and SplitLogTask. This addresses bug HBASE-5869 . https://issues.apache.org/jira/browse/HBASE-5869 Diffs src/main/java/org/apache/hadoop/hbase/DeserializationException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/EmptyWatcher.java 9881ec2 src/main/java/org/apache/hadoop/hbase/HBaseException.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/RegionTransition.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/ServerName.java 8fdb624 src/main/java/org/apache/hadoop/hbase/SplitLogCounters.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/SplitLogTask.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java 4121508 src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 06ca377 src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionData.java 35d7b70 src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java 47e3bd6 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/SplitLogManager.java 919c65f src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java 994cb76 src/main/java/org/apache/hadoop/hbase/protobuf/generated/ZooKeeperProtos.java 8457bdc src/main/java/org/apache/hadoop/hbase/regionserver/SplitLogWorker.java 8ea342f src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java ea12da4 src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java 587386c src/main/java/org/apache/hadoop/hbase/zookeeper/EmptyWatcher.java PRE-CREATION src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java f9575af src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java babde80 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java e94b672 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKSplitLog.java 30d7fe9 src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 46a6fde src/main/protobuf/ZooKeeper.proto 961ab65 src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java c3a1889 src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java d496d48 src/test/java/org/apache/hadoop/hbase/master/Mocking.java 676d6bb src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java 36046f8 src/test/java/org/apache/hadoop/hbase/master/TestDistributedLogSplitting.java 2669876 src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java 14cdb90 src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java f8029ba src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java 0f7d54e src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitLogWorker.java 26b9865 src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java 75b5aea src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java 07f8fc4 src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java 55a8c4a src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java 4314572 Diff: https://reviews.apache.org/r/4926/diff Testing ------- Thanks, Michael
        Hide
        stack added a comment -

        Attaching latest. Will put up on review board and try it against hadoopqa.

        Show
        stack added a comment - Attaching latest. Will put up on review board and try it against hadoopqa.
        Hide
        stack added a comment -

        Compiles

        Show
        stack added a comment - Compiles
        Hide
        stack added a comment -

        v4 I still have to convert the tests. This is taking a while. We serialize and deserialize from zk all over our codebase. Takes a while to chase down all cases.

        Show
        stack added a comment - v4 I still have to convert the tests. This is taking a while. We serialize and deserialize from zk all over our codebase. Takes a while to chase down all cases.
        Hide
        stack added a comment -

        Second cut. Still not done.

        Show
        stack added a comment - Second cut. Still not done.
        Hide
        stack added a comment -

        Not done yet. WIP.

        Show
        stack added a comment - Not done yet. WIP.

          People

          • Assignee:
            stack
            Reporter:
            stack
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development