Details
-
Sub-task
-
Status: Resolved
-
Major
-
Resolution: Won't Fix
-
None
-
None
Description
Common changes:
- PB support changes required for Admin APIs
- PB support for File System store (Priority Label Store)
Attachments
Attachments
- 0001-YARN-2896.patch
- 86 kB
- Sunil G
- 0002-YARN-2896.patch
- 97 kB
- Sunil G
- 0003-YARN-2896.patch
- 100 kB
- Sunil G
- 0004-YARN-2896.patch
- 99 kB
- Sunil G
- 0005-YARN-2896.patch
- 104 kB
- Sunil G
Activity
Uploading common PB changes as a single patch
This covers all the PB related changes in ResourceManager level. Also added tests in PBImplRecords
I will upload a prototype patch in parent JIRA, and will upload subjira patches. As per comments, this can be made into production quality.
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12691683/0003-YARN-2896.patch
against trunk revision d336d13.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. There were no new javadoc warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.
Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6331//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6331//console
This message is automatically generated.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12693123/0004-YARN-2896.patch
against trunk revision 4a5c3a4.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. There were no new javadoc warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
-1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.
Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6360//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-YARN-Build/6360//artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-api.html
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6360//console
This message is automatically generated.
Thanks, sunilg, for working on this feature and posting this patch to support PB framework.
I have a general question about why job priorities need labels. Why can't they just be number based? It seems like extra work to label them, pass the labels, and then interpret them.
PriorityLabelsPerQueue.java:
public String toString() { return "Max priority label: " + this.getMaxPriorityLabel() + " ," + "Default priority label: " + this.getDefaultPriorityLabel(); }
This is just a nit, but in PriorityLabelsPerQueue#toString, the space should be after the comma. Currently, this will output something like this:
Max priority label: foo ,Default priority label: bar
public int compareTo(PriorityLabelsPerQueue priorityLabelsPerQueue) { int defltLabelCompare = this.getDefaultPriorityLabel().compareTo( priorityLabelsPerQueue.getDefaultPriorityLabel()); if (defltLabelCompare == 0) { return this.getMaxPriorityLabel().compareTo( priorityLabelsPerQueue.getMaxPriorityLabel()); } else { return defltLabelCompare; } }
PriorityLabelsPerQueue#compareTo should probably check for NULL for priorityLabelsPerQueue, this.getDefaultPriorityLabel(), and this.getMaxPriorityLabel(). If priorityLabelsPerQueue is NULL, this.getDefaultPriorityLabel() returns NULL, or this.getMaxPriorityLabel() returns NULL, compareTo will throw NPE.
yarn_protos.proto:
message ApplicationPriorityProto { optional string app_priority = 1; }
Where is this referenced?
I have same question as epayne, beyond readability (user can know priority of an app is "high" instead of -1), is there any must-have reason to make priority to be text?
Priority setting in other systems like
Linux: http://www.ibm.com/developerworks/library/l-lpic1-v3-103-6/
Moab: https://computing.llnl.gov/tutorials/moab/#Priority
Slurm: https://computing.llnl.gov/linux/slurm/priority_multifactor.html
Are all using numbers, math operations can be applied on priorities, such as job_priority = queue_priority * user_priority.
Thanks,
Wangda
Thank you epayne and wangda for the comments
The idea of keeping Application Priority as a string is for better handling and for easiness from user perspective.
Internally RM will have a corresponding integer mapping, and only that will be used by Schedulers. Hence as wangda mentioned, it can be operated just like an integer with user priority etc.
A rough idea is like, user is submitting a job with priority as “High” and scheduler will be treating as an integer namely “3”.
Priority Label Manager will act as an interface to User and Scheduler and can give the priority as string or integer accordingly.
Now coming to the advantages, admin can operate on names or labels for priority, it will be easier. Also it can be displayed in UI very easily.
Also admin can config the priority label as per his norms along by defining corresponding integer mapping associated with each label.
For eg:
yarn.cluster.priority-labels=low:1,medium:3,high:5
Configuring ACLs based on a priority label name will be more easier.
yarn.scheduler.capacity.root.queueA.High.acl=user1,user2
Please share your thoughts.
I will address the other comments from Eric and will update a patch.
sunilg, it seems to me that labels can make things more confusing, not less, since different queues can have arbitrary names for the same concept. Also, it would eliminate the need to add an infrastructure for mapping, passing, and interpreting labels and priority numbers. YARN could always specify that priorities go from low to high, and each queue could then decide how hight to go with the priority numbers. Also, it seems to me that the following property definition could specify high priority:
yarn.scheduler.capacity.root.queueA.5.acl=user1,user2
I have similar feeling, giving a name per priority is not a scalable way, how to deal with user wants to have 100 different priorities in the cluster? If names like "priority_0".."priority_100", isn't it more clear to be a single number? People can easily compare which number is lower. If we choose number only, we can avoid a lot of complexities for PB changes and also "priority label manager" implementation.
Thank you epayne and leftnoteasy for sharing your thoughts.
It is easier in implementation with integers and directly it can be configured and operated with. On the other hand Labels makes more readable and easier for a user to submit an app and visualize same in UI. Same for admins. But as you mentioned it comes with slighter complexity at RM to be an interface to scheduler.
This thought of labels was one of initial idea where in design was formulated. As vinodkv also participated in design, its good to hear his thoughts too.
Also I surely feel that we still need PriorityLabelManager, it can help us in storing the data from various options such as config files or admin commands or even REST. Its better to take such complexities out of RMAppManager. Also for HA cases, PriorityLabelManager itself can help to bring back config loaded from memory/files. Also scheduler need not have to take any load, and PrioirtyLabelManager can help in providing priority for app and its acls.
Thanks sunilg, I agree to have a storage to simply configuration (no need to specify highest-priority for each user under each queue), all can be done via REST API or CLI. For other part, we can wait vinodkv's feedback.
sunilg, leftnoteasy, and vinodkv, can we move this discussion to YARN-1963 in order to achieve a higher visibility?
Yes. +1.
I will move this to parent JIRA for better visibility. Thank you Eric and Wangda for the comments.
Yes. +1.
I will move this to parent JIRA for better visibility. Thank you Eric and Wangda for the comments.
Yes. +1.
I will move this to parent JIRA for better visibility. Thank you Eric and Wangda for the comments.
Yes. +1.
I will move this to parent JIRA for better visibility. Thank you Eric and Wangda for the comments.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12696539/0005-YARN-2896.patch
against trunk revision 40a4157.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. There were no new javadoc warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
-1 findbugs. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.
Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6512//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-YARN-Build/6512//artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-api.html
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6512//console
This message is automatically generated.
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12696768/0005-YARN-2896.patch
against trunk revision 4e7ad4f.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. There were no new javadoc warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.
Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6519//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6519//console
This message is automatically generated.
This enhancement is not planned as of now.. So closing this as Wont Fix. If any discussions are happening towards this, we can reopen this.
Uploading an initial patch for common PB support. This patch is needed for Priority Label manager. Tests also will be added soon.