Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-5437 [Umbrella] Add more debug/diagnostic messages to scheduler
  3. YARN-4329

Allow fetching exact reason as to why a submitted app is in ACCEPTED state in Fair Scheduler

    Details

      Description

      Similar to YARN-3946, it would be useful to capture possible reason why the Application is in accepted state in FairScheduler

      1. Screen Shot 2016-10-18 at 3.13.59 PM.png
        190 kB
        Yufei Gu
      2. YARN-4329.001.patch
        11 kB
        Yufei Gu
      3. YARN-4329.002.patch
        11 kB
        Yufei Gu
      4. YARN-4329.003.patch
        12 kB
        Yufei Gu
      5. YARN-4329.004.patch
        12 kB
        Yufei Gu
      6. YARN-4329.005.patch
        12 kB
        Yufei Gu

        Issue Links

          Activity

          Hide
          yufeigu Yufei Gu added a comment - - edited

          Close YARN-5563 to work on this one. There are some reason why an app is in ACCEPTED state but doesn't run in FairScheduler.
          (1) Exceed queue max apps,
          (2) Exceed user max apps,
          (3) Exceed queue maxResources,
          (4) Exceed maxAMShare.

          Show
          yufeigu Yufei Gu added a comment - - edited Close YARN-5563 to work on this one. There are some reason why an app is in ACCEPTED state but doesn't run in FairScheduler. (1) Exceed queue max apps, (2) Exceed user max apps, (3) Exceed queue maxResources, (4) Exceed maxAMShare.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Yufei Gu for working on this! As i am not much acquainted with Fair Scheduler (and was only aware of the first 2 in the list) i did not go ahead with working on it, And yes its better than logging (as per YARN-5563) as it will be availble from REST/CLI/WEB etc...
          Basic framework is already set as part of YARN-3946, If you have any concerns or queries you can reach me.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Yufei Gu for working on this! As i am not much acquainted with Fair Scheduler (and was only aware of the first 2 in the list) i did not go ahead with working on it, And yes its better than logging (as per YARN-5563 ) as it will be availble from REST/CLI/WEB etc... Basic framework is already set as part of YARN-3946 , If you have any concerns or queries you can reach me.
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks Naganarasimha G R! I found you've done basic framework in YARN-3946. That's great! Thanks. Please comment if you have any heads-up or concern.

          Show
          yufeigu Yufei Gu added a comment - Thanks Naganarasimha G R ! I found you've done basic framework in YARN-3946 . That's great! Thanks. Please comment if you have any heads-up or concern.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s 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 8m 33s trunk passed
          +1 compile 0m 35s trunk passed
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 41s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 1m 0s trunk passed
          +1 javadoc 0m 24s trunk passed
          +1 mvninstall 0m 33s the patch passed
          +1 compile 0m 30s the patch passed
          +1 javac 0m 30s the patch passed
          +1 checkstyle 0m 20s 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 7s the patch passed
          +1 javadoc 0m 18s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 0 new + 938 unchanged - 3 fixed = 938 total (was 941)
          +1 unit 34m 6s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          50m 47s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829910/YARN-4329.001.patch
          JIRA Issue YARN-4329
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 1a042db26afd 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 / 40acace
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13188/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/13188/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 16s 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 8m 33s trunk passed +1 compile 0m 35s trunk passed +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 41s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 1m 0s trunk passed +1 javadoc 0m 24s trunk passed +1 mvninstall 0m 33s the patch passed +1 compile 0m 30s the patch passed +1 javac 0m 30s the patch passed +1 checkstyle 0m 20s 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 7s the patch passed +1 javadoc 0m 18s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 0 new + 938 unchanged - 3 fixed = 938 total (was 941) +1 unit 34m 6s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 50m 47s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829910/YARN-4329.001.patch JIRA Issue YARN-4329 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1a042db26afd 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 / 40acace Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13188/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/13188/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 for the patch, Yufei Gu.

          • In FSAppAttempt.hasContainerForNode(), you have
            if (cond1) {
              ...
              return false;
            }
            if (cond2) {
              ...
              return false;
            }
            return true;

            I'd much rather see

            boolean ret = true;
            if (cond1) {
              ...
              ret = false;
            } else if (cond2) {
              ...
              ret = false;
            }
            return ret;
          • Same thing in MaxRunningAppsEnforcer.canAppBeRunnable().
          • When you're building your message strings, use two appends instead of doing a string concat inside the append

          Love the refactor of MaxRunningAppsEnforcer.canAppBeRunnable(). Thanks for making that cleaner.

          Show
          templedf Daniel Templeton added a comment - Thanks for the patch, Yufei Gu . In FSAppAttempt.hasContainerForNode() , you have if (cond1) { ... return false ; } if (cond2) { ... return false ; } return true ; I'd much rather see boolean ret = true ; if (cond1) { ... ret = false ; } else if (cond2) { ... ret = false ; } return ret; Same thing in MaxRunningAppsEnforcer.canAppBeRunnable() . When you're building your message strings, use two appends instead of doing a string concat inside the append Love the refactor of MaxRunningAppsEnforcer.canAppBeRunnable() . Thanks for making that cleaner.
          Hide
          yufeigu Yufei Gu added a comment - - edited

          Hi Daniel Templeton, thanks for the review. I uploaded the new patch for your comments. I didn't change

                  diagnosticMessageBldr.append(" exceeds current queue or its parents"
                      + " maximum resource allowed).");
          

          to two appends since concatenation of string constants is done at compile time.

          Show
          yufeigu Yufei Gu added a comment - - edited Hi Daniel Templeton , thanks for the review. I uploaded the new patch for your comments. I didn't change diagnosticMessageBldr.append( " exceeds current queue or its parents" + " maximum resource allowed)." ); to two appends since concatenation of string constants is done at compile time.
          Hide
          templedf Daniel Templeton added a comment -

          Thanks, yufei. Maybe I'm an old codger, but I'd really rather you have a single return point from FSAppAttempt.hasContainerForNode() and MaxRunningAppsEnforcer.canAppBeRunnable(). There's no advantage to having multiple exit points in this case, and it makes maintenance a tiny bit harder.

          Show
          templedf Daniel Templeton added a comment - Thanks, yufei . Maybe I'm an old codger, but I'd really rather you have a single return point from FSAppAttempt.hasContainerForNode() and MaxRunningAppsEnforcer.canAppBeRunnable() . There's no advantage to having multiple exit points in this case, and it makes maintenance a tiny bit harder.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s 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 10s trunk passed
          +1 compile 0m 36s trunk passed
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 0m 41s trunk passed
          +1 mvneclipse 0m 20s trunk passed
          +1 findbugs 1m 10s trunk passed
          +1 javadoc 0m 24s trunk passed
          +1 mvninstall 0m 39s the patch passed
          +1 compile 0m 35s the patch passed
          +1 javac 0m 35s the patch passed
          +1 checkstyle 0m 22s the patch passed
          +1 mvnsite 0m 42s the patch passed
          +1 mvneclipse 0m 17s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 20s the patch passed
          +1 javadoc 0m 20s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 0 new + 938 unchanged - 3 fixed = 938 total (was 941)
          +1 unit 35m 18s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          53m 36s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830760/YARN-4329.002.patch
          JIRA Issue YARN-4329
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 39b5cdbe6643 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 / e19b37e
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13239/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/13239/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 17s 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 10s trunk passed +1 compile 0m 36s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 0m 41s trunk passed +1 mvneclipse 0m 20s trunk passed +1 findbugs 1m 10s trunk passed +1 javadoc 0m 24s trunk passed +1 mvninstall 0m 39s the patch passed +1 compile 0m 35s the patch passed +1 javac 0m 35s the patch passed +1 checkstyle 0m 22s the patch passed +1 mvnsite 0m 42s the patch passed +1 mvneclipse 0m 17s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 20s the patch passed +1 javadoc 0m 20s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 0 new + 938 unchanged - 3 fixed = 938 total (was 941) +1 unit 35m 18s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 53m 36s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830760/YARN-4329.002.patch JIRA Issue YARN-4329 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 39b5cdbe6643 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 / e19b37e Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13239/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/13239/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for working on the patch Yufei Gu and sorry for the delayed response,
          One major comment would be in CS when a node got skipped we were capturing that info too refer @ FiCaSchedulerApp.updateNodeInfoForAMDiagnostics(FiCaSchedulerNode), this would give some information about why a given node missed to schedule the app too. Since the approach is different may be we need to handle it slightly differently here but it would be good to capture
          And other minor nits :

          • SchedulerApplicationAttempt lno 1030 : Is this comment req ?
          • FSAppAttempt lno 820..833 : line indentation is wrong, needs formatting
          • FSAppAttempt lno 838..844 & 995..1000: looks similar may be can be optimized by having a private method ?
          • TestMaxRunningAppsEnforcer : test cases seems to not to cover all the scenarios
          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for working on the patch Yufei Gu and sorry for the delayed response, One major comment would be in CS when a node got skipped we were capturing that info too refer @ FiCaSchedulerApp.updateNodeInfoForAMDiagnostics(FiCaSchedulerNode) , this would give some information about why a given node missed to schedule the app too. Since the approach is different may be we need to handle it slightly differently here but it would be good to capture And other minor nits : SchedulerApplicationAttempt lno 1030 : Is this comment req ? FSAppAttempt lno 820..833 : line indentation is wrong, needs formatting FSAppAttempt lno 838..844 & 995..1000: looks similar may be can be optimized by having a private method ? TestMaxRunningAppsEnforcer : test cases seems to not to cover all the scenarios
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks Daniel Templeton's review. The new patched solve the comments.
          Thanks. Naganarasimha G R's review. Nice catch for the node skipping. I've uploaded the new patch for that. There are some reasons that a node got skipped. For example, there are not enough resources for the container in the node. The new patch covers this. Since AM resource request doesn't have the locality constraint, there is no need to check that.
          For your other comments:
          1. The comment is deleted.
          2. Formatted
          3. Done.
          4. Kind of out of scope, we can open another JIRA for it.

          Show
          yufeigu Yufei Gu added a comment - Thanks Daniel Templeton 's review. The new patched solve the comments. Thanks. Naganarasimha G R 's review. Nice catch for the node skipping. I've uploaded the new patch for that. There are some reasons that a node got skipped. For example, there are not enough resources for the container in the node. The new patch covers this. Since AM resource request doesn't have the locality constraint, there is no need to check that. For your other comments: 1. The comment is deleted. 2. Formatted 3. Done. 4. Kind of out of scope, we can open another JIRA for it.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s 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 42s trunk passed
          +1 compile 0m 31s trunk passed
          +1 checkstyle 0m 21s trunk passed
          +1 mvnsite 0m 38s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          +1 findbugs 0m 54s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 30s the patch passed
          +1 compile 0m 29s the patch passed
          +1 javac 0m 29s the patch passed
          +1 checkstyle 0m 18s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 80 unchanged - 1 fixed = 80 total (was 81)
          +1 mvnsite 0m 35s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 1s the patch passed
          +1 javadoc 0m 17s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 0 new + 935 unchanged - 3 fixed = 935 total (was 938)
          +1 unit 38m 22s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          52m 39s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831595/YARN-4329.003.patch
          JIRA Issue YARN-4329
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 3d9263a81fc8 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 / 88b9444
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13280/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/13280/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 17s 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 42s trunk passed +1 compile 0m 31s trunk passed +1 checkstyle 0m 21s trunk passed +1 mvnsite 0m 38s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 0m 54s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed +1 checkstyle 0m 18s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 80 unchanged - 1 fixed = 80 total (was 81) +1 mvnsite 0m 35s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 1s the patch passed +1 javadoc 0m 17s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 0 new + 935 unchanged - 3 fixed = 935 total (was 938) +1 unit 38m 22s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 52m 39s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831595/YARN-4329.003.patch JIRA Issue YARN-4329 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 3d9263a81fc8 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 / 88b9444 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13280/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/13280/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          TestMaxRunningAppsEnforcer : test cases seems to not to cover all the scenarios

          I meant this is only change in test case so not sure whether manual testing is covering all the scenarios. Hope either you update some snapshots in the of the webui for which you have tested or include related testcases to ascertain sufficient testing has been done. Generally in such cases we tend to see overlapping of messages or some issues with the syntax of messages hence mentioned for testing.

          Show
          Naganarasimha Naganarasimha G R added a comment - TestMaxRunningAppsEnforcer : test cases seems to not to cover all the scenarios I meant this is only change in test case so not sure whether manual testing is covering all the scenarios. Hope either you update some snapshots in the of the webui for which you have tested or include related testcases to ascertain sufficient testing has been done. Generally in such cases we tend to see overlapping of messages or some issues with the syntax of messages hence mentioned for testing.
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks for the explanation. That makes sense. I will update a new patch soon.

          Show
          yufeigu Yufei Gu added a comment - Thanks for the explanation. That makes sense. I will update a new patch soon.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Would like to see test cases for all scenarios but if you feel not all are possible then would suggest atleast you do the manual testing for them.

          Show
          Naganarasimha Naganarasimha G R added a comment - Would like to see test cases for all scenarios but if you feel not all are possible then would suggest atleast you do the manual testing for them.
          Hide
          yufeigu Yufei Gu added a comment -

          Hi Naganarasimha G R, I uploaded the screenshot of test results. Basically, I spun up a cluster with my patch, then submitted jobs and took screenshot.

          Show
          yufeigu Yufei Gu added a comment - Hi Naganarasimha G R , I uploaded the screenshot of test results. Basically, I spun up a cluster with my patch, then submitted jobs and took screenshot.
          Hide
          templedf Daniel Templeton added a comment -

          Latest patch looks great. A couple of tiny quibbles:

          • These lines
                 boolean nowRunnable = maxRunningEnforcer.canAppBeRunnable(newQueue,
                    attempt);

            can probably be combined into one.

          • Your messages "User runnable application number has reached its maximum limit." and "Queue runnable application number has reached its maximum limit." would be clearer as "The user has reached the maximum limit of runnable applications." and "The <queue_name> queue has reached the maximum limit of runnable applications.".
          • In some of your javadocs you put a newline between the description and params, and in some you don't. Better to be consistent. I prefer it with the newline.
          Show
          templedf Daniel Templeton added a comment - Latest patch looks great. A couple of tiny quibbles: These lines boolean nowRunnable = maxRunningEnforcer.canAppBeRunnable(newQueue, attempt); can probably be combined into one. Your messages "User runnable application number has reached its maximum limit." and "Queue runnable application number has reached its maximum limit." would be clearer as "The user has reached the maximum limit of runnable applications." and "The <queue_name> queue has reached the maximum limit of runnable applications." . In some of your javadocs you put a newline between the description and params, and in some you don't. Better to be consistent. I prefer it with the newline.
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks Daniel Templeton for the review. I've uploaded patch 004 for all your comments except the first one since there will be 81 characters in one line if I combine these two, which will trigger style checking issue.

          Show
          yufeigu Yufei Gu added a comment - Thanks Daniel Templeton for the review. I've uploaded patch 004 for all your comments except the first one since there will be 81 characters in one line if I combine these two, which will trigger style checking issue.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 4s YARN-4329 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Issue YARN-4329
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836478/YARN-4329.004.patch
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13744/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 0s Docker mode activated. -1 patch 0m 4s YARN-4329 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue YARN-4329 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836478/YARN-4329.004.patch Console output https://builds.apache.org/job/PreCommit-YARN-Build/13744/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          templedf Daniel Templeton added a comment -

          Latest patch looks good to me, but Jenkins doesn't seem to like it.

          Show
          templedf Daniel Templeton added a comment - Latest patch looks good to me, but Jenkins doesn't seem to like it.
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks Daniel Templeton for the review. I've rebased and uploaded a new patch.

          Show
          yufeigu Yufei Gu added a comment - Thanks Daniel Templeton for the review. I've rebased and uploaded a new patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s 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 2s trunk passed
          +1 compile 0m 34s trunk passed
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 40s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 0m 58s trunk passed
          +1 javadoc 0m 22s trunk passed
          +1 mvninstall 0m 33s the patch passed
          +1 compile 0m 30s the patch passed
          +1 javac 0m 30s the patch passed
          +1 checkstyle 0m 19s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 69 unchanged - 1 fixed = 69 total (was 70)
          +1 mvnsite 0m 37s the patch passed
          +1 mvneclipse 0m 15s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 2s the patch passed
          +1 javadoc 0m 18s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 0 new + 924 unchanged - 3 fixed = 924 total (was 927)
          +1 unit 35m 49s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          51m 31s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:e809691
          JIRA Issue YARN-4329
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12837927/YARN-4329.005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux deeb7f00d639 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 / f38a6d0
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13821/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/13821/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 17s 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 2s trunk passed +1 compile 0m 34s trunk passed +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 40s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 58s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 33s the patch passed +1 compile 0m 30s the patch passed +1 javac 0m 30s the patch passed +1 checkstyle 0m 19s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 0 new + 69 unchanged - 1 fixed = 69 total (was 70) +1 mvnsite 0m 37s the patch passed +1 mvneclipse 0m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 2s the patch passed +1 javadoc 0m 18s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 0 new + 924 unchanged - 3 fixed = 924 total (was 927) +1 unit 35m 49s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 51m 31s Subsystem Report/Notes Docker Image:yetus/hadoop:e809691 JIRA Issue YARN-4329 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12837927/YARN-4329.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux deeb7f00d639 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 / f38a6d0 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13821/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/13821/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          templedf Daniel Templeton added a comment -

          +1 I'll commit the latest patch shortly.

          Show
          templedf Daniel Templeton added a comment - +1 I'll commit the latest patch shortly.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10804 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10804/)
          YARN-4329. YARN-5437 Allow fetching exact reason as to why a submitted (templedf: rev 59ee8b7a88603e94b5661a8d5d088f7aa99fe049)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestMaxRunningAppsEnforcer.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FSAppAttempt.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/MaxRunningAppsEnforcer.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10804 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10804/ ) YARN-4329 . YARN-5437 Allow fetching exact reason as to why a submitted (templedf: rev 59ee8b7a88603e94b5661a8d5d088f7aa99fe049) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestMaxRunningAppsEnforcer.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FSAppAttempt.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/MaxRunningAppsEnforcer.java
          Hide
          templedf Daniel Templeton added a comment -

          Committed to branch-2 and trunk. Thanks for the patch, Yufei Gu, and the reviews, Naganarasimha G R.

          Show
          templedf Daniel Templeton added a comment - Committed to branch-2 and trunk. Thanks for the patch, Yufei Gu , and the reviews, Naganarasimha G R .
          Hide
          yufeigu Yufei Gu added a comment -

          Thanks Daniel Templeton for the review and commit! Thanks Naganarasimha G R for the review.

          Show
          yufeigu Yufei Gu added a comment - Thanks Daniel Templeton for the review and commit! Thanks Naganarasimha G R for the review.
          Hide
          djp Junping Du added a comment -

          This patch goes to branch-2 only instead of branch-2.8, set 2.9 as fix version.

          Show
          djp Junping Du added a comment - This patch goes to branch-2 only instead of branch-2.8, set 2.9 as fix version.

            People

            • Assignee:
              yufeigu Yufei Gu
              Reporter:
              Naganarasimha Naganarasimha G R
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development