Details
-
Sub-task
-
Status: Resolved
-
Major
-
Resolution: Fixed
-
None
-
None
-
Reviewed
Description
This causes a NPE issue. Beside the NPE, the metrics won't reflect the different attempts. We should pass ApplicationId Instead of AppAttemptId. The NPE caused by the issue:
2017-03-13 20:43:39,153 INFO appmaster.AMSimulator: Submit a new application application_1489463017173_0001 java.lang.NullPointerException at org.apache.hadoop.yarn.server.resourcemanager.scheduler.AbstractYarnScheduler.getApplicationAttempt(AbstractYarnScheduler.java:327) at org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.FairScheduler.getSchedulerApp(FairScheduler.java:1028) at org.apache.hadoop.yarn.sls.scheduler.FairSchedulerMetrics.trackApp(FairSchedulerMetrics.java:68) at org.apache.hadoop.yarn.sls.scheduler.ResourceSchedulerWrapper.addTrackedApp(ResourceSchedulerWrapper.java:799) at org.apache.hadoop.yarn.sls.appmaster.AMSimulator.trackApp(AMSimulator.java:338) at org.apache.hadoop.yarn.sls.appmaster.AMSimulator.firstStep(AMSimulator.java:156) at org.apache.hadoop.yarn.sls.scheduler.TaskRunner$Task.run(TaskRunner.java:90) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745) Exception in thread "pool-6-thread-1" java.lang.NullPointerException at org.apache.hadoop.yarn.sls.scheduler.TaskRunner$Task.run(TaskRunner.java:105) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)
Attachments
Attachments
- YARN-6326.005.patch
- 26 kB
- Yufei Gu
- YARN-6326.004.patch
- 29 kB
- Yufei Gu
- YARN-6326.003.patch
- 28 kB
- Yufei Gu
- YARN-6326.002.patch
- 24 kB
- Yufei Gu
- YARN-6326.001.patch
- 17 kB
- Yufei Gu
Issue Links
- is depended upon by
-
YARN-5654 Not be able to run SLS with FairScheduler
- Resolved
Activity
Seems fine overall; some minor things:
- whitespace as reported by Jenkins
- This code gets called a lot, I think we should make it a method so we can reduce code duplication:
FSAppAttempt appAttempt = (FSAppAttempt) app.getCurrentAppAttempt(); if (appAttempt != null) { return appAttempt.getFairShare().getVirtualCores(); } else { return 0; }
- It seems funny to change the ResourceScheduler in ResourceSchedulerWrapper to AbstractYarnScheduler.
Thanks rkanter for the review. Uploaded a new patch for your comments. I found it makes more sense to add a method for YarnScheduler to get an application by appId instead of changing ResourceScheduler to AbstractYarnScheduler.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 20s | 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. |
0 | mvndep | 1m 54s | Maven dependency ordering for branch |
+1 | mvninstall | 12m 52s | trunk passed |
-1 | compile | 8m 19s | root in trunk failed. |
+1 | checkstyle | 2m 8s | trunk passed |
+1 | mvnsite | 1m 21s | trunk passed |
+1 | mvneclipse | 1m 0s | trunk passed |
+1 | findbugs | 1m 54s | trunk passed |
+1 | javadoc | 1m 3s | trunk passed |
0 | mvndep | 0m 17s | Maven dependency ordering for patch |
+1 | mvninstall | 0m 51s | the patch passed |
-1 | compile | 6m 35s | root in the patch failed. |
-1 | javac | 6m 35s | root in the patch failed. |
-0 | checkstyle | 2m 4s | root: The patch generated 2 new + 232 unchanged - 44 fixed = 234 total (was 276) |
+1 | mvnsite | 1m 19s | the patch passed |
+1 | mvneclipse | 0m 57s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 2m 11s | the patch passed |
+1 | javadoc | 1m 5s | the patch passed |
+1 | unit | 42m 57s | hadoop-yarn-server-resourcemanager in the patch passed. |
+1 | unit | 1m 9s | hadoop-sls in the patch passed. |
+1 | asflicense | 0m 49s | The patch does not generate ASF License warnings. |
116m 12s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12858611/YARN-6326.002.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 9f1edec78e0c 3.13.0-108-generic #155-Ubuntu SMP Wed Jan 11 16:58:52 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 9832ae0 |
Default Java | 1.8.0_121 |
compile | https://builds.apache.org/job/PreCommit-YARN-Build/15255/artifact/patchprocess/branch-compile-root.txt |
findbugs | v3.0.0 |
compile | https://builds.apache.org/job/PreCommit-YARN-Build/15255/artifact/patchprocess/patch-compile-root.txt |
javac | https://builds.apache.org/job/PreCommit-YARN-Build/15255/artifact/patchprocess/patch-compile-root.txt |
checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/15255/artifact/patchprocess/diff-checkstyle-root.txt |
Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/15255/testReport/ |
modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: . |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/15255/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
-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. |
0 | mvndep | 0m 14s | Maven dependency ordering for branch |
+1 | mvninstall | 12m 25s | trunk passed |
-1 | compile | 7m 0s | root in trunk failed. |
+1 | checkstyle | 2m 3s | trunk passed |
+1 | mvnsite | 1m 13s | trunk passed |
+1 | mvneclipse | 0m 52s | trunk passed |
+1 | findbugs | 1m 51s | trunk passed |
+1 | javadoc | 1m 1s | trunk passed |
0 | mvndep | 0m 16s | Maven dependency ordering for patch |
+1 | mvninstall | 0m 50s | the patch passed |
-1 | compile | 6m 30s | root in the patch failed. |
-1 | javac | 6m 30s | root in the patch failed. |
-0 | checkstyle | 2m 4s | root: The patch generated 2 new + 233 unchanged - 45 fixed = 235 total (was 278) |
+1 | mvnsite | 1m 19s | the patch passed |
+1 | mvneclipse | 0m 57s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 2m 12s | the patch passed |
+1 | javadoc | 1m 6s | the patch passed |
+1 | unit | 39m 38s | hadoop-yarn-server-resourcemanager in the patch passed. |
+1 | unit | 1m 9s | hadoop-sls in the patch passed. |
-1 | asflicense | 0m 48s | The patch generated 43 ASF License warnings. |
108m 42s |
This message was automatically generated.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 18s | 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 14s | Maven dependency ordering for branch |
+1 | mvninstall | 12m 29s | trunk passed |
-1 | compile | 7m 0s | root in trunk failed. |
+1 | checkstyle | 2m 5s | trunk passed |
+1 | mvnsite | 1m 15s | trunk passed |
+1 | mvneclipse | 0m 52s | trunk passed |
+1 | findbugs | 1m 51s | trunk passed |
+1 | javadoc | 1m 1s | trunk passed |
0 | mvndep | 0m 17s | Maven dependency ordering for patch |
+1 | mvninstall | 0m 49s | the patch passed |
-1 | compile | 6m 31s | root in the patch failed. |
-1 | javac | 6m 31s | root in the patch failed. |
-0 | checkstyle | 2m 5s | root: The patch generated 2 new + 233 unchanged - 45 fixed = 235 total (was 278) |
+1 | mvnsite | 1m 19s | the patch passed |
+1 | mvneclipse | 0m 56s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 2m 12s | the patch passed |
+1 | javadoc | 1m 6s | the patch passed |
-1 | unit | 40m 4s | hadoop-yarn-server-resourcemanager in the patch failed. |
+1 | unit | 1m 9s | hadoop-sls in the patch passed. |
-1 | asflicense | 0m 48s | The patch generated 43 ASF License warnings. |
109m 19s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.yarn.server.resourcemanager.TestRMRestart |
This message was automatically generated.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 19s | 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 | 1m 59s | Maven dependency ordering for branch |
+1 | mvninstall | 13m 51s | trunk passed |
+1 | compile | 11m 24s | trunk passed |
+1 | checkstyle | 2m 11s | trunk passed |
+1 | mvnsite | 1m 15s | trunk passed |
+1 | mvneclipse | 0m 50s | trunk passed |
+1 | findbugs | 2m 6s | trunk passed |
+1 | javadoc | 1m 1s | trunk passed |
0 | mvndep | 0m 18s | Maven dependency ordering for patch |
+1 | mvninstall | 0m 50s | the patch passed |
+1 | compile | 11m 52s | the patch passed |
+1 | javac | 11m 52s | the patch passed |
-0 | checkstyle | 2m 11s | root: The patch generated 2 new + 233 unchanged - 45 fixed = 235 total (was 278) |
+1 | mvnsite | 1m 23s | the patch passed |
+1 | mvneclipse | 1m 1s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 2m 15s | the patch passed |
+1 | javadoc | 1m 9s | the patch passed |
-1 | unit | 40m 26s | hadoop-yarn-server-resourcemanager in the patch failed. |
+1 | unit | 1m 10s | hadoop-sls in the patch passed. |
-1 | asflicense | 0m 52s | The patch generated 43 ASF License warnings. |
123m 15s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.yarn.server.resourcemanager.TestRMRestart |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12858640/YARN-6326.003.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 2f90e04ada7d 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / bb6a214 |
Default Java | 1.8.0_121 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/15290/artifact/patchprocess/diff-checkstyle-root.txt |
unit | https://builds.apache.org/job/PreCommit-YARN-Build/15290/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/15290/testReport/ |
asflicense | https://builds.apache.org/job/PreCommit-YARN-Build/15290/artifact/patchprocess/patch-asflicense-problems.txt |
modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: . |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/15290/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Fixed one style issue. Added methods to delete metric output files once unit test is done.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 19s | 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 15s | Maven dependency ordering for branch |
+1 | mvninstall | 12m 49s | trunk passed |
+1 | compile | 11m 4s | trunk passed |
+1 | checkstyle | 2m 3s | trunk passed |
+1 | mvnsite | 1m 13s | trunk passed |
+1 | mvneclipse | 0m 52s | trunk passed |
+1 | findbugs | 1m 55s | trunk passed |
+1 | javadoc | 1m 0s | trunk passed |
0 | mvndep | 0m 16s | Maven dependency ordering for patch |
+1 | mvninstall | 0m 53s | the patch passed |
+1 | compile | 10m 30s | the patch passed |
+1 | javac | 10m 30s | the patch passed |
-0 | checkstyle | 2m 8s | root: The patch generated 1 new + 233 unchanged - 45 fixed = 234 total (was 278) |
+1 | mvnsite | 1m 22s | the patch passed |
+1 | mvneclipse | 0m 56s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 2m 27s | the patch passed |
+1 | javadoc | 1m 2s | the patch passed |
-1 | unit | 41m 7s | hadoop-yarn-server-resourcemanager in the patch failed. |
+1 | unit | 1m 16s | hadoop-sls in the patch passed. |
+1 | asflicense | 0m 51s | The patch does not generate ASF License warnings. |
118m 48s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.yarn.server.resourcemanager.TestRMRestart |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12859155/YARN-6326.004.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux d16fb8a89b94 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 09ad8ef |
Default Java | 1.8.0_121 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/15304/artifact/patchprocess/diff-checkstyle-root.txt |
unit | https://builds.apache.org/job/PreCommit-YARN-Build/15304/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/15304/testReport/ |
modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager hadoop-tools/hadoop-sls U: . |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/15304/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Two things:
- I like the way you made the metrics enums, but instead of
for (Metric metric: Metric.values()) { appTrackedMetrics.add(metric.value + ".memory"); appTrackedMetrics.add(metric.value + ".vcores"); } for (Metric metric: Metric.values()) { queueTrackedMetrics.add(metric.value + ".memory"); queueTrackedMetrics.add(metric.value + ".vcores"); }
we can just do
for (Metric metric : Metric.values()) { appTrackedMetrics.add(metric.value + ".memory"); appTrackedMetrics.add(metric.value + ".vcores"); queueTrackedMetrics.add(metric.value + ".memory"); queueTrackedMetrics.add(metric.value + ".vcores"); }
- I'm not sure if we should add getSchedulerApplication to YarnScheduler. Putting it in ResourceScheduler looks like it might be enough. In any case, instead of @LimitedPrivate("yarn"), you can just do @Private.
Thanks rkanter for the review. Uploaded patch v5 for your comments.
- Fixed
- I don't think it is a good idea to add a method into both interface after offline discussions. I am not fan of current design of YarnScheduler and ResourceScheduerl. But I realize that we'd better fix it in another Jira if there is any issue in them. Comparing to incompatibility, downcast is not so terrible. So I downcast ResourceScheduler to AbstractYarnScheduler to get the method I need.
+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 | 12m 28s | trunk passed |
+1 | compile | 0m 17s | trunk passed |
+1 | checkstyle | 0m 15s | trunk passed |
+1 | mvnsite | 0m 18s | trunk passed |
+1 | mvneclipse | 0m 13s | trunk passed |
+1 | findbugs | 0m 26s | trunk passed |
+1 | javadoc | 0m 13s | trunk passed |
+1 | mvninstall | 0m 16s | the patch passed |
+1 | compile | 0m 14s | the patch passed |
+1 | javac | 0m 14s | the patch passed |
-0 | checkstyle | 0m 13s | hadoop-tools/hadoop-sls: The patch generated 1 new + 187 unchanged - 45 fixed = 188 total (was 232) |
+1 | mvnsite | 0m 16s | the patch passed |
+1 | mvneclipse | 0m 11s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 0m 35s | the patch passed |
+1 | javadoc | 0m 12s | the patch passed |
+1 | unit | 0m 54s | hadoop-sls in the patch passed. |
+1 | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
18m 58s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12859413/YARN-6326.005.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 8a6edced411f 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / ffa160d |
Default Java | 1.8.0_121 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/15326/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-sls.txt |
Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/15326/testReport/ |
modules | C: hadoop-tools/hadoop-sls U: hadoop-tools/hadoop-sls |
Console output | https://builds.apache.org/job/PreCommit-YARN-Build/15326/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
I've pushed it to trunk, but this doesn't apply cleanly to branch-2. Can you upload a branch-2 version of the patch?
SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11439 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11439/)
YARN-6326. Shouldn't use AppAttemptIds to fetch applications while AM (rkanter: rev cc938e99ec0904824c8072184eff75619fcaf040)
- (edit) hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/scheduler/FairSchedulerMetrics.java
- (edit) hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/scheduler/SchedulerWrapper.java
- (edit) hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/scheduler/ResourceSchedulerWrapper.java
- (edit) hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/scheduler/SchedulerMetrics.java
- (edit) hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/appmaster/AMSimulator.java
- (edit) hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/scheduler/SLSCapacityScheduler.java
- (edit) hadoop-tools/hadoop-sls/src/test/java/org/apache/hadoop/yarn/sls/appmaster/TestAMSimulator.java
Looks like we don't need to put this on branch-2 since it based on YARN-1471 which is only available in Hadoop 3. Thanks rkanter for the review and commit.
YARN-6326This message was automatically generated.