HBase
  1. HBase
  2. HBASE-6012

Handling RegionOpeningState for bulk assign

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.95.2
    • Fix Version/s: 0.95.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Tags:
      0.96notable

      Description

      Since HBASE-5914, we using bulk assign for SSH

      But in the bulk assign case if we get an ALREADY_OPENED case there is no one to clear the znode created by bulk assign.

      Another thing, when RS opening a list of regions, if one region is already in transition, it will throw RegionAlreadyInTransitionException and stop opening other regions.

      1. HBASE-6012.patch
        4 kB
        chunhui shen
      2. HBASE-6012v2.patch
        2 kB
        chunhui shen
      3. HBASE-6012v3.patch
        10 kB
        chunhui shen
      4. HBASE-6012v4.patch
        15 kB
        chunhui shen
      5. HBASE-6012v5.patch
        15 kB
        chunhui shen
      6. HBASE-6012v6.patch
        15 kB
        chunhui shen
      7. HBASE-6012v7.patch
        16 kB
        chunhui shen
      8. HBASE-6012v8.patch
        16 kB
        chunhui shen

        Issue Links

          Activity

          Hide
          ramkrishna.s.vasudevan added a comment -

          @Chunhi
          You mean this change is applicable to trunk alone where HBASE-5914 has gone in?

          Show
          ramkrishna.s.vasudevan added a comment - @Chunhi You mean this change is applicable to trunk alone where HBASE-5914 has gone in?
          Hide
          chunhui shen added a comment -

          @ram
          Yes, we found this issue after HBASE-5914.
          Without HBASE-5914, I think it wouldn't happen when bulk assigning

          Show
          chunhui shen added a comment - @ram Yes, we found this issue after HBASE-5914 . Without HBASE-5914 , I think it wouldn't happen when bulk assigning
          Hide
          ramkrishna.s.vasudevan added a comment - - edited

          @Chunhuui
          We observed similar problems when we tried to work on HBASE-5917. Hence in HBASE-5917 we have tried to handle one such issue. We are trying to see if any such problems can happen.
          Because in normal assign flow we have many conditions added for various scenarios but the same doesn't apply in bulk assign. Will keep you updated on that.

          Mentioned the issue id wrongly. it is HBASE-5927

          {edit}
          We observed similar problems when we tried to work on HBASE-5927. Hence in HBASE-5927 we have tried to handle one such issue. We are trying to see if any such problems can happen.
          Because in normal assign flow we have many conditions added for various scenarios but the same doesn't apply in bulk assign. Will keep you updated on that.{edit}
          Show
          ramkrishna.s.vasudevan added a comment - - edited @Chunhuui We observed similar problems when we tried to work on HBASE-5917 . Hence in HBASE-5917 we have tried to handle one such issue. We are trying to see if any such problems can happen. Because in normal assign flow we have many conditions added for various scenarios but the same doesn't apply in bulk assign. Will keep you updated on that. Mentioned the issue id wrongly. it is HBASE-5927 {edit} We observed similar problems when we tried to work on HBASE-5927 . Hence in HBASE-5927 we have tried to handle one such issue. We are trying to see if any such problems can happen. Because in normal assign flow we have many conditions added for various scenarios but the same doesn't apply in bulk assign. Will keep you updated on that.{edit}
          Hide
          chunhui shen added a comment -

          @ram
          Do you mean about

          if (setOfflineInZK) {
                  // get the version of the znode after setting it to OFFLINE.
                  // versionOfOfflineNode will be -1 if the znode was not set to OFFLINE
                  versionOfOfflineNode = setOfflineInZooKeeper(state, hijack);
                  if (versionOfOfflineNode != -1) {
                    if (isDisabledorDisablingRegionInRIT(region)) {
                      return;
                    }
                    setEnabledTable(region);
                  }
                }
                if (setOfflineInZK && versionOfOfflineNode == -1) {
                  return;
                }

          When we bulk assign region, should we check each region? Could we ensure it in the upper layer who called the bulk assign.

          Show
          chunhui shen added a comment - @ram Do you mean about if (setOfflineInZK) { // get the version of the znode after setting it to OFFLINE. // versionOfOfflineNode will be -1 if the znode was not set to OFFLINE versionOfOfflineNode = setOfflineInZooKeeper(state, hijack); if (versionOfOfflineNode != -1) { if (isDisabledorDisablingRegionInRIT(region)) { return ; } setEnabledTable(region); } } if (setOfflineInZK && versionOfOfflineNode == -1) { return ; } When we bulk assign region, should we check each region? Could we ensure it in the upper layer who called the bulk assign.
          Hide
          ramkrishna.s.vasudevan added a comment - - edited

          @Chunhui
          Yes and not only that one

              if (!hijack && isDisabledorDisablingRegionInRIT(region)) {
                return;
              }
          

          So once assign is called immediately the disable and disabling will be taken care of.

          Show
          ramkrishna.s.vasudevan added a comment - - edited @Chunhui Yes and not only that one if (!hijack && isDisabledorDisablingRegionInRIT(region)) { return ; } So once assign is called immediately the disable and disabling will be taken care of.
          Hide
          chunhui shen added a comment -

          @ram
          I think we should take care of the disable and disabling table before called bulk assign

          Show
          chunhui shen added a comment - @ram I think we should take care of the disable and disabling table before called bulk assign
          Hide
          ramkrishna.s.vasudevan added a comment -

          I think this change we need to get into trunk after reviews for the SSH part to work fine introduced in HBASE-5914.

          Show
          ramkrishna.s.vasudevan added a comment - I think this change we need to get into trunk after reviews for the SSH part to work fine introduced in HBASE-5914 .
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Chunhui
          Please check the issue HBASE-6147. Along with the scenario mentioned there if we take the current patch here I feel the following problem can come
          -> Because as per the current patch attached here ,we force the znode to OFFLINE state, if there was any current assignment going on due to SSH in one of the RS that will get stopped because of znode version mismatch.
          Internally that failure to OPEN regin will make the znode to FAILED_OPEN. Now based on this master will again start a new assignment. So this along with the issue in HBASE-6147 may lead to double assignment. So what we felt here is this patch should go along with some changes in HBASE-6147.
          Pls feel free to correct me if am wrong.

          Show
          ramkrishna.s.vasudevan added a comment - @Chunhui Please check the issue HBASE-6147 . Along with the scenario mentioned there if we take the current patch here I feel the following problem can come -> Because as per the current patch attached here ,we force the znode to OFFLINE state, if there was any current assignment going on due to SSH in one of the RS that will get stopped because of znode version mismatch. Internally that failure to OPEN regin will make the znode to FAILED_OPEN. Now based on this master will again start a new assignment. So this along with the issue in HBASE-6147 may lead to double assignment. So what we felt here is this patch should go along with some changes in HBASE-6147 . Pls feel free to correct me if am wrong.
          Hide
          chunhui shen added a comment -

          @ram

          one of the RS that will get stopped because of znode version mismatch

          Yes, I has found this error case.

          In my opinion, when doing asyncSetOfflineInZooKeeper, if node exists, I delete it first.

          Done in patch v2.

          Show
          chunhui shen added a comment - @ram one of the RS that will get stopped because of znode version mismatch Yes, I has found this error case. In my opinion, when doing asyncSetOfflineInZooKeeper, if node exists, I delete it first. Done in patch v2.
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Chunhui
          But deleting the node, what improvment we get here? Still the scenario that i was mentioning may happen right?
          Correct me if am wrong.

          Show
          ramkrishna.s.vasudevan added a comment - @Chunhui But deleting the node, what improvment we get here? Still the scenario that i was mentioning may happen right? Correct me if am wrong.
          Hide
          chunhui shen added a comment -

          @ram
          If deleting the node first, znode version mismatch won't happen and RS could open region successfully.
          Existing nodes is small probability case, so we can ignore it when measure improvment.

          Anyway, about the double assignment, I think we should fix it through HBASE-6147.

          This issue tries to fix the case node exists when doing AssignmentManager#asyncSetOfflineInZooKeeper, is it right?

          Show
          chunhui shen added a comment - @ram If deleting the node first, znode version mismatch won't happen and RS could open region successfully. Existing nodes is small probability case, so we can ignore it when measure improvment. Anyway, about the double assignment, I think we should fix it through HBASE-6147 . This issue tries to fix the case node exists when doing AssignmentManager#asyncSetOfflineInZooKeeper, is it right?
          Hide
          stack added a comment -

          Chunhui This method is only called bulk assigning? Why would there be a znode at all if we are bulk assigning? Should we do cleanup of znode state before we bulk assign? The delete of the znode ahead of forcing it offline makes me nervous. If this issue only started showing up because we added bulk assigning to SSH (hbase-5914), then maybe in SSH before we do the bulk assign, we should be doing the clean of zk and not do this delete and then offline? What you think?

          Show
          stack added a comment - Chunhui This method is only called bulk assigning? Why would there be a znode at all if we are bulk assigning? Should we do cleanup of znode state before we bulk assign? The delete of the znode ahead of forcing it offline makes me nervous. If this issue only started showing up because we added bulk assigning to SSH (hbase-5914), then maybe in SSH before we do the bulk assign, we should be doing the clean of zk and not do this delete and then offline? What you think?
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Stack
          Yes I agree with you.
          @Chunhui
          The problem mainly i am worried is in the bulk assign case if we get an ALREADY_OPENED case there is no one to clear the znode created by bulk assign. So if we try to do like delete and force it to offline still the nodes will keep exisiting in zk.
          This change is actually very much needed and important now, but just need to make sure we don't create open areas. Good on you Chunhui.

          Show
          ramkrishna.s.vasudevan added a comment - @Stack Yes I agree with you. @Chunhui The problem mainly i am worried is in the bulk assign case if we get an ALREADY_OPENED case there is no one to clear the znode created by bulk assign. So if we try to do like delete and force it to offline still the nodes will keep exisiting in zk. This change is actually very much needed and important now, but just need to make sure we don't create open areas. Good on you Chunhui.
          Hide
          chunhui shen added a comment -

          @ram

          The problem mainly i am worried is in the bulk assign case if we get an ALREADY_OPENED case

          Yes, in the current code , we don't handle the reponses from sendRegionOpen RPC in the bulk assign.
          So we must also handle reponses for bulking assign.

          Another problem is that RS will throw RegionAlreadyInTransitionException in bulking assign, it means one exception would stop all the regions's opening.

          Show
          chunhui shen added a comment - @ram The problem mainly i am worried is in the bulk assign case if we get an ALREADY_OPENED case Yes, in the current code , we don't handle the reponses from sendRegionOpen RPC in the bulk assign. So we must also handle reponses for bulking assign. Another problem is that RS will throw RegionAlreadyInTransitionException in bulking assign, it means one exception would stop all the regions's opening.
          Hide
          chunhui shen added a comment -

          Updating patch,
          fetch Stack's and ram's suggestion in

          Show
          chunhui shen added a comment - Updating patch, fetch Stack's and ram's suggestion in
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Chunhui
          Great stuff.

          'alreadyOpendRegion' should be 'processAlreadyOpenedRegion'.
          In the sendRegionOpen() in the patch

              if (admin == null) {
                LOG.warn("Attempting to send OPEN RPC to server " + server.toString() +
                  " failed because no RPC connection found to this server");
                return null;
          

          we may need to handle FAILED_OPEN also. May be it was not present but i think handling it here is necessary or else the znode will be created and no one will delete the znode. And subsequent bulk assign may cause the same issue reported here.
          Others are great. Lets see others reviews also. Thanks Chunhui.

          Show
          ramkrishna.s.vasudevan added a comment - @Chunhui Great stuff. 'alreadyOpendRegion' should be 'processAlreadyOpenedRegion'. In the sendRegionOpen() in the patch if (admin == null ) { LOG.warn( "Attempting to send OPEN RPC to server " + server.toString() + " failed because no RPC connection found to this server" ); return null ; we may need to handle FAILED_OPEN also. May be it was not present but i think handling it here is necessary or else the znode will be created and no one will delete the znode. And subsequent bulk assign may cause the same issue reported here. Others are great. Lets see others reviews also. Thanks Chunhui.
          Hide
          ramkrishna.s.vasudevan added a comment -
                      }catch (KeeperException ke) {
                              throw new IOException(ke);
                            }
          

          This may not be correct I feel. Throwing exception from here will just kill the SSH thread. And may not carry on with assignment. What you feel? Can we abort master?

          Show
          ramkrishna.s.vasudevan added a comment - } catch (KeeperException ke) { throw new IOException(ke); } This may not be correct I feel. Throwing exception from here will just kill the SSH thread. And may not carry on with assignment. What you feel? Can we abort master?
          Hide
          Ted Yu added a comment -
          +          List<RegionOpeningState> regionOpeningStateList = this.serverManager
          +              .sendRegionOpen(destination, regions);
          +          for (int i = 0; i < regionOpeningStateList.size(); i++) {
          

          Should we check whether the return from sendRegionOpen() is null ?

          In ServerShutdownHandler.java:

          +                if(rit != null){
          +                  //clean zk node
          +                  try{
          +                    ZKAssign.deleteNodeFailSilent(services.getZooKeeper(), e.getKey());
          

          Log statement should be added that reveals the value of rit.

          Show
          Ted Yu added a comment - + List<RegionOpeningState> regionOpeningStateList = this .serverManager + .sendRegionOpen(destination, regions); + for ( int i = 0; i < regionOpeningStateList.size(); i++) { Should we check whether the return from sendRegionOpen() is null ? In ServerShutdownHandler.java: + if (rit != null ){ + //clean zk node + try { + ZKAssign.deleteNodeFailSilent(services.getZooKeeper(), e.getKey()); Log statement should be added that reveals the value of rit.
          Hide
          chunhui shen added a comment -

          Updating patch v4,
          fetching Ted's and Ram's comments in
          handling RegionOpeningState.FAILED_OPENING

          Show
          chunhui shen added a comment - Updating patch v4, fetching Ted's and Ram's comments in handling RegionOpeningState.FAILED_OPENING
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Chunhui
          Minor comments
          It should be 'processAlreadyOpenedRegion', i think you forgot to add 'e' in 'opened'. Currently it is processAlreadyOpendRegion
          'Reassigning region with rs=' should be 'Reassigning region with rs =' (a space is needed).

          Any specific reason for making this change now?

          for (HRegionInfo region : reassigningRegions) {
                  assign(region, true, true);
                }
          
          // Failed opening this region, reassign it
                        assign(regions.get(i), true, true);
          

          before doing this can we call getRegionPlan() passing the current server on which failure happened so that i can be excluded.(May not be mandatory).

          Overall looks good. Thanks Chunhui.

          Show
          ramkrishna.s.vasudevan added a comment - @Chunhui Minor comments It should be 'processAlreadyOpenedRegion', i think you forgot to add 'e' in 'opened'. Currently it is processAlreadyOpendRegion 'Reassigning region with rs=' should be 'Reassigning region with rs =' (a space is needed). Any specific reason for making this change now? for (HRegionInfo region : reassigningRegions) { assign(region, true , true ); } // Failed opening this region, reassign it assign(regions.get(i), true , true ); before doing this can we call getRegionPlan() passing the current server on which failure happened so that i can be excluded.(May not be mandatory). Overall looks good. Thanks Chunhui.
          Hide
          chunhui shen added a comment -

          @ram
          Thanks to reviewe,

          Any specific reason for making this change now?

          For the failed regions through bulk assigning, we already created zk node, it will fail again if using bulk assign.

          Show
          chunhui shen added a comment - @ram Thanks to reviewe, Any specific reason for making this change now? For the failed regions through bulk assigning, we already created zk node, it will fail again if using bulk assign.
          Hide
          chunhui shen added a comment -

          Getting ram's comment about name and log,
          updating patch v5.

          @ram
          Could you make it run by hadoopQA

          Show
          chunhui shen added a comment - Getting ram's comment about name and log, updating patch v5. @ram Could you make it run by hadoopQA
          Hide
          ramkrishna.s.vasudevan added a comment -

          For the failed regions through bulk assigning, we already created zk node, it will fail again if using bulk assign.

          Ok. Got it.

          Show
          ramkrishna.s.vasudevan added a comment - For the failed regions through bulk assigning, we already created zk node, it will fail again if using bulk assign. Ok. Got it.
          Hide
          Ted Yu added a comment -
                 LOG.info("Unable to communicate with the region server in order" +
                     " to assign regions", e);
          -      return false;
          +      // Server may already get RPC
          +      return true;
          

          What was the reasoning behind the above change ?

          -      try {
          -        if (!assign(e.getKey(), e.getValue())) {
          -          failedPlans.put(e.getKey(), e.getValue());
          -        }
          -      } catch (Throwable t) {
          +      if (!assign(e.getKey(), e.getValue())) {
          

          I think the catch clause should be kept.

          For HRegionServer.java, there're a lot of formatting changes which distract reviewing.

          +      } catch (RegionAlreadyInTransitionException rie) {
          +        LOG.warn("", rie);
          

          Please add some sentence for the log above.

          Show
          Ted Yu added a comment - LOG.info( "Unable to communicate with the region server in order" + " to assign regions" , e); - return false ; + // Server may already get RPC + return true ; What was the reasoning behind the above change ? - try { - if (!assign(e.getKey(), e.getValue())) { - failedPlans.put(e.getKey(), e.getValue()); - } - } catch (Throwable t) { + if (!assign(e.getKey(), e.getValue())) { I think the catch clause should be kept. For HRegionServer.java, there're a lot of formatting changes which distract reviewing. + } catch (RegionAlreadyInTransitionException rie) { + LOG.warn("", rie); Please add some sentence for the log above.
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Chunhui
          As Ted suggested pls correct the formatting changes like for eg

          +                if(rit != null){
          +                  //clean zk node
          +                  try{
          
          Show
          ramkrishna.s.vasudevan added a comment - @Chunhui As Ted suggested pls correct the formatting changes like for eg + if (rit != null ){ + //clean zk node + try {
          Hide
          chunhui shen added a comment -
          // Can be a socket timeout, EOF, NoRouteToHost, etc
          LOG.info("Unable to communicate with the region server in order" +
                     " to assign regions", e);
          -      return false;
          +      // Server may already get RPC
          +      return true;
          

          What was the reasoning behind the above change ?

          At first, I think we can't ensure regionserver not receive the OPENREGION rpc, in order to prevent double assign, so I make this change. Also here could return false because of protecting through single assign.

          throw new RegionAlreadyInTransitionException("Received:" + currentAction +
                  " for the region:" + region.getRegionNameAsString() +
                  " ,which we are already trying to " +
                  (openAction ? OPEN : CLOSE)+ ".");
          

          Please add some sentence for the log above.

          We have added message about it in RegionAlreadyInTransitionException, so I think we needn't add again.

          For other comments, I update it in patch v6.

          Thanks.

          Show
          chunhui shen added a comment - // Can be a socket timeout, EOF, NoRouteToHost, etc LOG.info( "Unable to communicate with the region server in order" + " to assign regions" , e); - return false ; + // Server may already get RPC + return true ; What was the reasoning behind the above change ? At first, I think we can't ensure regionserver not receive the OPENREGION rpc, in order to prevent double assign, so I make this change. Also here could return false because of protecting through single assign. throw new RegionAlreadyInTransitionException( "Received:" + currentAction + " for the region:" + region.getRegionNameAsString() + " ,which we are already trying to " + (openAction ? OPEN : CLOSE)+ "." ); Please add some sentence for the log above. We have added message about it in RegionAlreadyInTransitionException, so I think we needn't add again. For other comments, I update it in patch v6. Thanks.
          Hide
          chunhui shen added a comment -

          Created a Review Request with the patch v6.

          https://review.cloudera.org/r/2132/

          Could you review again? Thanks.

          Show
          chunhui shen added a comment - Created a Review Request with the patch v6. https://review.cloudera.org/r/2132/ Could you review again? Thanks.
          Hide
          chunhui shen added a comment -

          e...I find I can't upload diff for Review Request...

          Is it a server's error?

          Show
          chunhui shen added a comment - e...I find I can't upload diff for Review Request... Is it a server's error?
          Hide
          chunhui shen added a comment -

          The above Review Request is discarded

          New:
          https://review.cloudera.org/r/2136/

          Show
          chunhui shen added a comment - The above Review Request is discarded New: https://review.cloudera.org/r/2136/
          Hide
          Ted Yu added a comment -

          @Chunhui:
          Hadoop QA is not functioning. Can you run the whole test suite and post the result ?

          Show
          Ted Yu added a comment - @Chunhui: Hadoop QA is not functioning. Can you run the whole test suite and post the result ?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12531650/HBASE-6012v6.patch
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

          -1 findbugs. The patch appears to introduce 5 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.TestAssignmentManager
          org.apache.hadoop.hbase.master.TestZKBasedOpenCloseRegion

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2137//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2137//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2137//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2137//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/12531650/HBASE-6012v6.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 5 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.TestAssignmentManager org.apache.hadoop.hbase.master.TestZKBasedOpenCloseRegion Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2137//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2137//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2137//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2137//console This message is automatically generated.
          Hide
          Ted Yu added a comment -

          There was NPE in TestAssignmentManager#testSSHWhenSplitRegionInProgress
          Please fix.

          Thanks

          Show
          Ted Yu added a comment - There was NPE in TestAssignmentManager#testSSHWhenSplitRegionInProgress Please fix. Thanks
          Hide
          chunhui shen added a comment -

          Updating patchv7, fix the two failed test case

          Show
          chunhui shen added a comment - Updating patchv7, fix the two failed test case
          Hide
          chunhui shen added a comment -

          Updating patch v8, fetch ted's review board comments in

          Show
          chunhui shen added a comment - Updating patch v8, fetch ted's review board comments in
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12531768/HBASE-6012v8.patch
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

          -1 findbugs. The patch appears to introduce 5 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.TestAtomicOperation
          org.apache.hadoop.hbase.replication.TestReplication

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2141//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2141//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2141//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2141//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/12531768/HBASE-6012v8.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 5 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.TestAtomicOperation org.apache.hadoop.hbase.replication.TestReplication Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/2141//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2141//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2141//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2141//console This message is automatically generated.
          Hide
          Ted Yu added a comment -

          I ran the two failed tests manually and they passed.

          Will integrate tomorrow if there is no objection.

          Show
          Ted Yu added a comment - I ran the two failed tests manually and they passed. Will integrate tomorrow if there is no objection.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12531768/HBASE-6012v8.patch
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

          -1 findbugs. The patch appears to introduce 5 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/2143//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2143//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2143//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2143//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/12531768/HBASE-6012v8.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 5 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/2143//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2143//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/2143//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2143//console This message is automatically generated.
          Hide
          Ted Yu added a comment -

          Integrated to trunk.

          Thanks for the patch, Chunhui.

          Thanks for the review, Stack and Ram.

          Show
          Ted Yu added a comment - Integrated to trunk. Thanks for the patch, Chunhui. Thanks for the review, Stack and Ram.
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #3014 (See https://builds.apache.org/job/HBase-TRUNK/3014/)
          HBASE-6012 Handling RegionOpeningState for bulk assign (Chunhui) (Revision 1349377)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #3014 (See https://builds.apache.org/job/HBase-TRUNK/3014/ ) HBASE-6012 Handling RegionOpeningState for bulk assign (Chunhui) (Revision 1349377) Result = FAILURE tedyu : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #51 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/51/)
          HBASE-6012 Handling RegionOpeningState for bulk assign (Chunhui) (Revision 1349377)

          Result = FAILURE
          tedyu :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #51 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/51/ ) HBASE-6012 Handling RegionOpeningState for bulk assign (Chunhui) (Revision 1349377) Result = FAILURE tedyu : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
          Hide
          stack added a comment -

          Marking closed.

          Show
          stack added a comment - Marking closed.

            People

            • Assignee:
              chunhui shen
              Reporter:
              chunhui shen
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development