Details
-
Type:
Sub-task
-
Status: Resolved
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: 2.9.0, 3.0.0-alpha2
-
Component/s: resourcemanager
-
Labels:None
-
Target Version/s:
Description
This proposes to fix the the OpportunisticAllocator, used to allocate OPPORTUNISTIC containers (both Centrally as well as in a Distributed manner) to correctly populate the Node Http Address in the returned Container.
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Uploading initial patch..
Subru Krishnan, Can you take a quick look ?
 -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 appears to include 2 new or modified test files. |
| 0 | mvndep | 0m 8s | Maven dependency ordering for branch |
| +1 | mvninstall | 7m 2s | trunk passed |
| +1 | compile | 1m 34s | trunk passed |
| +1 | checkstyle | 0m 31s | trunk passed |
| +1 | mvnsite | 1m 28s | trunk passed |
| +1 | mvneclipse | 0m 44s | trunk passed |
| +1 | findbugs | 2m 23s | trunk passed |
| +1 | javadoc | 0m 54s | trunk passed |
| 0 | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 | mvninstall | 1m 19s | the patch passed |
| +1 | compile | 1m 39s | the patch passed |
| +1 | cc | 1m 39s | the patch passed |
| +1 | javac | 1m 39s | the patch passed |
| -0 | checkstyle | 0m 29s | hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 14 new + 56 unchanged - 2 fixed = 70 total (was 58) |
| +1 | mvnsite | 1m 29s | the patch passed |
| +1 | mvneclipse | 1m 20s | the patch passed |
| +1 | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 | findbugs | 2m 36s | the patch passed |
| -1 | javadoc | 0m 14s | hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common generated 1 new + 159 unchanged - 0 fixed = 160 total (was 159) |
| -1 | javadoc | 0m 16s | hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager generated 2 new + 235 unchanged - 0 fixed = 237 total (was 235) |
| -1 | javadoc | 0m 19s | hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 5 new + 938 unchanged - 0 fixed = 943 total (was 938) |
| +1 | unit | 0m 26s | hadoop-yarn-server-common in the patch passed. |
| -1 | unit | 0m 15s | hadoop-yarn-server-nodemanager in the patch failed. |
| -1 | unit | 0m 20s | hadoop-yarn-server-resourcemanager in the patch failed. |
| +1 | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| 31m 49s |
This message was automatically generated.
The patch LGTM. +1 (pending Yetus).
I have a couple of minor comments:
- We should move convertTo/FromProtoFormat to RemoteNodePBImpl instead of multiple places, in DistributedSchedulingAllocateResponsePBImpl & RegisterDistributedSchedulingAMResponsePBImpl.
- Please add visibility annotations in RemoteNodePBImpl and for getters/setters in RemoteNode.
| -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 2 new or modified test files. |
| 0 | mvndep | 0m 34s | Maven dependency ordering for branch |
| +1 | mvninstall | 7m 9s | trunk passed |
| +1 | compile | 1m 34s | trunk passed |
| +1 | checkstyle | 0m 32s | trunk passed |
| +1 | mvnsite | 1m 31s | trunk passed |
| +1 | mvneclipse | 0m 44s | trunk passed |
| +1 | findbugs | 2m 22s | trunk passed |
| +1 | javadoc | 0m 56s | trunk passed |
| 0 | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 | mvninstall | 1m 15s | the patch passed |
| +1 | compile | 1m 30s | the patch passed |
| +1 | cc | 1m 30s | the patch passed |
| +1 | javac | 1m 30s | the patch passed |
| -0 | checkstyle | 0m 29s | hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 13 new + 56 unchanged - 2 fixed = 69 total (was 58) |
| +1 | mvnsite | 1m 22s | the patch passed |
| +1 | mvneclipse | 0m 39s | the patch passed |
| +1 | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 | findbugs | 2m 38s | the patch passed |
| -1 | javadoc | 0m 14s | hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common generated 1 new + 159 unchanged - 0 fixed = 160 total (was 159) |
| +1 | unit | 0m 26s | hadoop-yarn-server-common in the patch passed. |
| +1 | unit | 14m 45s | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 | unit | 35m 39s | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 | asflicense | 0m 19s | The patch does not generate ASF License warnings. |
| 81m 3s |
| Subsystem | Report/Notes |
|---|---|
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Issue | |
| JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12835913/YARN-5799.001.patch |
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc |
| uname | Linux 46a516cad896 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 / ebb8823 |
| Default Java | 1.8.0_101 |
| findbugs | v3.0.0 |
| checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/13657/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server.txt |
| javadoc | https://builds.apache.org/job/PreCommit-YARN-Build/13657/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.txt |
| Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/13657/testReport/ |
| modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server |
| Console output | https://builds.apache.org/job/PreCommit-YARN-Build/13657/console |
| Powered by | Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Thanks for the review Subru Krishnan. Updated the patch based on your suggestions.
Will check it in once Jenkins comes clean (based on your +1).
 +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 2 new or modified test files. |
| 0 | mvndep | 0m 51s | Maven dependency ordering for branch |
| +1 | mvninstall | 6m 48s | trunk passed |
| +1 | compile | 1m 32s | trunk passed |
| +1 | checkstyle | 0m 30s | trunk passed |
| +1 | mvnsite | 1m 26s | trunk passed |
| +1 | mvneclipse | 0m 43s | trunk passed |
| +1 | findbugs | 2m 10s | trunk passed |
| +1 | javadoc | 0m 53s | trunk passed |
| 0 | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 | mvninstall | 1m 11s | the patch passed |
| +1 | compile | 1m 28s | the patch passed |
| +1 | cc | 1m 28s | the patch passed |
| +1 | javac | 1m 28s | the patch passed |
| -0 | checkstyle | 0m 29s | hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 2 new + 57 unchanged - 2 fixed = 59 total (was 59) |
| +1 | mvnsite | 1m 21s | the patch passed |
| +1 | mvneclipse | 0m 38s | the patch passed |
| +1 | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 | findbugs | 2m 32s | the patch passed |
| +1 | javadoc | 0m 48s | the patch passed |
| +1 | unit | 0m 26s | hadoop-yarn-server-common in the patch passed. |
| +1 | unit | 14m 51s | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 | unit | 38m 23s | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 | asflicense | 0m 17s | The patch does not generate ASF License warnings. |
| 83m 12s |
| Subsystem | Report/Notes |
|---|---|
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Issue | |
| JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12835939/YARN-5799.002.patch |
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc |
| uname | Linux e2dbe91ca818 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 / ebb8823 |
| Default Java | 1.8.0_101 |
| findbugs | v3.0.0 |
| checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/13660/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server.txt |
| Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/13660/testReport/ |
| modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server |
| Console output | https://builds.apache.org/job/PreCommit-YARN-Build/13660/console |
| Powered by | Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Committed this to trunk. Thanks again for the review Subru Krishnan !!
SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10728 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10728/)
YARN-5799. Fix Opportunistic Allocation to set the correct value of Node (arun suresh: rev aa3cab1eb29c56368d15882d7260a994e615e8d8)
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/proto/yarn_server_common_service_protos.proto
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/scheduler/TestDistributedScheduler.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/scheduler/OpportunisticContainerAllocator.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/impl/pb/DistributedSchedulingAllocateResponsePBImpl.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/scheduler/OpportunisticContainerContext.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/scheduler/DistributedScheduler.java
- (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/impl/pb/RemoteNodePBImpl.java
- (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/RemoteNode.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/OpportunisticContainerAllocatorAMService.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestOpportunisticContainerAllocatorAMService.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/DistributedSchedulingAllocateResponse.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/impl/pb/RegisterDistributedSchedulingAMResponsePBImpl.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/RegisterDistributedSchedulingAMResponse.java
- (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
Committing this to branch-2
Currently, the OpportunisticAllocator uses the NodeId to populate Node relevant information in the returned Container. Unfortunately, this is not sufficient, as the NodeId does not include the Http Address (which is the used in the Web UI to redirect to the Node), which is found only in the RMNode.
I propose to create a RemoteNode structure that wraps the NodeId and also includes the Http Address. Also this new structure should be placed in the yarn_server_common_service_protos.proto file, to ensure it is not a public facing API and used only in communication between NM and RM.