Details

      Description

      The state of the parentQueue and its childQeues need to be synchronized.

      • If the state of the parentQueue becomes STOPPED, the state of its childQueue need to become STOPPED as well.
      • If we change the state of the queue to RUNNING, we should make sure the state of all its ancestor must be RUNNING. Otherwise, we need to fail this operation.
      1. YARN-5746.7.patch
        9 kB
        Xuan Gong
      2. YARN-5746.6.patch
        9 kB
        Xuan Gong
      3. YARN-5746.5.patch
        9 kB
        Xuan Gong
      4. YARN-5746.4.patch
        9 kB
        Xuan Gong
      5. YARN-5746.3.patch
        9 kB
        Xuan Gong
      6. YARN-5746.2.patch
        8 kB
        Xuan Gong
      7. YARN-5746.1.patch
        8 kB
        Xuan Gong

        Activity

        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 14s 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.
        +1 mvninstall 9m 7s trunk passed
        +1 compile 0m 35s trunk passed
        +1 checkstyle 0m 22s trunk passed
        +1 mvnsite 0m 43s trunk passed
        +1 mvneclipse 0m 18s trunk passed
        +1 findbugs 1m 1s trunk passed
        +1 javadoc 0m 21s trunk passed
        +1 mvninstall 0m 32s the patch passed
        +1 compile 0m 30s the patch passed
        +1 javac 0m 30s the patch passed
        -1 checkstyle 0m 20s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 3 new + 67 unchanged - 0 fixed = 70 total (was 67)
        +1 mvnsite 0m 37s the patch passed
        +1 mvneclipse 0m 14s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 2s the patch passed
        +1 javadoc 0m 18s the patch passed
        -1 unit 36m 9s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 20s The patch does not generate ASF License warnings.
        53m 23s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.TestRMAdminService



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833859/YARN-5746.1.patch
        JIRA Issue YARN-5746
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 1f1c6163b531 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / b61fb26
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13418/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13418/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13418/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13418/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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13418/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s 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. +1 mvninstall 9m 7s trunk passed +1 compile 0m 35s trunk passed +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 43s trunk passed +1 mvneclipse 0m 18s trunk passed +1 findbugs 1m 1s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 32s the patch passed +1 compile 0m 30s the patch passed +1 javac 0m 30s the patch passed -1 checkstyle 0m 20s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 3 new + 67 unchanged - 0 fixed = 70 total (was 67) +1 mvnsite 0m 37s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 2s the patch passed +1 javadoc 0m 18s the patch passed -1 unit 36m 9s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 53m 23s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMAdminService Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833859/YARN-5746.1.patch JIRA Issue YARN-5746 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1f1c6163b531 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b61fb26 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13418/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13418/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13418/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13418/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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/13418/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 12s 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.
        +1 mvninstall 6m 58s trunk passed
        +1 compile 0m 31s trunk passed
        +1 checkstyle 0m 21s trunk passed
        +1 mvnsite 0m 36s trunk passed
        +1 mvneclipse 0m 17s trunk passed
        +1 findbugs 0m 55s trunk passed
        +1 javadoc 0m 20s trunk passed
        +1 mvninstall 0m 30s the patch passed
        +1 compile 0m 28s the patch passed
        +1 javac 0m 28s the patch passed
        -1 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 68 unchanged - 0 fixed = 69 total (was 68)
        +1 mvnsite 0m 34s the patch passed
        +1 mvneclipse 0m 13s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 0s the patch passed
        +1 javadoc 0m 17s the patch passed
        +1 unit 34m 55s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        49m 18s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834546/YARN-5746.2.patch
        JIRA Issue YARN-5746
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 969c8800fde6 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 262827c
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13457/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13457/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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13457/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s 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. +1 mvninstall 6m 58s trunk passed +1 compile 0m 31s trunk passed +1 checkstyle 0m 21s trunk passed +1 mvnsite 0m 36s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 55s trunk passed +1 javadoc 0m 20s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 28s the patch passed +1 javac 0m 28s the patch passed -1 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 68 unchanged - 0 fixed = 69 total (was 68) +1 mvnsite 0m 34s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 0s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 34m 55s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 49m 18s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834546/YARN-5746.2.patch JIRA Issue YARN-5746 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 969c8800fde6 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 262827c Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13457/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13457/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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/13457/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment -

        Thanks, Xuan Gong. Couple of comments:

        • Let's collapse these nested conditionals into an else if:
                  } else {
                    if (configuredState == QueueState.RUNNING
                        && parentState == QueueState.STOPPED) {
                      throw new IllegalArgumentException(
                          "Illegal" + " State of " + configuredState
                          + " for children of queue: " + queueName
                          + ". The state of its parent queue: " + parent.getQueueName()
                          + " is " + parentState);
                    } else {
                      this.state = configuredState;
                    }
                  }
        • It would be cleaner if getState() were rewritten to use getConfiguredState()
        Show
        templedf Daniel Templeton added a comment - Thanks, Xuan Gong . Couple of comments: Let's collapse these nested conditionals into an else if : } else { if (configuredState == QueueState.RUNNING && parentState == QueueState.STOPPED) { throw new IllegalArgumentException( "Illegal" + " State of " + configuredState + " for children of queue: " + queueName + ". The state of its parent queue: " + parent.getQueueName() + " is " + parentState); } else { this .state = configuredState; } } It would be cleaner if getState() were rewritten to use getConfiguredState()
        Hide
        ozawa Tsuyoshi Ozawa added a comment -

        Xuan Gong thanks for taking this issue.

          public QueueState getConfiguredState(String queue) {
            String state = get(getQueuePrefix(queue) + STATE);
            if (state == null) {
              return null;
            } else {
              return QueueState.valueOf(StringUtils.toUpperCase(state));
            }
        

        It's a bit difficult to understand what the state of "null" mean.
        I would like to suggest that we create new state, QueueState.NOT_FOUND, and return it instead of returning null. What do you think?

        Let's collapse these nested conditionals into an else if:

        +1

        In addition to Daniel's comments, how about adding new private method to wrap up the following routine?

              if (parent != null) {
                QueueState configuredState = csContext.getConfiguration()
                    .getConfiguredState(getQueuePath());
                QueueState parentState = parent.getState();
                if (configuredState == null) {
                  this.state = parentState;
                } else {
                  if (configuredState == QueueState.RUNNING
                      && parentState == QueueState.STOPPED) {
                    throw new IllegalArgumentException(
                        "Illegal" + " State of " + configuredState
                        + " for children of queue: " + queueName
                        + ". The state of its parent queue: " + parent.getQueueName()
                        + " is " + parentState);
                  } else {
                    this.state = configuredState;
                  }
                }
            } else {
                // if this is the root queue, get the state from the configuration.
                // if the state is not set, use RUNNING as default state.
                this.state = csContext.getConfiguration().getState(getQueuePath());
              }
        
        Show
        ozawa Tsuyoshi Ozawa added a comment - Xuan Gong thanks for taking this issue. public QueueState getConfiguredState( String queue) { String state = get(getQueuePrefix(queue) + STATE); if (state == null ) { return null ; } else { return QueueState.valueOf(StringUtils.toUpperCase(state)); } It's a bit difficult to understand what the state of "null" mean. I would like to suggest that we create new state, QueueState.NOT_FOUND, and return it instead of returning null. What do you think? Let's collapse these nested conditionals into an else if: +1 In addition to Daniel's comments, how about adding new private method to wrap up the following routine? if (parent != null ) { QueueState configuredState = csContext.getConfiguration() .getConfiguredState(getQueuePath()); QueueState parentState = parent.getState(); if (configuredState == null ) { this .state = parentState; } else { if (configuredState == QueueState.RUNNING && parentState == QueueState.STOPPED) { throw new IllegalArgumentException( "Illegal" + " State of " + configuredState + " for children of queue: " + queueName + ". The state of its parent queue: " + parent.getQueueName() + " is " + parentState); } else { this .state = configuredState; } } } else { // if this is the root queue, get the state from the configuration. // if the state is not set, use RUNNING as default state. this .state = csContext.getConfiguration().getState(getQueuePath()); }
        Hide
        xgong Xuan Gong added a comment -

        Thanks for the review. Tsuyoshi Ozawa Daniel Templeton

        I would like to suggest that we create new state, QueueState.NOT_FOUND, and return it instead of returning null. What do you think?

        I would like to keep null as the return value here. In the design doc of YARN-5724, we will create a State Machine for the queue, NOT_FOUND does not sound like a valid state for me.

        Uploaded a new patch to address all other comments.

        Show
        xgong Xuan Gong added a comment - Thanks for the review. Tsuyoshi Ozawa Daniel Templeton I would like to suggest that we create new state, QueueState.NOT_FOUND, and return it instead of returning null. What do you think? I would like to keep null as the return value here. In the design doc of YARN-5724 , we will create a State Machine for the queue, NOT_FOUND does not sound like a valid state for me. Uploaded a new patch to address all other comments.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 21s 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.
        +1 mvninstall 6m 46s trunk passed
        +1 compile 0m 32s trunk passed
        +1 checkstyle 0m 21s trunk passed
        +1 mvnsite 0m 39s trunk passed
        +1 mvneclipse 0m 17s trunk passed
        +1 findbugs 0m 56s trunk passed
        +1 javadoc 0m 21s trunk passed
        +1 mvninstall 0m 31s the patch passed
        +1 compile 0m 29s the patch passed
        +1 javac 0m 29s the patch passed
        -0 checkstyle 0m 18s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 67 unchanged - 0 fixed = 68 total (was 67)
        +1 mvnsite 0m 36s the patch passed
        +1 mvneclipse 0m 14s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 3s the patch passed
        +1 javadoc 0m 18s the patch passed
        +1 unit 35m 11s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        50m 27s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue YARN-5746
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836187/YARN-5746.3.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux f5dfe6298158 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 / f646fe3
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13698/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13698/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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13698/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s 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. +1 mvninstall 6m 46s trunk passed +1 compile 0m 32s trunk passed +1 checkstyle 0m 21s trunk passed +1 mvnsite 0m 39s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 56s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 31s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed -0 checkstyle 0m 18s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 67 unchanged - 0 fixed = 68 total (was 67) +1 mvnsite 0m 36s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 3s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 35m 11s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 50m 27s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-5746 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836187/YARN-5746.3.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f5dfe6298158 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 / f646fe3 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13698/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13698/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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/13698/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Xuan Gong, thanks for working on the patch.

        Does it make sense to you to move all implementations to parseQueue(...)? It looks more clean to me. It looks like we don't need to change any of AbstractCSQueue and CapacitySchedulerConfiguarition.

        And I suggest to build this patch on top of YARN-5761.

        Show
        leftnoteasy Wangda Tan added a comment - Xuan Gong , thanks for working on the patch. Does it make sense to you to move all implementations to parseQueue(...)? It looks more clean to me. It looks like we don't need to change any of AbstractCSQueue and CapacitySchedulerConfiguarition. And I suggest to build this patch on top of YARN-5761 .
        Hide
        jhung Jonathan Hung added a comment -

        Xuan Gong It looks good to me, just one small nit, on the line "Illegal" + " State of " + configuredState you can combine the strings.

        Does it make sense to you to move all implementations to parseQueue(...)? It looks more clean to me. It looks like we don't need to change any of AbstractCSQueue and CapacitySchedulerConfiguarition.

        Wangda Tan can you elaborate on this for me? Do you mean setting the queue's state inside CapacityScheduler#parseQueue? I think it makes sense to leave it at AbstractCSQueue#setupQueueConfigs, let me know if I am missing something.

        Show
        jhung Jonathan Hung added a comment - Xuan Gong It looks good to me, just one small nit, on the line "Illegal" + " State of " + configuredState you can combine the strings. Does it make sense to you to move all implementations to parseQueue(...)? It looks more clean to me. It looks like we don't need to change any of AbstractCSQueue and CapacitySchedulerConfiguarition. Wangda Tan can you elaborate on this for me? Do you mean setting the queue's state inside CapacityScheduler#parseQueue ? I think it makes sense to leave it at AbstractCSQueue#setupQueueConfigs , let me know if I am missing something.
        Hide
        xgong Xuan Gong added a comment -

        rebase the patch

        Show
        xgong Xuan Gong added a comment - rebase the patch
        Hide
        jianhe Jian He added a comment - - edited

        the message could be more clear as "Parent queue (queueName) state is STOPPED, child queue (queueName) state cannot be RUNNING"
        otherwise, looks good to me

        Show
        jianhe Jian He added a comment - - edited the message could be more clear as "Parent queue (queueName) state is STOPPED, child queue (queueName) state cannot be RUNNING" otherwise, looks good to me
        Hide
        xgong Xuan Gong added a comment -

        Thanks for the review. Jian He

        Attached a new patch

        Show
        xgong Xuan Gong added a comment - Thanks for the review. Jian He Attached a new patch
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 15s 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.
        +1 mvninstall 7m 43s trunk passed
        +1 compile 0m 33s trunk passed
        +1 checkstyle 0m 22s trunk passed
        +1 mvnsite 0m 43s trunk passed
        +1 mvneclipse 0m 18s trunk passed
        +1 findbugs 1m 3s trunk passed
        +1 javadoc 0m 22s trunk passed
        +1 mvninstall 0m 34s the patch passed
        +1 compile 0m 32s the patch passed
        +1 javac 0m 32s the patch passed
        -0 checkstyle 0m 20s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 68 unchanged - 0 fixed = 69 total (was 68)
        +1 mvnsite 0m 41s the patch passed
        +1 mvneclipse 0m 15s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 9s the patch passed
        +1 javadoc 0m 19s the patch passed
        -1 unit 39m 9s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        55m 51s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestQueueState
          hadoop.yarn.server.resourcemanager.TestRMRestart



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-5746
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841150/YARN-5746.5.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 4454d7fcedac 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 69fb70c
        Default Java 1.8.0_111
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14134/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/14134/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14134/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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14134/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s 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. +1 mvninstall 7m 43s trunk passed +1 compile 0m 33s trunk passed +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 43s trunk passed +1 mvneclipse 0m 18s trunk passed +1 findbugs 1m 3s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 34s the patch passed +1 compile 0m 32s the patch passed +1 javac 0m 32s the patch passed -0 checkstyle 0m 20s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 68 unchanged - 0 fixed = 69 total (was 68) +1 mvnsite 0m 41s the patch passed +1 mvneclipse 0m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 9s the patch passed +1 javadoc 0m 19s the patch passed -1 unit 39m 9s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 55m 51s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestQueueState   hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5746 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841150/YARN-5746.5.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4454d7fcedac 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 69fb70c Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14134/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14134/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14134/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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/14134/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        xgong Xuan Gong added a comment -

        Fix the checkstyle issue

        Show
        xgong Xuan Gong added a comment - Fix the checkstyle issue
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 11s 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.
        +1 mvninstall 7m 0s trunk passed
        +1 compile 0m 33s trunk passed
        +1 checkstyle 0m 21s trunk passed
        +1 mvnsite 0m 40s trunk passed
        +1 mvneclipse 0m 17s trunk passed
        +1 findbugs 0m 59s trunk passed
        +1 javadoc 0m 20s trunk passed
        +1 mvninstall 0m 32s the patch passed
        +1 compile 0m 30s the patch passed
        +1 javac 0m 30s the patch passed
        +1 checkstyle 0m 18s the patch passed
        +1 mvnsite 0m 35s the patch passed
        +1 mvneclipse 0m 14s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 3s the patch passed
        +1 javadoc 0m 19s the patch passed
        -1 unit 38m 44s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 18s The patch does not generate ASF License warnings.
        54m 13s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestQueueState



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-5746
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841332/YARN-5746.6.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 11cfb2c855bd 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 96c5749
        Default Java 1.8.0_111
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/14146/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14146/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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14146/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s 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. +1 mvninstall 7m 0s trunk passed +1 compile 0m 33s trunk passed +1 checkstyle 0m 21s trunk passed +1 mvnsite 0m 40s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 59s trunk passed +1 javadoc 0m 20s trunk passed +1 mvninstall 0m 32s the patch passed +1 compile 0m 30s the patch passed +1 javac 0m 30s the patch passed +1 checkstyle 0m 18s the patch passed +1 mvnsite 0m 35s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 3s the patch passed +1 javadoc 0m 19s the patch passed -1 unit 38m 44s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 54m 13s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestQueueState Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5746 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841332/YARN-5746.6.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 11cfb2c855bd 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 96c5749 Default Java 1.8.0_111 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/14146/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14146/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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/14146/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        xgong Xuan Gong added a comment -

        Fix the unit test failure

        Show
        xgong Xuan Gong added a comment - Fix the unit test failure
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 14s 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.
        +1 mvninstall 6m 49s trunk passed
        +1 compile 0m 34s trunk passed
        +1 checkstyle 0m 21s trunk passed
        +1 mvnsite 0m 39s trunk passed
        +1 mvneclipse 0m 17s trunk passed
        +1 findbugs 0m 57s trunk passed
        +1 javadoc 0m 22s trunk passed
        +1 mvninstall 0m 31s the patch passed
        +1 compile 0m 31s the patch passed
        +1 javac 0m 31s the patch passed
        +1 checkstyle 0m 18s the patch passed
        +1 mvnsite 0m 36s the patch passed
        +1 mvneclipse 0m 15s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 4s the patch passed
        +1 javadoc 0m 19s the patch passed
        +1 unit 42m 15s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        57m 33s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-5746
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841386/YARN-5746.7.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 2cd21036df09 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 2ff84a0
        Default Java 1.8.0_111
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14169/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
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14169/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s 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. +1 mvninstall 6m 49s trunk passed +1 compile 0m 34s trunk passed +1 checkstyle 0m 21s trunk passed +1 mvnsite 0m 39s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 57s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 31s the patch passed +1 compile 0m 31s the patch passed +1 javac 0m 31s the patch passed +1 checkstyle 0m 18s the patch passed +1 mvnsite 0m 36s the patch passed +1 mvneclipse 0m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 4s the patch passed +1 javadoc 0m 19s the patch passed +1 unit 42m 15s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 57m 33s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5746 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841386/YARN-5746.7.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2cd21036df09 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2ff84a0 Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14169/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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/14169/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        jianhe Jian He added a comment -

        Committed to branch-2, trunk, thanks Xuan !
        Thanks Tsuyoshi Ozawa, Jonathan Hung for the review !

        Show
        jianhe Jian He added a comment - Committed to branch-2, trunk, thanks Xuan ! Thanks Tsuyoshi Ozawa , Jonathan Hung for the review !
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10934 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10934/)
        YARN-5746. The state of the parentQueue and its childQueues should be (jianhe: rev f885160f4ac56a0999e3b051eb7bccce928c1c33)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestQueueState.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10934 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10934/ ) YARN-5746 . The state of the parentQueue and its childQueues should be (jianhe: rev f885160f4ac56a0999e3b051eb7bccce928c1c33) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestQueueState.java

          People

          • Assignee:
            xgong Xuan Gong
            Reporter:
            xgong Xuan Gong
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development