[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17271331#comment-17271331
]
Gergely Pollak commented on YARN-10425:
---
[~ahussein]Thank you for the feedback, I've created the followup Jira
YARN-10597 . You are right, this shouldn't instantiated by the
{{CSMappingPlacementRule}} class, because it will break the cache.
The oversight was when I checked it, I thought the legacy code worked this way,
and that's why I kept it here. However now I've double checked, and I realized,
I must had checked an older branch last time, since it was fixed about a year
ago. So thank you for pointing this out. We'll need to update a few tests, but
I'll take a look at them.
> Replace the legacy placement engine in CS with the new one
> --
>
> Key: YARN-10425
> URL: https://issues.apache.org/jira/browse/YARN-10425
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Fix For: 3.4.0
>
> Attachments: YARN-10425.001.patch, YARN-10425.002.patch,
> YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch,
> YARN-10425.006.patch, YARN-10425.007.patch
>
>
> Remove the UserGroupMapping and ApplicationName mapping classes, and use the
> new CSMappingPlacementRule instead. Also cleanup the orphan classes which are
> used by these classes only.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17270425#comment-17270425
]
Ahmed Hussein commented on YARN-10425:
--
[~shuzirra], [~pbacsko] thanks for the contribution.
I have a question about the changes introduced by this Ticket. The following
code block
is from
[CSMappingPlacementRule#L128|https://github.com/apache/hadoop/commit/567600fd80896c1c9b0db1f228368d4eb2a694a2#diff-92b5797cf7739d330364d967172e65e61a859c776d9ebe526aba03ea33039033R127]
{code:java}
if (groups == null) {
//We cannot use Groups#getUserToGroupsMappingService here, because when
//tests change the HADOOP_SECURITY_GROUP_MAPPING, Groups won't refresh its
//cached instance of groups, so we might get a Group instance which
//ignores the HADOOP_SECURITY_GROUP_MAPPING settings.
groups = new Groups(conf);
}
{code}
IIUC, the design of groups caching "{{Groups.cache}}" relies on the fact that
the Groups being a singleton. Otherwise, there will be inconsistent behavior
especially in classes like {{JniBasedUnixGroupsNetgroupMapping}} and
{{ShellBasedUnixGroupsNetgroupMapping}}. Both mapping implementations have a
second caching layer for the netgroups "{{NetgroupCache}}".
I have the following two concerns regarding an independent Groups instance in
{{CSMappingPlacementRule.java}}
* It breaks the design leading to inconsistent behaviors that do not match the
expected. As I mentioned, {{NetgroupCache}} contents won't be defined.
* Performance considerations. Allocating "N" instances of {{Groups}} means
fetching the user's groups "N" times. Therefore, Guava cacheLoader's refresh
will be done "N" times, and so on.
Why did you decide to make that change instead of fixing the design of the unit
tests?
IIUC, there is a need to fix that bug in a follow up Jira.
> Replace the legacy placement engine in CS with the new one
> --
>
> Key: YARN-10425
> URL: https://issues.apache.org/jira/browse/YARN-10425
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Fix For: 3.4.0
>
> Attachments: YARN-10425.001.patch, YARN-10425.002.patch,
> YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch,
> YARN-10425.006.patch, YARN-10425.007.patch
>
>
> Remove the UserGroupMapping and ApplicationName mapping classes, and use the
> new CSMappingPlacementRule instead. Also cleanup the orphan classes which are
> used by these classes only.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17240788#comment-17240788 ] Gergely Pollak commented on YARN-10425: --- [~aajisaka] thank you for spotting this, I've left a comment on the PR. > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Fix For: 3.4.0 > > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch, > YARN-10425.006.patch, YARN-10425.007.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17240473#comment-17240473 ] Akira Ajisaka commented on YARN-10425: -- Hi [~pbacsko] and [~shuzirra], TestRouterWebServicesREST is failing after this patch is committed. Would you check https://github.com/apache/hadoop/pull/2490 ? > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Fix For: 3.4.0 > > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch, > YARN-10425.006.patch, YARN-10425.007.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17230058#comment-17230058 ] Peter Bacsko commented on YARN-10425: - Thanks [~shuzirra], committed to trunk! Thanks [~snemeth], [~BilwaST] for the review. > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch, > YARN-10425.006.patch, YARN-10425.007.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17230047#comment-17230047 ] Peter Bacsko commented on YARN-10425: - Thanks [~shuzirra] the patch LGTM. +1. [~snemeth] do you have anything to add? > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch, > YARN-10425.006.patch, YARN-10425.007.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17229548#comment-17229548
]
Hadoop QA commented on YARN-10425:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Logfile || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
46s{color} | | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} || ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
0s{color} | | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | | {color:green} The patch does not contain any @author tags.
{color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | | {color:green} The patch appears to include 9 new or modified
test files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 22m
22s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
59s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.9+11-Ubuntu-0ubuntu1.18.04.1 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
51s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_272-8u272-b10-0ubuntu1~18.04-b10 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
37s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
53s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
18m 11s{color} | | {color:green} branch has no errors when building and
testing our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
38s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.9+11-Ubuntu-0ubuntu1.18.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
34s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_272-8u272-b10-0ubuntu1~18.04-b10 {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 1m
48s{color} | | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
46s{color} | | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
52s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
52s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.9+11-Ubuntu-0ubuntu1.18.04.1 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
52s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
44s{color} | | {color:green} the patch passed with JDK Private
Build-1.8.0_272-8u272-b10-0ubuntu1~18.04-b10 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
44s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} blanks {color} | {color:green} 0m
0s{color} | | {color:green} The patch has no blanks issues. {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
31s{color} | | {color:green}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
The patch generated 0 new + 300 unchanged - 9 fixed = 300 total (was 309)
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
48s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
18m 24s{color} | | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
36s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.9+11-Ubuntu-0ubuntu1.18.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
32s{color} | | {color:green} the patch passed with JDK Private
Build-1.8.0_272-8u272-b10-0ubuntu1~18.04-b10 {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
51s{color} | | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} || ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 93m 57s{color}
|
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17229321#comment-17229321
]
Hadoop QA commented on YARN-10425:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Logfile || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
46s{color} | | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} || ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
0s{color} | | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | | {color:green} The patch does not contain any @author tags.
{color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | | {color:green} The patch appears to include 9 new or modified
test files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 22m
56s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
0s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.9+11-Ubuntu-0ubuntu1.18.04.1 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
49s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_272-8u272-b10-0ubuntu1~18.04-b10 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
38s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
53s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
17m 46s{color} | | {color:green} branch has no errors when building and
testing our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
38s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.9+11-Ubuntu-0ubuntu1.18.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
34s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_272-8u272-b10-0ubuntu1~18.04-b10 {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 1m
49s{color} | | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
46s{color} | | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
49s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
53s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.9+11-Ubuntu-0ubuntu1.18.04.1 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
52s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
44s{color} | | {color:green} the patch passed with JDK Private
Build-1.8.0_272-8u272-b10-0ubuntu1~18.04-b10 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
44s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} blanks {color} | {color:green} 0m
0s{color} | | {color:green} The patch has no blanks issues. {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 32s{color} |
[/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt|https://ci-hadoop.apache.org/job/PreCommit-YARN-Build/288/artifact/out/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt]
| {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
The patch generated 7 new + 301 unchanged - 8 fixed = 308 total (was 309)
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
47s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
18m 0s{color} | | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
37s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.9+11-Ubuntu-0ubuntu1.18.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
32s{color} | | {color:green} the patch passed with JDK Private
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17229220#comment-17229220 ] Peter Bacsko commented on YARN-10425: - BTW have you created the Jira you mentioned in response #1? Would be good perhaps to link to this one? > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch, > YARN-10425.006.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17229219#comment-17229219 ] Peter Bacsko commented on YARN-10425: - "ret represents return value, I think it's quite expressive, since the function's name and description already define what kind of data we store there it's more important to let the reader know, we are setting the return value" Did some research on this before posting, turns out there are people who indeed use "ret". I'd still be more comfortable with a name like "result", though. "_There are only two hard things in Computer Science: cache invalidation and_ _naming things._*_”_* > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch, > YARN-10425.006.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17229206#comment-17229206 ] Gergely Pollak commented on YARN-10425: --- [~pbacsko] thank you for the feedback. 1) Added some explanation, it is still quite confusing unfortunately, but I'll try to fix the whole recovery path in an other jira, since there are conceptional problems there due to legacy reasons. 2) I intentionally did not merge those conditions, since it's already quite confusing, also I find it more clear to have a dedicated recovery branch, and the other condition is only a step on that branch (currently the only one) 3) ret represents return value, I think it's quite expressive, since the function's name and description already define what kind of data we store there it's more important to let the reader know, we are setting the return value, not just some random applicationPlacementContext, which might get returned later. Using ret explicitly lets us know, we are changing the return value. 4) Checkstyle warnings lost, so uploaded a patch to regenerate them, fixing those in the next iteration. > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch, > YARN-10425.006.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17225305#comment-17225305
]
Peter Bacsko commented on YARN-10425:
-
Thanks [~shuzirra] for the update. I have a question regarding this:
{noformat}
if (ret == null) {
//If no rule was applied we return null, to let the engine move
onto the
//next placementRule class
LOG.info("No matching rule found for application '{}', moving to
next " +
"PlacementRule engine", asc.getApplicationName());
}
if (recovery) {
if (ret == null || !ret.getQueue().equals(asc.getQueue())) {
return null;
}
}
{noformat}
1. The "recovery" path looks a bit non-straightforward. Could you add some
comments there?
2. You can merge the two ifs:
{noformat}
if (recovery && (ret == null || !ret.getQueue().equals(asc.getQueue())) { ... }
{noformat}
3. Could you rename "ret" to something else? It's not expressive enough.
4. There some checkstyle warnings, pls fix those as well.
> Replace the legacy placement engine in CS with the new one
> --
>
> Key: YARN-10425
> URL: https://issues.apache.org/jira/browse/YARN-10425
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10425.001.patch, YARN-10425.002.patch,
> YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch
>
>
> Remove the UserGroupMapping and ApplicationName mapping classes, and use the
> new CSMappingPlacementRule instead. Also cleanup the orphan classes which are
> used by these classes only.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17223833#comment-17223833
]
Hadoop QA commented on YARN-10425:
--
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Logfile || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m
22s{color} | | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} || ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
0s{color} | | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | | {color:green} The patch does not contain any @author tags.
{color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | | {color:green} The patch appears to include 9 new or modified
test files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 23m
54s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
1s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
53s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
38s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
53s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
18m 58s{color} | | {color:green} branch has no errors when building and
testing our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
38s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
37s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 1m
55s{color} | | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
53s{color} | | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
51s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
57s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
57s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
47s{color} | | {color:green} the patch passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
47s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} blanks {color} | {color:green} 0m
0s{color} | | {color:green} The patch has no blanks issues. {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 34s{color} |
[/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt|https://ci-hadoop.apache.org/job/PreCommit-YARN-Build/276/artifact/out/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt]
| {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
The patch generated 7 new + 301 unchanged - 8 fixed = 308 total (was 309)
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
52s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
18m 23s{color} | | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
37s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
32s{color} | | {color:green} the patch passed with JDK Private
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17223708#comment-17223708 ] Peter Bacsko commented on YARN-10425: - Thanks for the explanation [~shuzirra], I'll do another round on Monday, I guess it will be OK and we can commit to trunk. > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17223701#comment-17223701 ] Gergely Pollak commented on YARN-10425: --- Thank you for the review [~pbacsko], [~BilwaST] and [~snemeth], I've uploaded the newest patch with the asked changes, please find my answers below: [~pbacsko] 1. Nits: instead of directly using the constructor new Groups(conf), you might want to use Groups.getUserToGroupsMappingServiceWithLoadedConfiguration() Actually I cannot, because some tests fail, if I don't create a new instance each time, since the Groups class caches the value of HADOOP_SECURITY_GROUP_MAPPING, which is changed by multiple tests. I tried to explain it in the comment above the line 2. I think the condition below should not be allowed. If, for whatever reason we couldn't retrieve the groups service provider, that is a serious error and we shouldn't proceed any further. What is the rationale behind this change? 3. Same here, don't catch this if we know that the error is group-related: Groups errors are not fatal, we can proceed if a group cannot be determined for a user, the rationale behind adding this less strict version is, we evaluate groups early, even if there is no group related rule, or the actual app submission would not hit any group related rule. If no group is found, placement will fail, for group related placements anyway, which will get to the REJECT for legacy rules, so for legacy we have the same behavior, new format users can determine if they want to fail on group errors. Actually if I threw an exception at that point, we's loose backwards compatibility, as multiple tests pointed out. [~BilwaST] I'm relying on the interface specification, it does not specify I cannot receive null at this point, os I check for it, better safe than sorry. Receiving null is no reason to fail here, since we can handle it properly, so I feel safer if I check for it. (Also please note that some tests mock the hell out of the RM engines, and you might get nulls at surprising places.) > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17223045#comment-17223045
]
Hadoop QA commented on YARN-10425:
--
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Logfile || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
42s{color} | | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} || ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
0s{color} | | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | | {color:green} The patch does not contain any @author tags.
{color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | | {color:green} The patch appears to include 9 new or modified
test files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
34s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
0s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
51s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
40s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
55s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
16m 7s{color} | | {color:green} branch has no errors when building and
testing our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
41s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
39s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 1m
44s{color} | | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
42s{color} | | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
52s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
51s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
51s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
44s{color} | | {color:green} the patch passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
44s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} blanks {color} | {color:green} 0m
0s{color} | | {color:green} The patch has no blanks issues. {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 33s{color} |
[/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt|https://ci-hadoop.apache.org/job/PreCommit-YARN-Build/270/artifact/out/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt]
| {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
The patch generated 7 new + 301 unchanged - 8 fixed = 308 total (was 309)
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
48s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
16m 17s{color} | | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
38s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
36s{color} | | {color:green} the patch passed with JDK Private
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17222918#comment-17222918 ] Gergely Pollak commented on YARN-10425: --- Patch#4 is about bugfixes again, this time probably all of them are fixed and can move onto the actual review feedbacks. > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch, YARN-10425.004.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17222391#comment-17222391
]
Hadoop QA commented on YARN-10425:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Logfile || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
43s{color} | | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} || ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
1s{color} | | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | | {color:green} The patch does not contain any @author tags.
{color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | | {color:green} The patch appears to include 7 new or modified
test files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
49s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
0s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
51s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
39s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
57s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
16m 29s{color} | | {color:green} branch has no errors when building and
testing our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
39s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
39s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 1m
46s{color} | | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m
44s{color} | | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
52s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
57s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
57s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
47s{color} | | {color:green} the patch passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
47s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} blanks {color} | {color:green} 0m
0s{color} | | {color:green} The patch has no blanks issues. {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 30s{color} |
[/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt|https://ci-hadoop.apache.org/job/PreCommit-YARN-Build/265/artifact/out/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt]
| {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
The patch generated 7 new + 241 unchanged - 8 fixed = 248 total (was 249)
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
48s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
16m 22s{color} | | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
39s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
36s{color} | | {color:green} the patch passed with JDK Private
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1777#comment-1777 ] Gergely Pollak commented on YARN-10425: --- Patch#3 is a rebased version and addressing a major issue during recovery, but I need to see if it broke anything else. Review comment related fixes are expected when I've dealt with the bug. [~wangda] thank you, yes backwards compatibility is of utmost importance, there might be some slight differences in the error handling, due to the inconsistencies of the legacy solution, but we also provide a way to define on a per rule basis what should happen in the case of an error (error when the placement rule cannot be executed eg. invalid target path). > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch, > YARN-10425.003.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17221619#comment-17221619 ] Wangda Tan commented on YARN-10425: --- Thanks [~shuzirra], this is a really big and important effort :). I don't think I will have a chance to review the code, just one ask: let's make sure the behavior is backward compatible, and let's keep the original test cases in place (I remember we have a bunch of them). > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17221489#comment-17221489
]
Hadoop QA commented on YARN-10425:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Logfile || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
0s{color} | | {color:blue} Docker mode activated. {color} |
| {color:red}-1{color} | {color:red} patch {color} | {color:red} 0m 10s{color}
| | {color:red} YARN-10425 does not apply to trunk. Rebase required? Wrong
Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. {color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | YARN-10425 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/13014204/YARN-10425.002.patch |
| Console output |
https://ci-hadoop.apache.org/job/PreCommit-YARN-Build/254/console |
| versions | git=2.17.1 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
> Replace the legacy placement engine in CS with the new one
> --
>
> Key: YARN-10425
> URL: https://issues.apache.org/jira/browse/YARN-10425
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10425.001.patch, YARN-10425.002.patch
>
>
> Remove the UserGroupMapping and ApplicationName mapping classes, and use the
> new CSMappingPlacementRule instead. Also cleanup the orphan classes which are
> used by these classes only.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[ https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17221487#comment-17221487 ] Gergely Pollak commented on YARN-10425: --- Reuploading patch#1 as patch#2 as the test results have been lost, and did not manage to sort all of the out. > Replace the legacy placement engine in CS with the new one > -- > > Key: YARN-10425 > URL: https://issues.apache.org/jira/browse/YARN-10425 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: Gergely Pollak >Assignee: Gergely Pollak >Priority: Major > Attachments: YARN-10425.001.patch, YARN-10425.002.patch > > > Remove the UserGroupMapping and ApplicationName mapping classes, and use the > new CSMappingPlacementRule instead. Also cleanup the orphan classes which are > used by these classes only. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17213135#comment-17213135
]
Szilard Nemeth commented on YARN-10425:
---
Hi [~shuzirra],
Thanks for working on this.
Some comments on top of the other ones you have already got:
1. Nit: For the comment added in CSMappingPlacementRule#initialize:
Can you change it to this?
{code:java}
//We cannot use Groups#getUserToGroupsMappingService here, because when tests
//change the HADOOP_SECURITY_GROUP_MAPPING, Groups won't refresh its
//cached instance of groups, so we might get a Group instance which
//ignores the HADOOP_SECURITY_GROUP_MAPPING settings.
{code}
2. In the same method as 1, you may fix the LOG string to something like:
{code:java}
LOG.info("Initialized queue mappings. Can override user specified " +
"queues: {}, number of rules: {}, mapping rules: {}", overrideWithQueueMappings,
mappingRules.size(), mappingRules);
{code}
3. Nit: If you are uploading a new patch already, can you fix this one as well?
In validateDynamicQueuePath, there's a typo in the bigger comment at the
bottom.
accessibe -> accessible
4. This is just thanking you that you named the group name constants at the top
of the TestCapacitySchedulerAutoCreatedQueueBase class correctly.
5. In TestCapacitySchedulerQueueMappingFactory, you could replace the string
pointing to the fully qualified name of the CSMappingPLacementRule class with:
{code:java}
private static final String QUEUE_MAPPING_RULE =
CSMappingPlacementRule.class.getCanonicalName();
{code}
6. In
TestCapacitySchedulerQueueMappingFactory#testFixedUserWithDynamicGroupQueue:
The last queue mapping (called userQueueMapping3) you have this as queue:
"c.%secondary_group", but the comment above the variable doesn't seem to
reflect this, it misses the "c."
> Replace the legacy placement engine in CS with the new one
> --
>
> Key: YARN-10425
> URL: https://issues.apache.org/jira/browse/YARN-10425
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10425.001.patch
>
>
> Remove the UserGroupMapping and ApplicationName mapping classes, and use the
> new CSMappingPlacementRule instead. Also cleanup the orphan classes which are
> used by these classes only.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17210927#comment-17210927
]
Bilwa S T commented on YARN-10425:
--
Thanks [~shuzirra] for patch. I have minor nit
1. Why is null check needed in below code? Null can be returned only in case of
LeafQueue.getChildQueues() but here its parentQueue and
ParentQueue.getChildQueues() never return null.
{code:java}
if (parentQueue.getChildQueues() != null) {
for (CSQueue queue : parentQueue.getChildQueues()) {
if (queue instanceof LeafQueue) {
//if a non managed parent queue has at least one leaf queue,
this
/mapping can be valid, we cannot do any more checks
return true;
}
{code}
> Replace the legacy placement engine in CS with the new one
> --
>
> Key: YARN-10425
> URL: https://issues.apache.org/jira/browse/YARN-10425
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10425.001.patch
>
>
> Remove the UserGroupMapping and ApplicationName mapping classes, and use the
> new CSMappingPlacementRule instead. Also cleanup the orphan classes which are
> used by these classes only.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17210822#comment-17210822
]
Peter Bacsko commented on YARN-10425:
-
Thanks for the patch [~shuzirra]. I mostly have minor comments & questions:
1. Nits:
* instead of directly using the constructor {{new Groups(conf)}}, you might
want to use {{Groups.getUserToGroupsMappingServiceWithLoadedConfiguration()}}
* the comment "Groups won't refresh it's" --> "its"
2. I think the condition below should not be allowed. If, for whatever reason
we couldn't retrieve the groups service provider, that is a serious error and
we shouldn't proceed any further.
{noformat}
if (groups == null) {
LOG.warn(
"Group provider hasn't been set, cannot query groups for user
{}",
user);
return;
}
{noformat}
What is the rationale behind this change?
3. Same here, don't catch this if we know that the error is group-related:
{noformat}
try {
setupGroupsForVariableContext(vctx, user);
} catch (IOException e) {
LOG.warn("Unable to setup groups: {}", e.getMessage());
}
{noformat}
4. CapacityScheduler.java: don't use "*" imports
5. TestUserGroupMappingPlacementRule.java: the test {{testNullGroupMapping()}}
no longer throws exception because of #2.
6. TestQueueMappings.java: don't use "*" imports
> Replace the legacy placement engine in CS with the new one
> --
>
> Key: YARN-10425
> URL: https://issues.apache.org/jira/browse/YARN-10425
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10425.001.patch
>
>
> Remove the UserGroupMapping and ApplicationName mapping classes, and use the
> new CSMappingPlacementRule instead. Also cleanup the orphan classes which are
> used by these classes only.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10425) Replace the legacy placement engine in CS with the new one
[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17210233#comment-17210233
]
Hadoop QA commented on YARN-10425:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Logfile || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m
16s{color} | | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} || ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
0s{color} | | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | | {color:green} The patch does not contain any @author tags.
{color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | | {color:green} The patch appears to include 7 new or modified
test files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 22m
25s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
1s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
48s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
40s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m
0s{color} | | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
17m 27s{color} | | {color:green} branch has no errors when building and
testing our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
46s{color} | | {color:green} trunk passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
38s{color} | | {color:green} trunk passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 2m
13s{color} | | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m
11s{color} | | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} || ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
1s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m
4s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m
4s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
52s{color} | | {color:green} the patch passed with JDK Private
Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
52s{color} | | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 36s{color} |
[/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt|https://ci-hadoop.apache.org/job/PreCommit-YARN-Build/220/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt]
| {color:orange}
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
The patch generated 4 new + 213 unchanged - 8 fixed = 217 total (was 221)
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
57s{color} | | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
17m 40s{color} | | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
35s{color} | | {color:green} the patch passed with JDK
Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
33s{color} | | {color:green} the patch passed with JDK Private
