-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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-11041. Replace all occurences of queuePath with the new QueuePath class - followup #5332
Conversation
…h class - followup Change-Id: I2d080bb414b08c2cae6a83d8d043667464451fc9
…h class - followup Change-Id: Ifdaae6f0b3b5be0efbd841988973f3f91a67f22e
…h class - followup Change-Id: I41a3be26a916b260005677ef228f69e094240d98
Change-Id: I532c5244a411ac230664058c2e92cad1002978fc
Change-Id: Id5fc099521dcd0411e3db324fba3d5f2a0493c02
…tion Change-Id: I2275d216e15fa5ba9ee9752de749cbfc7ffac980
Change-Id: I404aa88232a5ab5c8b30c0cc663da2a2700d9e4d
💔 -1 overall
This message was automatically generated. |
Change-Id: Ic9ce9d8658bbd0fdf610e36b3f0291ac61cd0300
💔 -1 overall
This message was automatically generated. |
Change-Id: I8be239b799bd04461d8b7e4ccc04b395a7666d8d
Change-Id: I78659093ec68af98ca411cfca4bb48ba219fb323
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Change-Id: I95540b77da3983f82943000b32f8d1ede791761f
🎊 +1 overall
This message was automatically generated. |
Change-Id: I7394673085b5e1b9309c9f49f814558ec6568847
Change-Id: I9d1c61101efcce976a822b1320dd4505e38c8728
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
...e/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesCapacitySchedDynamicConfig.java
Outdated
Show resolved
Hide resolved
...he/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java
Outdated
Show resolved
Hide resolved
...he/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java
Outdated
Show resolved
Hide resolved
...test/java/org/apache/hadoop/yarn/server/router/webapp/TestableFederationInterceptorREST.java
Outdated
Show resolved
Hide resolved
...che/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/QueuePlacementConverter.java
Show resolved
Hide resolved
...hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java
Outdated
Show resolved
Hide resolved
...g/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AutoCreatedQueueTemplate.java
Show resolved
Hide resolved
...rc/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/QueuePath.java
Show resolved
Hide resolved
...ain/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/QueuePrefixes.java
Outdated
Show resolved
Hide resolved
Thanks @p-szucs for this patch, good job. |
Change-Id: I2bd30982c32386d68dc662dc268773e0d56be777
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Thank you @szilard-nemeth for your thorough review on this change. I updated it addressing all your comments and questions. May I ask you to take another look on it please, if you'll have some time for it? |
Hi @p-szucs , |
💔 -1 overall
This message was automatically generated. |
Change-Id: Ic5acdae90659b4addc3ef23601d1528b4784b9b6
💔 -1 overall
This message was automatically generated. |
Change-Id: Idf99d51c16345b74b851e9f7883bce6da33f8fa7
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Change-Id: Id93e6ff990e9dad22362f4dc330783f7128a35f8
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Thanks @p-szucs for the patch, @szilard-nemeth for the review. I had one small comment but given the size of the commit and that it's long running already I don't think it's worth it to retrigger builds and everything for that one small nit, can be done later in a follow-up commit. Merging to trunk. |
|
||
boolean isReservableQueue = conf.isReservable(fullQueueName); | ||
boolean isAutoCreateEnabled = conf.isAutoCreateChildQueueEnabled(fullQueueName); | ||
QueuePath fullQueuePath = (parent == null) ? new QueuePath(queueName) : |
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.
Nit: fullQueuePath is a bit redundant as far as naming goes, as by definition all queuepath objects contain the full queue path. Something like queueToParse would be a bit more descriptive, but retriggering the build because of a new commit only for this is probably not worth it, this easily can be done later.
@p-szucs @brumi1024 This seems to cause some issue.
|
@brumi1024 I revert this change first, this pr causing YARN to fail to compile. |
@slfan1989 thanks for checking! The issue is that the build was from December and #6433 was merged 2 weeks ago using a method with the old signature. Should be a one line change, @p-szucs can you please reopen this PR? |
Thanks @slfan1989 for catching! @brumi1024 sure, I'm reopening this PR |
…h class - followup (apache#5332)
Description of PR
JIRA ticket: https://issues.apache.org/jira/browse/YARN-11041
The changes on this PR are mostly replacing string queuePath occurences to QueuePath objects.
There are also some minor refactorings recommended by the JIRA ticket, which are the followings:
How was this patch tested?
Tested with running Hadoop locally.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?