HBase
  1. HBase
  2. HBASE-4558

Refactor TestOpenedRegionHandler and TestOpenRegionHandler.

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.92.0
    • Component/s: None
    • Labels:
      None

      Description

      This is an improvement task taken up to refactor TestOpenedRegionandler and TestOpenRegionHandler so that MockServer and MockRegionServerServices can be accessed from a common utility package.
      If we do this then one of the testcases in TestOpenedRegionHandler need not start up a cluster and also moving it into a common package will help in mocking the server for future testcases.

      1. HBASE-4558_1.patch
        18 kB
        ramkrishna.s.vasudevan
      2. HBASE-4558_2.patch
        18 kB
        ramkrishna.s.vasudevan
      3. HBASE-4558_3.patch
        18 kB
        ramkrishna.s.vasudevan

        Activity

        Hide
        ramkrishna.s.vasudevan added a comment -

        Refactors MockServer and MockRegionServer to org.apache.hadoop.hbase.util package.
        TestOpenedRegionHandler.testShouldNotCompeleteOpenedRegionSuccessfullyIfVersionMismatches() now will not use the mini cluster except for minizkCluster.

        Show
        ramkrishna.s.vasudevan added a comment - Refactors MockServer and MockRegionServer to org.apache.hadoop.hbase.util package. TestOpenedRegionHandler.testShouldNotCompeleteOpenedRegionSuccessfullyIfVersionMismatches() now will not use the mini cluster except for minizkCluster.
        Hide
        Ted Yu added a comment -

        Patch looks good.

        +    // Should not invoke assignmentmanager.regionOnline. If it invokes as per
        +    // the
        +    // current mocking we have done it will throw null pointer.
        

        'the' in the middle is not needed. How about the following ?

        +    // Should not invoke assignmentmanager.regionOnline. If it is invoked as per
        +    // current mocking it will throw null pointer exception.
        
        Show
        Ted Yu added a comment - Patch looks good. + // Should not invoke assignmentmanager.regionOnline. If it invokes as per + // the + // current mocking we have done it will throw null pointer. 'the' in the middle is not needed. How about the following ? + // Should not invoke assignmentmanager.regionOnline. If it is invoked as per + // current mocking it will throw null pointer exception.
        Hide
        ramkrishna.s.vasudevan added a comment -

        New patch uploaded addressing Ted's comment.

        Show
        ramkrishna.s.vasudevan added a comment - New patch uploaded addressing Ted's comment.
        Hide
        Ted Yu added a comment -

        +1 on patch v2.

        Show
        Ted Yu added a comment - +1 on patch v2.
        Hide
        stack added a comment -

        In testOpenedRegionHandlerOnMasterRestart, after you start the cluster, do you open a try so you can catch any exceptions and for sure shutdown the cluster in a finally block? If not, add on commit.

        +1 on commit. Thanks for making this fix Ram.

        Show
        stack added a comment - In testOpenedRegionHandlerOnMasterRestart, after you start the cluster, do you open a try so you can catch any exceptions and for sure shutdown the cluster in a finally block? If not, add on commit. +1 on commit. Thanks for making this fix Ram.
        Hide
        ramkrishna.s.vasudevan added a comment -

        @Stack
        Though we start the minicluster inside the testcase the minicluster shutdown is called in teardown. That will for sure shutdown the cluster.

        Show
        ramkrishna.s.vasudevan added a comment - @Stack Though we start the minicluster inside the testcase the minicluster shutdown is called in teardown. That will for sure shutdown the cluster.
        Hide
        ramkrishna.s.vasudevan added a comment -

        Added a try finally block.

        Show
        ramkrishna.s.vasudevan added a comment - Added a try finally block.
        Hide
        Jonathan Gray added a comment -

        Did this break the build? TestMasterFailover is not compiling for me.

        Show
        Jonathan Gray added a comment - Did this break the build? TestMasterFailover is not compiling for me.
        Hide
        Jonathan Gray added a comment -
        • metaRegion, regionServer);
          + metaRegion, regionServer.getServerName());

        ?

        Show
        Jonathan Gray added a comment - metaRegion, regionServer); + metaRegion, regionServer.getServerName()); ?
        Hide
        ramkrishna.s.vasudevan added a comment -

        @Jon
        Yes. sorry for that ..will correct that immediately.

        Show
        ramkrishna.s.vasudevan added a comment - @Jon Yes. sorry for that ..will correct that immediately.
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92 #60 (See https://builds.apache.org/job/HBase-0.92/60/)
        HBASE-4558 - Addendum for TestMasterFailover (Ram)
        HBASE-4558 Refactor TestOpenedRegionHandler and TestOpenRegionHandler. (Ram)

        ramkrishna :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java

        ramkrishna :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockRegionServerServices.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockServer.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/MockRegionServerServices.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/MockServer.java
        Show
        Hudson added a comment - Integrated in HBase-0.92 #60 (See https://builds.apache.org/job/HBase-0.92/60/ ) HBASE-4558 - Addendum for TestMasterFailover (Ram) HBASE-4558 Refactor TestOpenedRegionHandler and TestOpenRegionHandler. (Ram) ramkrishna : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java ramkrishna : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockRegionServerServices.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockServer.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/MockRegionServerServices.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/MockServer.java
        Hide
        Lars Hofhansl added a comment -

        I don't think the addendum needs to be mentioned in CHANGES.txt. A user looking at the set of changes won't care

        Show
        Lars Hofhansl added a comment - I don't think the addendum needs to be mentioned in CHANGES.txt. A user looking at the set of changes won't care
        Hide
        ramkrishna.s.vasudevan added a comment -

        Resolved in trunk and 0.92

        Show
        ramkrishna.s.vasudevan added a comment - Resolved in trunk and 0.92
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2325 (See https://builds.apache.org/job/HBase-TRUNK/2325/)
        HBASE-4558 - Addendum for TestMasterFailOver (Ram)
        HBASE-4558 Refactor TestOpenedRegionHandler and TestOpenRegionHandler. (Ram)

        ramkrishna :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java

        ramkrishna :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockRegionServerServices.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockServer.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/MockRegionServerServices.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/MockServer.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2325 (See https://builds.apache.org/job/HBase-TRUNK/2325/ ) HBASE-4558 - Addendum for TestMasterFailOver (Ram) HBASE-4558 Refactor TestOpenedRegionHandler and TestOpenRegionHandler. (Ram) ramkrishna : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java ramkrishna : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockRegionServerServices.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockServer.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/MockRegionServerServices.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/MockServer.java

          People

          • Assignee:
            ramkrishna.s.vasudevan
            Reporter:
            ramkrishna.s.vasudevan
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development