-
Notifications
You must be signed in to change notification settings - Fork 8.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
YARN-10891. Extend QueueInfo with max-parallel-apps in CS. #3314
Conversation
💔 -1 overall
This message was automatically generated. |
Probably I should have just extended the CapacitySchedulerQueueInfo for the Cluster Scheduler API (REST). marking as WIP |
🎊 +1 overall
This message was automatically generated. |
58af851
to
cf6bd2d
Compare
💔 -1 overall
This message was automatically generated. |
cf6bd2d
to
b1be624
Compare
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @tomicooler for working on this issue. Apart from some minor issue, I think the overall change is straightforward!
...ava/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CSQueue.java
Show resolved
Hide resolved
...ava/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySched.java
Outdated
Show resolved
Hide resolved
Speaking of which: Could you please update the REST API documentation?
You can also add the maxParallelsApps JSON / XML property to the example responses down there. |
💔 -1 overall
This message was automatically generated. |
Done. The example response was updated manually - only with the maxParallelApps properties - but it might be outdated, this part of the doc was not changed too much in the recent years. I just noticed that the Applications and Apps is not too coherent in the Rest API: | numApplications | int | The number of applications currently in the queue | |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Change-Id: Ie2cbe30cf1351381b7e49e58e2011998e9a4eb12
4720ac7
to
ecb67be
Compare
💔 -1 overall
This message was automatically generated. |
Change-Id: Ic2b6480de90f06f7a0583b541584e2a946669233
No additional comments from my side, +1. |
🎊 +1 overall
This message was automatically generated. |
Thanks @tomicooler for working on this. Nice patch, was easy to review. |
Co-authored-by: Tamas Domok <tdomok@cloudera.com>
apache#3314) Co-authored-by: Tamas Domok <tdomok@cloudera.com> (cherry picked from commit 16e6030) Change-Id: I71a53d9882bd68d761ac7f950dd9823b0022157b
Description of PR
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?