Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-4386

refreshNodesGracefully() should send recommission event to active RMNodes only

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha1
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: graceful
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      In refreshNodesGracefully(), during recommissioning, the entryset from getRMNodes() which has only active nodes (RUNNING, DECOMMISSIONING etc.) is used for checking 'decommissioned' nodes which are present in getInactiveRMNodes() map alone.

      for (Entry<NodeId, RMNode> entry:rmContext.getRMNodes().entrySet()) { .........................
       // Recommissioning the nodes
              if (entry.getValue().getState() == NodeState.DECOMMISSIONING
                  || entry.getValue().getState() == NodeState.DECOMMISSIONED) {
                this.rmContext.getDispatcher().getEventHandler()
                    .handle(new RMNodeEvent(nodeId, RMNodeEventType.RECOMMISSION));
              }
      
      1. YARN-4386-v1.patch
        1 kB
        Kuhu Shukla
      2. YARN-4386-v2.patch
        4 kB
        Kuhu Shukla

        Activity

        Hide
        kshukla Kuhu Shukla added a comment -

        Thank you Junping for the review and commit!

        Show
        kshukla Kuhu Shukla added a comment - Thank you Junping for the review and commit!
        Hide
        djp Junping Du added a comment -

        Mark issue as resolved should be good enough for now. In general, we close the issue until we are sure we won't backport patch to other branches or we could lose track of backporting effort.

        Show
        djp Junping Du added a comment - Mark issue as resolved should be good enough for now. In general, we close the issue until we are sure we won't backport patch to other branches or we could lose track of backporting effort.
        Hide
        djp Junping Du added a comment -

        I have commit the patch to trunk, branch-2 and branch-2.8. Thanks Kuhu Shukla for patch contribution and Sunil G for review and comments!

        Show
        djp Junping Du added a comment - I have commit the patch to trunk, branch-2 and branch-2.8. Thanks Kuhu Shukla for patch contribution and Sunil G for review and comments!
        Hide
        kshukla Kuhu Shukla added a comment -

        Thanks a lot Junping. Can we close the JIRA as resolved?

        Show
        kshukla Kuhu Shukla added a comment - Thanks a lot Junping. Can we close the JIRA as resolved?
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #9340 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9340/)
        YARN-4386. refreshNodesGracefully() should send recommission event to (junping_du: rev 3fab88540f079591e5dae1c9184f8b26bb843427)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestResourceTrackerService.java
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java
        • hadoop-yarn-project/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9340 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9340/ ) YARN-4386 . refreshNodesGracefully() should send recommission event to (junping_du: rev 3fab88540f079591e5dae1c9184f8b26bb843427) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestResourceTrackerService.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java hadoop-yarn-project/CHANGES.txt
        Hide
        djp Junping Du added a comment -

        v2 patch LGTM. +1. Will commit it shortly.

        Show
        djp Junping Du added a comment - v2 patch LGTM. +1. Will commit it shortly.
        Hide
        kshukla Kuhu Shukla added a comment -

        Thank you Junping Du! Will wait for your comments.

        Show
        kshukla Kuhu Shukla added a comment - Thank you Junping Du ! Will wait for your comments.
        Hide
        djp Junping Du added a comment -

        Sorry that I was in travel since mid of this week. Will review on this soon.

        Show
        djp Junping Du added a comment - Sorry that I was in travel since mid of this week. Will review on this soon.
        Hide
        kshukla Kuhu Shukla added a comment -

        Junping Du, requesting for comments/review from you on this. Thanks a lot!

        Show
        kshukla Kuhu Shukla added a comment - Junping Du , requesting for comments/review from you on this. Thanks a lot!
        Hide
        sunilg Sunil G added a comment -

        Thanks Kuhu Shukla for the patch.
        I think patch generally cover the intended scenario by covering the changed code. Looks good. We can wait for Junping Du comments too.

        Show
        sunilg Sunil G added a comment - Thanks Kuhu Shukla for the patch. I think patch generally cover the intended scenario by covering the changed code. Looks good. We can wait for Junping Du comments too.
        Hide
        kshukla Kuhu Shukla added a comment -

        Updating patch with a test to check if a decommissioned node can ever transition to running state by graceful decommissioning process. The test TestRMNodeTransitions#testRecommissionNode covers the other case where a node can be recommissioned after being in decommissioning state. Since we know that only inactiveRMNodes will contain the decommissioned node, the check for such in a node in active list is not useful.

        Junping Du, Sunil G Request for comments/review. Thanks a lot!

        Show
        kshukla Kuhu Shukla added a comment - Updating patch with a test to check if a decommissioned node can ever transition to running state by graceful decommissioning process. The test TestRMNodeTransitions#testRecommissionNode covers the other case where a node can be recommissioned after being in decommissioning state. Since we know that only inactiveRMNodes will contain the decommissioned node, the check for such in a node in active list is not useful. Junping Du , Sunil G Request for comments/review. Thanks a lot!
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        0 mvndep 0m 43s Maven dependency ordering for branch
        +1 mvninstall 7m 12s trunk passed
        +1 compile 0m 25s trunk passed with JDK v1.8.0_66
        +1 compile 0m 28s trunk passed with JDK v1.7.0_91
        +1 checkstyle 0m 18s trunk passed
        +1 mvnsite 0m 34s trunk passed
        +1 mvneclipse 0m 15s trunk passed
        +1 findbugs 1m 6s trunk passed
        +1 javadoc 0m 20s trunk passed with JDK v1.8.0_66
        +1 javadoc 0m 25s trunk passed with JDK v1.7.0_91
        0 mvndep 0m 9s Maven dependency ordering for patch
        +1 mvninstall 0m 29s the patch passed
        +1 compile 0m 22s the patch passed with JDK v1.8.0_66
        +1 javac 0m 22s the patch passed
        +1 compile 0m 25s the patch passed with JDK v1.7.0_91
        +1 javac 0m 25s the patch passed
        +1 checkstyle 0m 16s the patch passed
        +1 mvnsite 0m 31s the patch passed
        +1 mvneclipse 0m 12s the patch passed
        +1 whitespace 0m 0s Patch has no whitespace issues.
        +1 findbugs 1m 13s the patch passed
        +1 javadoc 0m 18s the patch passed with JDK v1.8.0_66
        +1 javadoc 0m 22s the patch passed with JDK v1.7.0_91
        -1 unit 67m 50s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.8.0_66.
        -1 unit 67m 21s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_91.
        +1 asflicense 0m 25s Patch does not generate ASF License warnings.
        152m 44s



        Reason Tests
        JDK v1.8.0_66 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens
          hadoop.yarn.server.resourcemanager.TestAMAuthorization
        JDK v1.7.0_91 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens
          hadoop.yarn.server.resourcemanager.TestAMAuthorization



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:0ca8df7
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786096/YARN-4386-v2.patch
        JIRA Issue YARN-4386
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 7e54e1f6f974 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / fa328e2
        Default Java 1.7.0_91
        Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/10486/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_66.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/10486/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_91.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/10486/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/10486/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_91.txt
        JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10486/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Max memory used 76MB
        Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/10486/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 43s Maven dependency ordering for branch +1 mvninstall 7m 12s trunk passed +1 compile 0m 25s trunk passed with JDK v1.8.0_66 +1 compile 0m 28s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 34s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 1m 6s trunk passed +1 javadoc 0m 20s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 25s trunk passed with JDK v1.7.0_91 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 0m 29s the patch passed +1 compile 0m 22s the patch passed with JDK v1.8.0_66 +1 javac 0m 22s the patch passed +1 compile 0m 25s the patch passed with JDK v1.7.0_91 +1 javac 0m 25s the patch passed +1 checkstyle 0m 16s the patch passed +1 mvnsite 0m 31s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 13s the patch passed +1 javadoc 0m 18s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 22s the patch passed with JDK v1.7.0_91 -1 unit 67m 50s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.8.0_66. -1 unit 67m 21s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_91. +1 asflicense 0m 25s Patch does not generate ASF License warnings. 152m 44s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens   hadoop.yarn.server.resourcemanager.TestAMAuthorization JDK v1.7.0_91 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens   hadoop.yarn.server.resourcemanager.TestAMAuthorization Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786096/YARN-4386-v2.patch JIRA Issue YARN-4386 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7e54e1f6f974 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fa328e2 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/10486/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/10486/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_91.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/10486/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/10486/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/10486/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Max memory used 76MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-YARN-Build/10486/console This message was automatically generated.
        Hide
        kshukla Kuhu Shukla added a comment -

        Updating patch with a test to check if a decommissioned node can ever transition to running state by graceful decommissioning process. The test TestRMNodeTransitions#testRecommissionNode covers the other case where a node can be recommissioned after being in decommissioning state. Since we know that only inactiveRMNodes will contain the decommissioned node, the check for such in a node in active list is not useful.

        Show
        kshukla Kuhu Shukla added a comment - Updating patch with a test to check if a decommissioned node can ever transition to running state by graceful decommissioning process. The test TestRMNodeTransitions#testRecommissionNode covers the other case where a node can be recommissioned after being in decommissioning state. Since we know that only inactiveRMNodes will contain the decommissioned node, the check for such in a node in active list is not useful.
        Hide
        kshukla Kuhu Shukla added a comment -

        I agree about the transitions being safe. We can try this test just to be super sure although the code in deactivateNode which is called every time there is a transition to DECOMMISSIONED state the first time, shows that such nodes are removed from active list and put in Inactive list.

        rmNode.context.getRMNodes().remove(rmNode.nodeId);
            LOG.info("Deactivating Node " + rmNode.nodeId + " as it is now "
                + finalState);
            rmNode.context.getInactiveRMNodes().put(rmNode.nodeId, rmNode);
        

        Will give this test a shot nonetheless and get back.

        Show
        kshukla Kuhu Shukla added a comment - I agree about the transitions being safe. We can try this test just to be super sure although the code in deactivateNode which is called every time there is a transition to DECOMMISSIONED state the first time, shows that such nodes are removed from active list and put in Inactive list. rmNode.context.getRMNodes().remove(rmNode.nodeId); LOG.info( "Deactivating Node " + rmNode.nodeId + " as it is now " + finalState); rmNode.context.getInactiveRMNodes().put(rmNode.nodeId, rmNode); Will give this test a shot nonetheless and get back.
        Hide
        sunilg Sunil G added a comment -

        Hi Kuhu Shukla
        Sorry for replying late here.

        Unless there are 2 refreshNodes done in parallel such that the first deactivateNodeTransition has not finished and the other refreshNodes is also trying to do the same transition

        Since the transitions are happening under write lock, this may not happen.

        I have one suggestion here.
        I feel You could mark a node for GRACEFUL DECOMMISSION and ensure that node is in DECOMMISSIONING state. (can try to fire event to RMNodeImpl directly to do this). Later invoke refreshNodesGracefully and verify that an event named RECOMMISSION is raised to dispatcher or not. Similarly mark a node as DECOMMISSIONED and then invoke refreshNodesGracefully and verify the event RECOMMISSION is NOT raised. In second case, it will not enter for loop. but I feel this will clear cover our case here though its not direct.
        Pls correct me if I am wrong.

        Show
        sunilg Sunil G added a comment - Hi Kuhu Shukla Sorry for replying late here. Unless there are 2 refreshNodes done in parallel such that the first deactivateNodeTransition has not finished and the other refreshNodes is also trying to do the same transition Since the transitions are happening under write lock, this may not happen. I have one suggestion here. I feel You could mark a node for GRACEFUL DECOMMISSION and ensure that node is in DECOMMISSIONING state. (can try to fire event to RMNodeImpl directly to do this). Later invoke refreshNodesGracefully and verify that an event named RECOMMISSION is raised to dispatcher or not. Similarly mark a node as DECOMMISSIONED and then invoke refreshNodesGracefully and verify the event RECOMMISSION is NOT raised. In second case, it will not enter for loop. but I feel this will clear cover our case here though its not direct. Pls correct me if I am wrong.
        Hide
        kshukla Kuhu Shukla added a comment -

        Sunil G, Junping Du. Request for comments on how to test this since even during the transition, the RMNode is removed from active list first and then put in the inactive RMNode list. Unless there are 2 refreshNodes done in parallel such that the first deactivateNodeTransition has not finished and the other refreshNodes is also trying to do the same transition, only one of them would succeed and this would not be a race . Let me know if that makes sense.

        Show
        kshukla Kuhu Shukla added a comment - Sunil G , Junping Du . Request for comments on how to test this since even during the transition, the RMNode is removed from active list first and then put in the inactive RMNode list. Unless there are 2 refreshNodes done in parallel such that the first deactivateNodeTransition has not finished and the other refreshNodes is also trying to do the same transition, only one of them would succeed and this would not be a race . Let me know if that makes sense.
        Hide
        sunilg Sunil G added a comment -

        getRMNodes will have only Active/Decommissioning nodes. hence as you mentioned, its highly unlikely that a node will be getRMNodes list which is also DECOMMISSIONED.

        For test case, you can try add a node which is DECOMMISSIONED to active node list forcefully. But this seems again not a very valid case. Junping Du, will this happen only if a race condition exits in active->decommisioned window.

        Show
        sunilg Sunil G added a comment - getRMNodes will have only Active/Decommissioning nodes. hence as you mentioned, its highly unlikely that a node will be getRMNodes list which is also DECOMMISSIONED. For test case, you can try add a node which is DECOMMISSIONED to active node list forcefully. But this seems again not a very valid case. Junping Du , will this happen only if a race condition exits in active->decommisioned window.
        Hide
        kshukla Kuhu Shukla added a comment -

        Junping Du, without the patch, the decommissioned node is looked up in the list returned by getRMNodes() which will never have any node with nodestate=DECOMMISSIONED; this means that currently a decommissioned node is not even looked at for recommissioning since its part of inactiveNodes list and not the getRMNodes() list. I will continue to think of a test case for this. Appreciate your comments and inputs.

        Show
        kshukla Kuhu Shukla added a comment - Junping Du , without the patch, the decommissioned node is looked up in the list returned by getRMNodes() which will never have any node with nodestate=DECOMMISSIONED; this means that currently a decommissioned node is not even looked at for recommissioning since its part of inactiveNodes list and not the getRMNodes() list. I will continue to think of a test case for this. Appreciate your comments and inputs.
        Hide
        kshukla Kuhu Shukla added a comment -

        Sure that would help. I will update with revised patch soon. Thank you!

        Show
        kshukla Kuhu Shukla added a comment - Sure that would help. I will update with revised patch soon. Thank you!
        Hide
        djp Junping Du added a comment -

        Thanks Kuhu Shukla for the patch. I agree these test failures are not related. However, can we add a test to verify no InvalidState get throw after the patch if recommission a decommissioned node when calling refreshNodesGracefully()? That test should get failed without applying your code here.

        Show
        djp Junping Du added a comment - Thanks Kuhu Shukla for the patch. I agree these test failures are not related. However, can we add a test to verify no InvalidState get throw after the patch if recommission a decommissioned node when calling refreshNodesGracefully()? That test should get failed without applying your code here.
        Hide
        kshukla Kuhu Shukla added a comment -

        Thank you Junping Du for your comments.
        TestClientRMTokens is failing regardless of the patch, tracked through YARN-4306.
        TestAMAuthorization fails the same way and I believe is tracked through YARN-4318.
        No tests were attached since the final outcome of this patch remains unchanged and a decommissioned node stays in that state regardless.
        Sunil G, Junping Du, request for review. Thanks a lot.

        Show
        kshukla Kuhu Shukla added a comment - Thank you Junping Du for your comments. TestClientRMTokens is failing regardless of the patch, tracked through YARN-4306 . TestAMAuthorization fails the same way and I believe is tracked through YARN-4318 . No tests were attached since the final outcome of this patch remains unchanged and a decommissioned node stays in that state regardless. Sunil G , Junping Du , request for review. Thanks a lot.
        Hide
        djp Junping Du added a comment -

        Thanks Kuhu Shukla to report this issue and Sunil G for review!
        I think Recommission event shouldn't be applied on decommissioned nodes as it won't have any affect and we'd better to keep consistent with previous behavior before graceful decommission comes out.
        Thus, I would prefer to change "if (entry.getValue().getState() == NodeState.DECOMMISSIONING || entry.getValue().getState() == NodeState.DECOMMISSIONED)" to "if (entry.getValue().getState() == NodeState.DECOMMISSIONING)" to get rid of InvalidState exception in state machine.

        Show
        djp Junping Du added a comment - Thanks Kuhu Shukla to report this issue and Sunil G for review! I think Recommission event shouldn't be applied on decommissioned nodes as it won't have any affect and we'd better to keep consistent with previous behavior before graceful decommission comes out. Thus, I would prefer to change "if (entry.getValue().getState() == NodeState.DECOMMISSIONING || entry.getValue().getState() == NodeState.DECOMMISSIONED)" to "if (entry.getValue().getState() == NodeState.DECOMMISSIONING)" to get rid of InvalidState exception in state machine.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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 mvninstall 8m 38s trunk passed
        +1 compile 0m 35s trunk passed with JDK v1.8.0_66
        +1 compile 0m 35s trunk passed with JDK v1.7.0_85
        +1 checkstyle 0m 15s trunk passed
        +1 mvnsite 0m 42s trunk passed
        +1 mvneclipse 0m 17s trunk passed
        +1 findbugs 1m 24s trunk passed
        +1 javadoc 0m 27s trunk passed with JDK v1.8.0_66
        +1 javadoc 0m 30s trunk passed with JDK v1.7.0_85
        +1 mvninstall 0m 39s the patch passed
        +1 compile 0m 33s the patch passed with JDK v1.8.0_66
        +1 javac 0m 33s the patch passed
        +1 compile 0m 35s the patch passed with JDK v1.7.0_85
        +1 javac 0m 35s the patch passed
        +1 checkstyle 0m 15s the patch passed
        +1 mvnsite 0m 42s the patch passed
        +1 mvneclipse 0m 17s the patch passed
        +1 whitespace 0m 0s Patch has no whitespace issues.
        +1 findbugs 1m 33s the patch passed
        +1 javadoc 0m 27s the patch passed with JDK v1.8.0_66
        +1 javadoc 0m 31s the patch passed with JDK v1.7.0_85
        -1 unit 66m 46s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.8.0_66.
        -1 unit 67m 23s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_85.
        +1 asflicense 0m 25s Patch does not generate ASF License warnings.
        154m 43s



        Reason Tests
        JDK v1.8.0_66 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens
          hadoop.yarn.server.resourcemanager.TestAMAuthorization
        JDK v1.7.0_85 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens
          hadoop.yarn.server.resourcemanager.TestAMAuthorization



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:0ca8df7
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12774079/YARN-4386-v1.patch
        JIRA Issue YARN-4386
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 52b8ea35fca2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 28dfe72
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/9781/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_66.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/9781/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_85.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/9781/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/9781/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_85.txt
        JDK v1.7.0_85 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9781/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Max memory used 76MB
        Powered by Apache Yetus http://yetus.apache.org
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/9781/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 8m 38s trunk passed +1 compile 0m 35s trunk passed with JDK v1.8.0_66 +1 compile 0m 35s trunk passed with JDK v1.7.0_85 +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 42s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 1m 24s trunk passed +1 javadoc 0m 27s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 30s trunk passed with JDK v1.7.0_85 +1 mvninstall 0m 39s the patch passed +1 compile 0m 33s the patch passed with JDK v1.8.0_66 +1 javac 0m 33s the patch passed +1 compile 0m 35s the patch passed with JDK v1.7.0_85 +1 javac 0m 35s the patch passed +1 checkstyle 0m 15s the patch passed +1 mvnsite 0m 42s the patch passed +1 mvneclipse 0m 17s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 33s the patch passed +1 javadoc 0m 27s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 31s the patch passed with JDK v1.7.0_85 -1 unit 66m 46s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.8.0_66. -1 unit 67m 23s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_85. +1 asflicense 0m 25s Patch does not generate ASF License warnings. 154m 43s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens   hadoop.yarn.server.resourcemanager.TestAMAuthorization JDK v1.7.0_85 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens   hadoop.yarn.server.resourcemanager.TestAMAuthorization Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12774079/YARN-4386-v1.patch JIRA Issue YARN-4386 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 52b8ea35fca2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 28dfe72 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/9781/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_66.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/9781/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_85.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/9781/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-YARN-Build/9781/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_85.txt JDK v1.7.0_85 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9781/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Max memory used 76MB Powered by Apache Yetus http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-YARN-Build/9781/console This message was automatically generated.
        Hide
        kshukla Kuhu Shukla added a comment -

        refreshNodesGracefully() : if condition only checks for decommissioning nodes. No tests included since this does not exhibit changed behavior before and after the change.

        Show
        kshukla Kuhu Shukla added a comment - refreshNodesGracefully() : if condition only checks for decommissioning nodes. No tests included since this does not exhibit changed behavior before and after the change.
        Hide
        kshukla Kuhu Shukla added a comment -

        Yes I agree. Missed correlating the DECOMMISSIONED state transition to this check. Changing the Priority to Minor.

        Show
        kshukla Kuhu Shukla added a comment - Yes I agree. Missed correlating the DECOMMISSIONED state transition to this check. Changing the Priority to Minor.
        Hide
        sunilg Sunil G added a comment -

        Hi Kuhu Shukla,
        As I see it, we can RECOMMISSION only those nodes which are in DECOMMISSIONING mode. Such nodes are present in getRMNodes which is correct.

        Also if you see RMNodeImpl, RECOMMISSION event is not present from DECOMMISSIONED state. Hence even if it hits the code, it will throw an InvalidState Exception. So looping only in rmContext.getRMNodes() looks fine for me, however I also feel we do not need that extra if check which does for DECOMMISSIONED.
        cc/ Junping Du

        Show
        sunilg Sunil G added a comment - Hi Kuhu Shukla , As I see it, we can RECOMMISSION only those nodes which are in DECOMMISSIONING mode. Such nodes are present in getRMNodes which is correct. Also if you see RMNodeImpl , RECOMMISSION event is not present from DECOMMISSIONED state. Hence even if it hits the code, it will throw an InvalidState Exception. So looping only in rmContext.getRMNodes() looks fine for me, however I also feel we do not need that extra if check which does for DECOMMISSIONED. cc/ Junping Du

          People

          • Assignee:
            kshukla Kuhu Shukla
            Reporter:
            kshukla Kuhu Shukla
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development