HBase
  1. HBase
  2. HBASE-4540

OpenedRegionHandler is not enforcing atomicity of the operation it is performing

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.90.5
    • Component/s: None
    • Labels:
      None

      Description

      -> OpenedRegionHandler has not yet deleted the znode of the region R1 opened by RS1.
      -> RS1 goes down.
      -> Servershutdownhandler assigns the region R1 to RS2.
      -> The znode of R1 is moved to OFFLINE state by master or OPENING state by RS2 if RS2 has started opening the region.
      -> Now the first OpenedRegionHandler tries to delete the znode thinking its in OPENED state but fails.
      -> Though it fails it removes the node from RIT and adds RS1 as the owner of R1 in master's memory.
      -> Now when RS2 completes opening the region the master is not able to open the region as already the reigon has been deleted from RIT.

      Master
      ======
      2011-10-05 20:49:45,301 INFO org.apache.hadoop.hbase.master.handler.ServerShutdownHandler: Finished processing of shutdown of linux146,60020,1317827727647
      2011-10-05 20:49:54,177 DEBUG org.apache.hadoop.hbase.master.HMaster: Not running balancer because 1 region(s) in transition: {3e69d628a8bd8e9b7c5e7a2a6e03aad9=t1,,1317827883842.3e69d628a8bd8e9b7c5e7a2a6e03aad9. state=PENDING_OPEN, ts=1317827985272, server=linux76,60020,1317827746847}
      2011-10-05 20:49:57,720 DEBUG org.apache.hadoop.hbase.master.AssignmentManager: Handling transition=M_ZK_REGION_OFFLINE, server=linux76,60000,1317827742012, region=3e69d628a8bd8e9b7c5e7a2a6e03aad9
      2011-10-05 20:50:14,501 DEBUG org.apache.hadoop.hbase.zookeeper.ZKAssign: master:60000-0x132d3dc13090023 Deleting existing unassigned node for 3e69d628a8bd8e9b7c5e7a2a6e03aad9 that is in expected state RS_ZK_REGION_OPENED
      2011-10-05 20:50:14,505 WARN org.apache.hadoop.hbase.zookeeper.ZKAssign: master:60000-0x132d3dc13090023 Attempting to delete unassigned node 3e69d628a8bd8e9b7c5e7a2a6e03aad9 in RS_ZK_REGION_OPENED state but node is in RS_ZK_REGION_OPENING state
      
      After the region is opened in RS2
      =================================
      2011-10-05 20:50:48,066 DEBUG org.apache.hadoop.hbase.master.AssignmentManager: Handling transition=RS_ZK_REGION_OPENING, server=linux76,60020,1317827746847, region=3e69d628a8bd8e9b7c5e7a2a6e03aad9, which is more than 15 seconds late
      2011-10-05 20:50:48,290 WARN org.apache.hadoop.hbase.master.AssignmentManager: Received OPENING for region 3e69d628a8bd8e9b7c5e7a2a6e03aad9 from server linux76,60020,1317827746847 but region was in  the state null and not in expected PENDING_OPEN or OPENING states
      2011-10-05 20:50:53,743 DEBUG org.apache.hadoop.hbase.master.AssignmentManager: Handling transition=RS_ZK_REGION_OPENING, server=linux76,60020,1317827746847, region=3e69d628a8bd8e9b7c5e7a2a6e03aad9
      2011-10-05 20:50:54,182 DEBUG org.apache.hadoop.hbase.master.CatalogJanitor: Scanned 1 catalog row(s) and gc'd 0 unreferenced parent region(s)
      2011-10-05 20:50:54,397 WARN org.apache.hadoop.hbase.master.AssignmentManager: Received OPENING for region 3e69d628a8bd8e9b7c5e7a2a6e03aad9 from server linux76,60020,1317827746847 but region was in  the state null and not in expected PENDING_OPEN or OPENING states
      
      1. HBASE-4540_90_1.patch
        37 kB
        ramkrishna.s.vasudevan
      2. HBASE-4540_90.patch
        37 kB
        ramkrishna.s.vasudevan
      3. HBASE-4540_1.patch
        14 kB
        ramkrishna.s.vasudevan

        Issue Links

          Activity

          Hide
          ramkrishna.s.vasudevan added a comment -
              try {
                ZKAssign.deleteOpenedNode(server.getZooKeeper(),
                    regionInfo.getEncodedName());
              } catch (KeeperException e) {
                server.abort("Error deleting OPENED node in ZK for transition ZK node ("
                  + regionInfo.getRegionNameAsString() + ")", e);
              }
          

          The return type of deleteOpenedNode is not getting used. Using it may solve the problem

          Show
          ramkrishna.s.vasudevan added a comment - try { ZKAssign.deleteOpenedNode(server.getZooKeeper(), regionInfo.getEncodedName()); } catch (KeeperException e) { server.abort( "Error deleting OPENED node in ZK for transition ZK node (" + regionInfo.getRegionNameAsString() + ")" , e); } The return type of deleteOpenedNode is not getting used. Using it may solve the problem
          Hide
          ramkrishna.s.vasudevan added a comment -

          I have not yet completely run the test suite.
          Once run will let you know the results.

          Show
          ramkrishna.s.vasudevan added a comment - I have not yet completely run the test suite. Once run will let you know the results.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Addresses HBASE-4539 also.

          Show
          ramkrishna.s.vasudevan added a comment - Addresses HBASE-4539 also.
          Hide
          ramkrishna.s.vasudevan added a comment -

          The reason for getting the znode version for the following scenario
          -> RS1 tries opening a region by transiting it to OPENED
          ->OpenedRegionHandler has still not processed.
          -> RS1 goes down and the region is assigned to RS2.
          -> RS2 has transited the node to OPENED
          -> Now the OpenedRegionHandler will try to delete the znode and it will succeed thinking the region is in RS1.
          -> To avoid the above scenario i have tried to use the znode version that comes along when we get the callback after transiting the node to OPENED state.

          Show
          ramkrishna.s.vasudevan added a comment - The reason for getting the znode version for the following scenario -> RS1 tries opening a region by transiting it to OPENED ->OpenedRegionHandler has still not processed. -> RS1 goes down and the region is assigned to RS2. -> RS2 has transited the node to OPENED -> Now the OpenedRegionHandler will try to delete the znode and it will succeed thinking the region is in RS1. -> To avoid the above scenario i have tried to use the znode version that comes along when we get the callback after transiting the node to OPENED state.
          Hide
          Jonathan Gray added a comment -

          Looks pretty good. Once you get the unit tests passing, want to put it up on RB?

          Also, it'd be really good if you could start thinking about how to mock these scenarios better in our unit tests. You are finding lots of great bugs but without tests it will be hard to prevent regressions.

          Show
          Jonathan Gray added a comment - Looks pretty good. Once you get the unit tests passing, want to put it up on RB? Also, it'd be really good if you could start thinking about how to mock these scenarios better in our unit tests. You are finding lots of great bugs but without tests it will be hard to prevent regressions.
          Hide
          Jean-Daniel Cryans added a comment -

          This reminds me of HBASE-4416.

          Show
          Jean-Daniel Cryans added a comment - This reminds me of HBASE-4416 .
          Hide
          Ted Yu added a comment -

          For ZKAssign.deleteNode(), the following can be removed:

              LOG.debug(zkw.prefix("Successfully deleted unassigned node for region " +
                  regionName + " in expected state " + expectedState));
          
          Show
          Ted Yu added a comment - For ZKAssign.deleteNode(), the following can be removed: LOG.debug(zkw.prefix( "Successfully deleted unassigned node for region " + regionName + " in expected state " + expectedState));
          Hide
          Ted Yu added a comment -

          For ZKAssign.deleteNode(), the javadoc needs to be updated as it covers both opened and closed states:

             * <p>Returns false if the node was not in the proper state but did exist.
             *
             * <p>This method is used during table disables when a region finishes
             * successfully closing.  This is the Master acknowledging completion
             * of the specified regions transition to being closed.
          
          Show
          Ted Yu added a comment - For ZKAssign.deleteNode(), the javadoc needs to be updated as it covers both opened and closed states: * <p>Returns false if the node was not in the proper state but did exist. * * <p>This method is used during table disables when a region finishes * successfully closing. This is the Master acknowledging completion * of the specified regions transition to being closed.
          Hide
          ramkrishna.s.vasudevan added a comment -

          @JD
          Yes the bug is same as hbase-4416. I did not know one already was there.
          @Jon
          I am working on writing test cases
          Will submit it once done

          Thanks guys for your reviews
          @Ted
          will address your comment in the updated patch.

          Show
          ramkrishna.s.vasudevan added a comment - @JD Yes the bug is same as hbase-4416. I did not know one already was there. @Jon I am working on writing test cases Will submit it once done Thanks guys for your reviews @Ted will address your comment in the updated patch.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary
          -------

          Fix for handling HBASE-4539 and HBASE-4540.
          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.
          Also addresses Ted's comments.

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

          Diffs


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing
          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
          <https://reviews.apache.org/r/2251/#comment5498>

          The two tests share a lot of the same code, some refactoring would be good

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
          <https://reviews.apache.org/r/2251/#comment5497>

          You should be resetting the conf to what was created inside TEST_UTIL.

          • Jean-Daniel

          On 2011-10-06 17:55:05, ramkrishna vasudevan wrote:

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

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

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

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

          (Updated 2011-10-06 17:55:05)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary

          -------

          Fix for handling HBASE-4539 and HBASE-4540.

          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.

          Also addresses Ted's comments.

          This addresses bug HBASE-4540.

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

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing

          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/#review2395 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java < https://reviews.apache.org/r/2251/#comment5498 > The two tests share a lot of the same code, some refactoring would be good http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java < https://reviews.apache.org/r/2251/#comment5497 > You should be resetting the conf to what was created inside TEST_UTIL. Jean-Daniel On 2011-10-06 17:55:05, ramkrishna vasudevan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-06 17:55:05) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java
          <https://reviews.apache.org/r/2251/#comment5504>

          Can this debugLog be folded into the one above ?

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java
          <https://reviews.apache.org/r/2251/#comment5505>

          Remove this extra line.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java
          <https://reviews.apache.org/r/2251/#comment5506>

          'for transition ZK node' seems redundant.

          • Ted

          On 2011-10-06 17:55:05, ramkrishna vasudevan wrote:

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

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

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

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

          (Updated 2011-10-06 17:55:05)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary

          -------

          Fix for handling HBASE-4539 and HBASE-4540.

          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.

          Also addresses Ted's comments.

          This addresses bug HBASE-4540.

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

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing

          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/#review2399 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java < https://reviews.apache.org/r/2251/#comment5504 > Can this debugLog be folded into the one above ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java < https://reviews.apache.org/r/2251/#comment5505 > Remove this extra line. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java < https://reviews.apache.org/r/2251/#comment5506 > 'for transition ZK node' seems redundant. Ted On 2011-10-06 17:55:05, ramkrishna vasudevan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-06 17:55:05) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
          <https://reviews.apache.org/r/2251/#comment5510>

          This new method is similar to deleteNode() above.
          Maybe we should retrofit the existing deleteNode() by adding expectedVersion ?
          We can designate some negative constant to signify that version check should be skipped.

          • Ted

          On 2011-10-06 17:55:05, ramkrishna vasudevan wrote:

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

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

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

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

          (Updated 2011-10-06 17:55:05)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary

          -------

          Fix for handling HBASE-4539 and HBASE-4540.

          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.

          Also addresses Ted's comments.

          This addresses bug HBASE-4540.

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

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing

          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/#review2400 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java < https://reviews.apache.org/r/2251/#comment5510 > This new method is similar to deleteNode() above. Maybe we should retrofit the existing deleteNode() by adding expectedVersion ? We can designate some negative constant to signify that version check should be skipped. Ted On 2011-10-06 17:55:05, ramkrishna vasudevan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-06 17:55:05) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
          <https://reviews.apache.org/r/2251/#comment5519>

          Yes Ted. I too was thinking of unifying both the deleteNode() apis.
          Was thinking what can the expectedVersion that can be passed when we need not check it. Can we pass -1? and check if -1 is passed for expectedVersion we will skip that check.

          • ramkrishna

          On 2011-10-06 17:55:05, ramkrishna vasudevan wrote:

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

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

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

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

          (Updated 2011-10-06 17:55:05)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary

          -------

          Fix for handling HBASE-4539 and HBASE-4540.

          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.

          Also addresses Ted's comments.

          This addresses bug HBASE-4540.

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

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing

          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/#review2425 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java < https://reviews.apache.org/r/2251/#comment5519 > Yes Ted. I too was thinking of unifying both the deleteNode() apis. Was thinking what can the expectedVersion that can be passed when we need not check it. Can we pass -1? and check if -1 is passed for expectedVersion we will skip that check. ramkrishna On 2011-10-06 17:55:05, ramkrishna vasudevan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-06 17:55:05) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179238 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          (Updated 2011-10-07 14:27:20.231903)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Changes
          -------

          LOG.debug(zkw.prefix("Successfully deleted unassigned node for region " +
          regionName + " in expected state " + expectedState));
          @Ted - I have not removed this log so that it can be used for debugging.
          Refactored the testcase and made it much simpler so that it doesn't take much time.

          Summary
          -------

          Fix for handling HBASE-4539 and HBASE-4540.
          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.
          Also addresses Ted's comments.

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

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945

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

          Testing
          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-07 14:27:20.231903) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Changes ------- LOG.debug(zkw.prefix("Successfully deleted unassigned node for region " + regionName + " in expected state " + expectedState)); @Ted - I have not removed this log so that it can be used for debugging. Refactored the testcase and made it much simpler so that it doesn't take much time. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945 Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          Ted Yu added a comment -

          For Ram's comment @ 07/Oct/11 07:22
          Since -1 is a possible return value from ZKAssign methods, I think we should use other values such as -2.

          Show
          Ted Yu added a comment - For Ram's comment @ 07/Oct/11 07:22 Since -1 is a possible return value from ZKAssign methods, I think we should use other values such as -2.
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Ted
          I just uploaded the patch before you had commented this. In that patch i had used -1.
          So if we are going to use -2 or some negative value is it ok to add in javadoc something like

          • @param expectedVersion of the znode that is to be deleted.
          • If expectedVersion need not be compared while deleting the znode
          • pass -2(NEGATIVE_VERSION)
            Is it ok Ted?
          Show
          ramkrishna.s.vasudevan added a comment - @Ted I just uploaded the patch before you had commented this. In that patch i had used -1. So if we are going to use -2 or some negative value is it ok to add in javadoc something like @param expectedVersion of the znode that is to be deleted. If expectedVersion need not be compared while deleting the znode pass -2(NEGATIVE_VERSION) Is it ok Ted?
          Hide
          ramkrishna.s.vasudevan added a comment -

          Can we better document like anything less than some value. may be either 0 or -1? Instead of going with one value.

          Show
          ramkrishna.s.vasudevan added a comment - Can we better document like anything less than some value. may be either 0 or -1? Instead of going with one value.
          Hide
          Ted Yu added a comment -

          We may designate some negative value for other purpose in the future.
          I think using one known value is recommended.
          The Javadoc addition above is nice.

          Show
          Ted Yu added a comment - We may designate some negative value for other purpose in the future. I think using one known value is recommended. The Javadoc addition above is nice.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          (Updated 2011-10-07 16:13:33.022073)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Changes
          -------

          If we do not want to compare the version of znode while deleting we can pass -2 to the deleteNode api.
          Uploaded the patch with the change.

          Summary
          -------

          Fix for handling HBASE-4539 and HBASE-4540.
          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.
          Also addresses Ted's comments.

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

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing
          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-07 16:13:33.022073) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Changes ------- If we do not want to compare the version of znode while deleting we can pass -2 to the deleteNode api. Uploaded the patch with the change. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java
          <https://reviews.apache.org/r/2251/#comment5524>

          Space between } and catch, please.

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
          <https://reviews.apache.org/r/2251/#comment5525>

          Should we expose this constant as public ?
          How about naming this constant DONT_COMPARE_VERSION or NO_VERSION_COMPARISON ?

          • Ted

          On 2011-10-07 16:13:33, ramkrishna vasudevan wrote:

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

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

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

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

          (Updated 2011-10-07 16:13:33)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary

          -------

          Fix for handling HBASE-4539 and HBASE-4540.

          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.

          Also addresses Ted's comments.

          This addresses bug HBASE-4540.

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

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing

          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/#review2431 ----------------------------------------------------------- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java < https://reviews.apache.org/r/2251/#comment5524 > Space between } and catch, please. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java < https://reviews.apache.org/r/2251/#comment5525 > Should we expose this constant as public ? How about naming this constant DONT_COMPARE_VERSION or NO_VERSION_COMPARISON ? Ted On 2011-10-07 16:13:33, ramkrishna vasudevan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-07 16:13:33) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          Ted Yu added a comment -

          @Ramkrishna:
          ZKAssign.transitionNode() is already using -1 to indicate no version comparison.
          Your patch @ 07/Oct/11 14:27 should be good.

          Sorry for the confusion.

          Show
          Ted Yu added a comment - @Ramkrishna: ZKAssign.transitionNode() is already using -1 to indicate no version comparison. Your patch @ 07/Oct/11 14:27 should be good. Sorry for the confusion.
          Hide
          ramkrishna.s.vasudevan added a comment -

          If any node exists the version will start from 0.
          Thanks Ted for the confirmation. I will wait for one day for further reviews and will make changes accordingly if not will take the patc at @ 07/Oct/11 14:27.
          The space between catch and } i will take care.

          Show
          ramkrishna.s.vasudevan added a comment - If any node exists the version will start from 0. Thanks Ted for the confirmation. I will wait for one day for further reviews and will make changes accordingly if not will take the patc at @ 07/Oct/11 14:27. The space between catch and } i will take care.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          (Updated 2011-10-08 05:13:32.657832)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Changes
          -------

          This updated patch is same as uploaded at @ 07/Oct/11 14:27
          Reverted the change of passing -2 for not comparing the version and address Ted's comment to add spaces.

          Summary
          -------

          Fix for handling HBASE-4539 and HBASE-4540.
          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.
          Also addresses Ted's comments.

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

          Diffs (updated)


          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945
          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing
          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-08 05:13:32.657832) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Changes ------- This updated patch is same as uploaded at @ 07/Oct/11 14:27 Reverted the change of passing -2 for not comparing the version and address Ted's comment to add spaces. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs (updated) http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Ship it!

          • Ted

          On 2011-10-08 05:13:32, ramkrishna vasudevan wrote:

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

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

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

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

          (Updated 2011-10-08 05:13:32)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary

          -------

          Fix for handling HBASE-4539 and HBASE-4540.

          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.

          Also addresses Ted's comments.

          This addresses bug HBASE-4540.

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

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing

          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/#review2461 ----------------------------------------------------------- Ship it! Ted On 2011-10-08 05:13:32, ramkrishna vasudevan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-08 05:13:32) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Ship it!

          I'm good on commit.

          Have some suggestions for future handler tests below. I'm ok if we commit w/o addressing them here.

          Nice fix Ram

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
          <https://reviews.apache.org/r/2251/#comment5578>

          We don't have this method already in our ZK* classes?

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
          <https://reviews.apache.org/r/2251/#comment5579>

          Do you have to spin up the cluster twice? Could you do it once only in @BeforeClass and then shut it down in @AfterClass? So its run once only?

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
          <https://reviews.apache.org/r/2251/#comment5580>

          Good test.

          Would it be possible to test the handler without spinning up the cluster? See TestOpenRegionHandler over under regionserver.handler in tests – they don't spin up a cluster, just zk. Test can run faster if no dfs+hbase. Not important. For the future.

          • Michael

          On 2011-10-08 05:13:32, ramkrishna vasudevan wrote:

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

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

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

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

          (Updated 2011-10-08 05:13:32)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary

          -------

          Fix for handling HBASE-4539 and HBASE-4540.

          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.

          Also addresses Ted's comments.

          This addresses bug HBASE-4540.

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

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing

          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/#review2469 ----------------------------------------------------------- Ship it! I'm good on commit. Have some suggestions for future handler tests below. I'm ok if we commit w/o addressing them here. Nice fix Ram http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java < https://reviews.apache.org/r/2251/#comment5578 > We don't have this method already in our ZK* classes? http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java < https://reviews.apache.org/r/2251/#comment5579 > Do you have to spin up the cluster twice? Could you do it once only in @BeforeClass and then shut it down in @AfterClass? So its run once only? http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java < https://reviews.apache.org/r/2251/#comment5580 > Good test. Would it be possible to test the handler without spinning up the cluster? See TestOpenRegionHandler over under regionserver.handler in tests – they don't spin up a cluster, just zk. Test can run faster if no dfs+hbase. Not important. For the future. Michael On 2011-10-08 05:13:32, ramkrishna vasudevan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-08 05:13:32) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2011-10-08 21:55:31, Michael Stack wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java, line 873

          > <https://reviews.apache.org/r/2251/diff/4/?file=48949#file48949line873>

          >

          > We don't have this method already in our ZK* classes?

          @Stack
          ZKAssign() did have getDataAndWatch() that accepts stat object. Only ZKUtil had but it returned data in bytes which had to be again converted to RegionTransitionData.
          Hence added an utility api in ZKAssign itself and thought it may be useful in future also.

          On 2011-10-08 21:55:31, Michael Stack wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java, line 102

          > <https://reviews.apache.org/r/2251/diff/4/?file=48951#file48951line102>

          >

          > Good test.

          >

          > Would it be possible to test the handler without spinning up the cluster? See TestOpenRegionHandler over under regionserver.handler in tests – they don't spin up a cluster, just zk. Test can run faster if no dfs+hbase. Not important. For the future.

          @Stack
          I can do like that atleast for one of the testcases in TestOpenedRegionHandler. But i have to use the MockServer and MockRegionServices.
          I will raise one minor improvement task to do that. Currently MockServer and MockRegionServices are under regionserver.handler package but the new testcase is in master package. So better we can move it to a test.utility package and then use it across. So i will currently go with this commit and then track the new improvement JIRA to closure.

          • ramkrishna

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

          On 2011-10-08 05:13:32, ramkrishna vasudevan wrote:

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

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

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

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

          (Updated 2011-10-08 05:13:32)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary

          -------

          Fix for handling HBASE-4539 and HBASE-4540.

          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.

          Also addresses Ted's comments.

          This addresses bug HBASE-4540.

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

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing

          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - On 2011-10-08 21:55:31, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java , line 873 > < https://reviews.apache.org/r/2251/diff/4/?file=48949#file48949line873 > > > We don't have this method already in our ZK* classes? @Stack ZKAssign() did have getDataAndWatch() that accepts stat object. Only ZKUtil had but it returned data in bytes which had to be again converted to RegionTransitionData. Hence added an utility api in ZKAssign itself and thought it may be useful in future also. On 2011-10-08 21:55:31, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java , line 102 > < https://reviews.apache.org/r/2251/diff/4/?file=48951#file48951line102 > > > Good test. > > Would it be possible to test the handler without spinning up the cluster? See TestOpenRegionHandler over under regionserver.handler in tests – they don't spin up a cluster, just zk. Test can run faster if no dfs+hbase. Not important. For the future. @Stack I can do like that atleast for one of the testcases in TestOpenedRegionHandler. But i have to use the MockServer and MockRegionServices. I will raise one minor improvement task to do that. Currently MockServer and MockRegionServices are under regionserver.handler package but the new testcase is in master package. So better we can move it to a test.utility package and then use it across. So i will currently go with this commit and then track the new improvement JIRA to closure. ramkrishna ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/#review2469 ----------------------------------------------------------- On 2011-10-08 05:13:32, ramkrishna vasudevan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-08 05:13:32) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          ramkrishna.s.vasudevan added a comment -

          Integrated to 0.92 and trunk.
          Thanks for the reviews Ted, Gray, J-D and Stack.
          Created HBASE-4558 to refactor Stack's comment on using testcases without starting cluster.

          Show
          ramkrishna.s.vasudevan added a comment - Integrated to 0.92 and trunk. Thanks for the reviews Ted, Gray, J-D and Stack. Created HBASE-4558 to refactor Stack's comment on using testcases without starting cluster.
          Hide
          Hudson added a comment -

          Integrated in HBase-0.92 #54 (See https://builds.apache.org/job/HBase-0.92/54/)
          HBASE-4540 OpenedRegionHandler is not enforcing atomicity of the operation it is performing. Also addresses HBASE-4539. (Ram)

          ramkrishna :
          Files :

          • /hbase/branches/0.92/CHANGES.txt
          • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
          • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java
          • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
          • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
          • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
          Show
          Hudson added a comment - Integrated in HBase-0.92 #54 (See https://builds.apache.org/job/HBase-0.92/54/ ) HBASE-4540 OpenedRegionHandler is not enforcing atomicity of the operation it is performing. Also addresses HBASE-4539 . (Ram) ramkrishna : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #2311 (See https://builds.apache.org/job/HBase-TRUNK/2311/)
          HBASE-4540 OpenedRegionHandler is not enforcing atomicity of the operation it is performing . Also fixes HBASE-4539 (ram)

          ramkrishna :
          Files :

          • /hbase/trunk/CHANGES.txt
          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java
          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
          • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #2311 (See https://builds.apache.org/job/HBase-TRUNK/2311/ ) HBASE-4540 OpenedRegionHandler is not enforcing atomicity of the operation it is performing . Also fixes HBASE-4539 (ram) ramkrishna : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2011-10-08 21:55:31, Michael Stack wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java, line 102

          > <https://reviews.apache.org/r/2251/diff/4/?file=48951#file48951line102>

          >

          > Good test.

          >

          > Would it be possible to test the handler without spinning up the cluster? See TestOpenRegionHandler over under regionserver.handler in tests – they don't spin up a cluster, just zk. Test can run faster if no dfs+hbase. Not important. For the future.

          ramkrishna vasudevan wrote:

          @Stack

          I can do like that atleast for one of the testcases in TestOpenedRegionHandler. But i have to use the MockServer and MockRegionServices.

          I will raise one minor improvement task to do that. Currently MockServer and MockRegionServices are under regionserver.handler package but the new testcase is in master package. So better we can move it to a test.utility package and then use it across. So i will currently go with this commit and then track the new improvement JIRA to closure.

          Sounds good Ram. Yes, we should move these out if more generally useful.

          On 2011-10-08 21:55:31, Michael Stack wrote:

          > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java, line 873

          > <https://reviews.apache.org/r/2251/diff/4/?file=48949#file48949line873>

          >

          > We don't have this method already in our ZK* classes?

          ramkrishna vasudevan wrote:

          @Stack

          ZKAssign() did have getDataAndWatch() that accepts stat object. Only ZKUtil had but it returned data in bytes which had to be again converted to RegionTransitionData.

          Hence added an utility api in ZKAssign itself and thought it may be useful in future also.

          Sounds good.

          • Michael

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

          On 2011-10-08 05:13:32, ramkrishna vasudevan wrote:

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

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

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

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

          (Updated 2011-10-08 05:13:32)

          Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray.

          Summary

          -------

          Fix for handling HBASE-4539 and HBASE-4540.

          Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios.

          Also addresses Ted's comments.

          This addresses bug HBASE-4540.

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

          Diffs

          -----

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945

          http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION

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

          Testing

          -------

          Yes

          Thanks,

          ramkrishna

          Show
          jiraposter@reviews.apache.org added a comment - On 2011-10-08 21:55:31, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java , line 102 > < https://reviews.apache.org/r/2251/diff/4/?file=48951#file48951line102 > > > Good test. > > Would it be possible to test the handler without spinning up the cluster? See TestOpenRegionHandler over under regionserver.handler in tests – they don't spin up a cluster, just zk. Test can run faster if no dfs+hbase. Not important. For the future. ramkrishna vasudevan wrote: @Stack I can do like that atleast for one of the testcases in TestOpenedRegionHandler. But i have to use the MockServer and MockRegionServices. I will raise one minor improvement task to do that. Currently MockServer and MockRegionServices are under regionserver.handler package but the new testcase is in master package. So better we can move it to a test.utility package and then use it across. So i will currently go with this commit and then track the new improvement JIRA to closure. Sounds good Ram. Yes, we should move these out if more generally useful. On 2011-10-08 21:55:31, Michael Stack wrote: > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java , line 873 > < https://reviews.apache.org/r/2251/diff/4/?file=48949#file48949line873 > > > We don't have this method already in our ZK* classes? ramkrishna vasudevan wrote: @Stack ZKAssign() did have getDataAndWatch() that accepts stat object. Only ZKUtil had but it returned data in bytes which had to be again converted to RegionTransitionData. Hence added an utility api in ZKAssign itself and thought it may be useful in future also. Sounds good. Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/#review2469 ----------------------------------------------------------- On 2011-10-08 05:13:32, ramkrishna vasudevan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2251/ ----------------------------------------------------------- (Updated 2011-10-08 05:13:32) Review request for hbase, Ted Yu, Michael Stack, and Jonathan Gray. Summary ------- Fix for handling HBASE-4539 and HBASE-4540 . Ran all the testcases. Added one new testcase to verify OpenedRegionHandler scenarios. Also addresses Ted's comments. This addresses bug HBASE-4540 . https://issues.apache.org/jira/browse/HBASE-4540 Diffs ----- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/OpenedRegionHandler.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 1179945 http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java PRE-CREATION Diff: https://reviews.apache.org/r/2251/diff Testing ------- Yes Thanks, ramkrishna
          Hide
          ramkrishna.s.vasudevan added a comment -

          Reopening the issue to backport to 0.90 so that the fix can be available in 0.90 branch also.

          Show
          ramkrishna.s.vasudevan added a comment - Reopening the issue to backport to 0.90 so that the fix can be available in 0.90 branch also.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Contains patch for 0.90.x version. Moving MockServer and MockRegionServices to test.util package has also been done. Contains similar changes as in trunk version.

          Show
          ramkrishna.s.vasudevan added a comment - Contains patch for 0.90.x version. Moving MockServer and MockRegionServices to test.util package has also been done. Contains similar changes as in trunk version.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Testcases results will let you know tomorrow.

          Show
          ramkrishna.s.vasudevan added a comment - Testcases results will let you know tomorrow.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Testcases are passing.
          TestScanner
          TestCatalogTrackerOnCluster had failures which were unrelated to this.

          Show
          ramkrishna.s.vasudevan added a comment - Testcases are passing. TestScanner TestCatalogTrackerOnCluster had failures which were unrelated to this.
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Ted,
          I verified by running the testcases using my IDE, they passed. So i feel it is ok.

          Show
          ramkrishna.s.vasudevan added a comment - @Ted, I verified by running the testcases using my IDE, they passed. So i feel it is ok.
          Hide
          stack added a comment -

          +1 on commit to 0.90 branch.

          Show
          stack added a comment - +1 on commit to 0.90 branch.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Added a try finally block in TestOpenedRegionHandler.java

          Show
          ramkrishna.s.vasudevan added a comment - Added a try finally block in TestOpenedRegionHandler.java
          Hide
          stack added a comment -

          Can we resolve this issue now Ram?

          Show
          stack added a comment - Can we resolve this issue now Ram?
          Hide
          ramkrishna.s.vasudevan added a comment -

          Resolved both in 0.92 and 0.90.5.

          Show
          ramkrishna.s.vasudevan added a comment - Resolved both in 0.92 and 0.90.5.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development