[jira] [Commented] (YARN-10639) Queueinfo related capacity, should ajusted to weight mode.

2021-02-24 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10639?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17290022#comment-17290022
 ] 

Gergely Pollak commented on YARN-10639:
---

While this fix fills the capacity field in the case of a weight configured 
queue, I think we should investigate if we could painlessly add a weight field 
to the queue info and fill it rather. The problem is I've check the usages of 
the capacity field of the QueueInfo, and while it is mostly used for display 
purposes, it is considered a percentage value, and parsed as such, so it might 
cause confusion or even worse it might result in bogus behaviour.

> Queueinfo related capacity, should ajusted to weight mode.
> --
>
> Key: YARN-10639
> URL: https://issues.apache.org/jira/browse/YARN-10639
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Qi Zhu
>Assignee: Qi Zhu
>Priority: Major
> Attachments: YARN-10639.001.patch, YARN-10639.002.patch
>
>
> {color:#172b4d}The class QueueInfo capacity field should consider the weight 
> mode.{color}
> {color:#172b4d}Now when client use getQueueInfo to get queue capacity in 
> weight mode, i always return 0, it is wrong.{color}



--
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-10623) Capacity scheduler should support refresh queue automatically by a thread policy.

2021-02-24 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17290013#comment-17290013
 ] 

Gergely Pollak commented on YARN-10623:
---

[~zhuqi] thank you for the patch!

I really like the idea that the solution is a configurable extra component 
(scheduling edit policy), this way it is cleanly decoupled from the main code 
base, won't cause any issue for those who don't want to use it. 

I very much agree with [~gandras] that last modification check was necessary 
however I think it introduced a bug:

 
{code:java}
126if (lastModified > lastSuccessfulReload &&
127time > lastModified + monitoringInterval) {
{code}
So in the edge case when you update this file MORE frequently than the 
monitoringInterval (which is configurable, so it might be in the minute or even 
10 minutes range), then you won't ever refresh the config, since last modified 
+ monitoringInterval will be ALWAYS greater than the current time. I think you 
should go with

 
{code:java}
126 if (lastModified > lastSuccessfulReload &&
127   time > lastSuccessfulReload + monitoringInterval) {
{code}
Or even better introduce a lastReloadAttempt, since a reload can fail, and in 
this case it would result keep trying to reload the invalid configuration, so 
if you'd introduce a lastReloadAttempt and set it each time before you try to 
reload the configuration, then you could use 
{code:java}
126 if (lastModified > lastReloadAttempt && 
127   time > lastReloadAttempt + monitoringInterval) {
{code}
This would guarantee that you don't reload more frequently than the 
monitoringInterval, you don't reload if the configuration hasn't been modified, 
but still keep reloading if the file is updated frequently.

Otherwise the patch looks good to me (non-binding).

> Capacity scheduler should support refresh queue automatically by a thread 
> policy.
> -
>
> Key: YARN-10623
> URL: https://issues.apache.org/jira/browse/YARN-10623
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: capacity scheduler
>Reporter: Qi Zhu
>Assignee: Qi Zhu
>Priority: Major
> Attachments: YARN-10623.001.patch, YARN-10623.002.patch, 
> YARN-10623.003.patch
>
>
> In fair scheduler, it is supported that refresh queue related conf 
> automatically by a thread to reload, but in capacity scheduler we only 
> support to refresh queue related changes by refreshQueues, it is needed for 
> our cluster to realize queue manage.
> cc [~wangda] [~ztang] [~pbacsko] [~snemeth] [~gandras]  [~bteke] [~shuzirra]



--
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-10632) Make maximum depth allowed configurable.

2021-02-19 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10632?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17287256#comment-17287256
 ] 

Gergely Pollak commented on YARN-10632:
---

Also please note that this has an effect on the placement engine as well, and 
currently the path validation methods are separated, so even if CS accepts 
deeper queue creations, placement rules wouldn't. First we need to centralize 
the validation for the queue paths to be created (YARN-10584), and also we need 
to make a few adjustments in the placement engine in order to make this work.

> Make maximum depth allowed configurable.
> 
>
> Key: YARN-10632
> URL: https://issues.apache.org/jira/browse/YARN-10632
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Qi Zhu
>Assignee: Qi Zhu
>Priority: Major
> Attachments: YARN-10632.001.patch
>
>
> Now the max depth allowed are fixed to 2. But i think this should be 
> configurable.



--
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] [Updated] (YARN-10636) CS Auto Queue creation should reject submissions with empty path parts

2021-02-19 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10636?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10636:
--
Attachment: YARN-10636.002.patch

> CS Auto Queue creation should reject submissions with empty path parts
> --
>
> Key: YARN-10636
> URL: https://issues.apache.org/jira/browse/YARN-10636
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10636.001.patch, YARN-10636.002.patch
>
>




--
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-10636) CS Auto Queue creation should reject submissions with empty path parts

2021-02-19 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17287056#comment-17287056
 ] 

Gergely Pollak commented on YARN-10636:
---

Thank you for the feedbacks [~zhuqi] [~gandras] [~bteke] much appreciated!

[~bteke] [~gandras] I'd suggest to create a followup Jira for test refinements.

Fixed the checkstyle issue and the redundant null check in patchset 2.

Test failures seem to be unrelated, manually ran them, all passed.

> CS Auto Queue creation should reject submissions with empty path parts
> --
>
> Key: YARN-10636
> URL: https://issues.apache.org/jira/browse/YARN-10636
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10636.001.patch, YARN-10636.002.patch
>
>




--
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-10414) Improve logging in CS placement rules to help debuggability

2021-02-19 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10414?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17287000#comment-17287000
 ] 

Gergely Pollak commented on YARN-10414:
---

A message improvement suggestion was provided in:

https://issues.apache.org/jira/browse/YARN-10635

> Improve logging in CS placement rules to help debuggability
> ---
>
> Key: YARN-10414
> URL: https://issues.apache.org/jira/browse/YARN-10414
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
>
> It should be always clear which MappingRule determined the placement (or 
> rejection) of the application, in the case of debug logging the whys should 
> also be more visible.



--
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] [Assigned] (YARN-10636) CS Auto Queue creation should reject submissions with empty path parts

2021-02-18 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10636?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak reassigned YARN-10636:
-

Assignee: Gergely Pollak

> CS Auto Queue creation should reject submissions with empty path parts
> --
>
> Key: YARN-10636
> URL: https://issues.apache.org/jira/browse/YARN-10636
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
>




--
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] [Created] (YARN-10636) CS Auto Queue creation should reject submissions with empty path parts

2021-02-18 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10636:
-

 Summary: CS Auto Queue creation should reject submissions with 
empty path parts
 Key: YARN-10636
 URL: https://issues.apache.org/jira/browse/YARN-10636
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak






--
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-10635) CSMapping rule can return paths with empty parts

2021-02-18 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17286816#comment-17286816
 ] 

Gergely Pollak commented on YARN-10635:
---

Reuploading to see if the yetus issue have been resolved.

> CSMapping rule can return paths with empty parts
> 
>
> Key: YARN-10635
> URL: https://issues.apache.org/jira/browse/YARN-10635
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10635.001.patch, YARN-10635.002.patch, 
> YARN-10635.003.patch
>
>
> When a variable to be substituted evaluates to empty string, we might result 
> with paths where one of the parts is empty, these paths are obviously 
> problematic, but sometimes (when the path includes a dynamicParent) we accept 
> them as valid paths instead of getting the fallback action of the rule.



--
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] [Updated] (YARN-10635) CSMapping rule can return paths with empty parts

2021-02-18 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10635?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10635:
--
Attachment: YARN-10635.003.patch

> CSMapping rule can return paths with empty parts
> 
>
> Key: YARN-10635
> URL: https://issues.apache.org/jira/browse/YARN-10635
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10635.001.patch, YARN-10635.002.patch, 
> YARN-10635.003.patch
>
>
> When a variable to be substituted evaluates to empty string, we might result 
> with paths where one of the parts is empty, these paths are obviously 
> problematic, but sometimes (when the path includes a dynamicParent) we accept 
> them as valid paths instead of getting the fallback action of the rule.



--
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-10635) CSMapping rule can return paths with empty parts

2021-02-18 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17286671#comment-17286671
 ] 

Gergely Pollak commented on YARN-10635:
---

Seems to be a YESTUS issue, reuploading to see if the issue persists.

> CSMapping rule can return paths with empty parts
> 
>
> Key: YARN-10635
> URL: https://issues.apache.org/jira/browse/YARN-10635
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10635.001.patch, YARN-10635.002.patch
>
>
> When a variable to be substituted evaluates to empty string, we might result 
> with paths where one of the parts is empty, these paths are obviously 
> problematic, but sometimes (when the path includes a dynamicParent) we accept 
> them as valid paths instead of getting the fallback action of the rule.



--
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] [Comment Edited] (YARN-10635) CSMapping rule can return paths with empty parts

2021-02-18 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17286671#comment-17286671
 ] 

Gergely Pollak edited comment on YARN-10635 at 2/18/21, 6:58 PM:
-

Seems to be a YETUS issue, reuploading to see if the issue persists.


was (Author: shuzirra):
Seems to be a YESTUS issue, reuploading to see if the issue persists.

> CSMapping rule can return paths with empty parts
> 
>
> Key: YARN-10635
> URL: https://issues.apache.org/jira/browse/YARN-10635
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10635.001.patch, YARN-10635.002.patch
>
>
> When a variable to be substituted evaluates to empty string, we might result 
> with paths where one of the parts is empty, these paths are obviously 
> problematic, but sometimes (when the path includes a dynamicParent) we accept 
> them as valid paths instead of getting the fallback action of the rule.



--
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] [Updated] (YARN-10635) CSMapping rule can return paths with empty parts

2021-02-18 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10635?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10635:
--
Attachment: YARN-10635.002.patch

> CSMapping rule can return paths with empty parts
> 
>
> Key: YARN-10635
> URL: https://issues.apache.org/jira/browse/YARN-10635
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10635.001.patch, YARN-10635.002.patch
>
>
> When a variable to be substituted evaluates to empty string, we might result 
> with paths where one of the parts is empty, these paths are obviously 
> problematic, but sometimes (when the path includes a dynamicParent) we accept 
> them as valid paths instead of getting the fallback action of the rule.



--
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] [Updated] (YARN-10635) CSMapping rule can return paths with empty parts

2021-02-18 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10635?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10635:
--
Attachment: YARN-10635.001.patch

> CSMapping rule can return paths with empty parts
> 
>
> Key: YARN-10635
> URL: https://issues.apache.org/jira/browse/YARN-10635
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10635.001.patch
>
>
> When a variable to be substituted evaluates to empty string, we might result 
> with paths where one of the parts is empty, these paths are obviously 
> problematic, but sometimes (when the path includes a dynamicParent) we accept 
> them as valid paths instead of getting the fallback action of the rule.



--
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] [Assigned] (YARN-10635) CSMapping rule can return paths with empty parts

2021-02-18 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10635?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak reassigned YARN-10635:
-

Assignee: Gergely Pollak

> CSMapping rule can return paths with empty parts
> 
>
> Key: YARN-10635
> URL: https://issues.apache.org/jira/browse/YARN-10635
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
>
> When a variable to be substituted evaluates to empty string, we might result 
> with paths where one of the parts is empty, these paths are obviously 
> problematic, but sometimes (when the path includes a dynamicParent) we accept 
> them as valid paths instead of getting the fallback action of the rule.



--
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] [Created] (YARN-10635) CSMapping rule can return paths with empty parts

2021-02-18 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10635:
-

 Summary: CSMapping rule can return paths with empty parts
 Key: YARN-10635
 URL: https://issues.apache.org/jira/browse/YARN-10635
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak


When a variable to be substituted evaluates to empty string, we might result 
with paths where one of the parts is empty, these paths are obviously 
problematic, but sometimes (when the path includes a dynamicParent) we accept 
them as valid paths instead of getting the fallback action of the rule.



--
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-10618) RM UI2 Application page shows the AM preempted containers instead of the nonAM ones

2021-02-10 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17282642#comment-17282642
 ] 

Gergely Pollak commented on YARN-10618:
---

[~bteke] thank you for the patch it is quite straightforward LGTM+1 
(non-binding).

> RM UI2 Application page shows the AM preempted containers instead of the 
> nonAM ones
> ---
>
> Key: YARN-10618
> URL: https://issues.apache.org/jira/browse/YARN-10618
> Project: Hadoop YARN
>  Issue Type: Bug
>  Components: yarn-ui-v2
>Reporter: Benjamin Teke
>Assignee: Benjamin Teke
>Priority: Minor
> Attachments: YARN-10618.001.patch
>
>
> YARN RM UIv2 application page shows the AM preempted containers under both 
> the _Num Non-AM container preempted_ and _Num AM container preempted_.



--
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] [Created] (YARN-10619) CS Mapping Rule %specified rule catches default submissions

2021-02-09 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10619:
-

 Summary: CS Mapping Rule %specified rule catches default 
submissions
 Key: YARN-10619
 URL: https://issues.apache.org/jira/browse/YARN-10619
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak
Assignee: Gergely Pollak


If we have a mapping rule which places the application to the %specified queue, 
then application submissions without specified queues (default) will get placed 
to default. 

The expected behaviour would be to fail the specified placement when no queue 
was specified, and move on or reject based on the fallback action of the rule. 

Also it is impossible to differentiate between explicitly specified 'default' 
and when the user does not specify any actual queue, so these will be handled 
the same.



--
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] [Comment Edited] (YARN-10612) Fix find bugs issue introduced in YARN-10585

2021-02-03 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10612?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17278453#comment-17278453
 ] 

Gergely Pollak edited comment on YARN-10612 at 2/4/21, 1:25 AM:


Trunk compile shows the findbugs error, patch compile doesn't so I think we can 
consider it fixed. The reason we don't have any tests for this change is the 
actual findbugs warning was caused by an unnecessary null check, and null 
inputs already have test cases.

Console log relevant part:
{code:java}


 findbugs detection: patch


...

Writing 
/home/jenkins/jenkins-agent/workspace/PreCommit-YARN-Build/out/combined-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.xmlhadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
 generated 0 new + 0 unchanged - 1 fixed = 0 total (was 1)
{code}


was (Author: shuzirra):
Trunk compile shows the findbugs error, patch compile doesn't so I think we can 
consider it fixed. The reason we don't have any tests for this change is the 
actual findbugs warning was caused by an unnecessary null check, and null 
inputs already have test cases.

> Fix find bugs issue introduced in YARN-10585
> 
>
> Key: YARN-10612
> URL: https://issues.apache.org/jira/browse/YARN-10612
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10612.001.patch
>
>




--
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] [Comment Edited] (YARN-10612) Fix find bugs issue introduced in YARN-10585

2021-02-03 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10612?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17278453#comment-17278453
 ] 

Gergely Pollak edited comment on YARN-10612 at 2/4/21, 1:18 AM:


Trunk compile shows the findbugs error, patch compile doesn't so I think we can 
consider it fixed. The reason we don't have any tests for this change is the 
actual findbugs warning was caused by an unnecessary null check, and null 
inputs already have test cases.


was (Author: shuzirra):
Trunk compile shows the findbugs error, patch compile doesn't so I think we can 
consider it fixed. The reason we don't have any tests for this change is the 
actual findbugs warning was caused by an unnecessary null check, and null 
inputs already have test cases.

 

 

 

> Fix find bugs issue introduced in YARN-10585
> 
>
> Key: YARN-10612
> URL: https://issues.apache.org/jira/browse/YARN-10612
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10612.001.patch
>
>




--
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-10612) Fix find bugs issue introduced in YARN-10585

2021-02-03 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10612?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17278453#comment-17278453
 ] 

Gergely Pollak commented on YARN-10612:
---

Trunk compile shows the findbugs error, patch compile doesn't so I think we can 
consider it fixed. The reason we don't have any tests for this change is the 
actual findbugs warning was caused by an unnecessary null check, and null 
inputs already have test cases.

 

 

 

> Fix find bugs issue introduced in YARN-10585
> 
>
> Key: YARN-10612
> URL: https://issues.apache.org/jira/browse/YARN-10612
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10612.001.patch
>
>




--
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-10585) Create a class which can convert from legacy mapping rule format to the new JSON format

2021-02-03 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10585?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17278360#comment-17278360
 ] 

Gergely Pollak commented on YARN-10585:
---

[~ahussein] thank you for your suggestions.
{code:java}
For future code mergse and commits, please make sure that the patch/PR does not 
generate Yetus errors before merging.
{code}
While I'm really sorry for the oversight, mistakes unfortunately do happen, as 
soon we realized it we opened an other Jira to fix it. Please let's not argue 
about what are the "proper way to fix things" but rather let's focus on 
actually fixing the thing.
{code:java}
It is not scalable to have several Jiras filed just to fix checkstyle, and 
findbugs.
{code}
It has nothing to do with scalability, also we are not planning to make it a 
habit, but I think it's more important to get this resolved than to worry about 
one extra JIRA.

> Create a class which can convert from legacy mapping rule format to the new 
> JSON format
> ---
>
> Key: YARN-10585
> URL: https://issues.apache.org/jira/browse/YARN-10585
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Fix For: 3.4.0
>
> Attachments: YARN-10585.001.patch, YARN-10585.002.patch, 
> YARN-10585.003.patch
>
>
> To make transition easier we need to create tooling to support the migration 
> effort. The first step is to create a class which can migrate from legacy to 
> the new JSON format.



--
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-10612) Fix find bugs issue introduced in YARN-10585

2021-02-03 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10612?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17278354#comment-17278354
 ] 

Gergely Pollak commented on YARN-10612:
---

[~ahussein] I don't see how it will make anything better if we reopen an other 
jira. Currently we have the patch which fails the tests in the trunk, so to 
remove it we either have to do a revert commit, then a recommit. But the 
commits between the actual commit and the revert will still fail the findbugs 
warning (which is a false positive I might add). So I don't see why would 2 
commits (revert + fix) would be better than just fixing this Jira and solve all 
in one go.

Anyways, until it settles, I'm setting it to patch available to let the jenkins 
run the findbugs again.

> Fix find bugs issue introduced in YARN-10585
> 
>
> Key: YARN-10612
> URL: https://issues.apache.org/jira/browse/YARN-10612
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10612.001.patch
>
>




--
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] [Assigned] (YARN-10612) Fix find bugs issue introduced in YARN-10585

2021-02-03 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10612?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak reassigned YARN-10612:
-

Assignee: Gergely Pollak

> Fix find bugs issue introduced in YARN-10585
> 
>
> Key: YARN-10612
> URL: https://issues.apache.org/jira/browse/YARN-10612
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10612.001.patch
>
>




--
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] [Updated] (YARN-10612) Fix find bugs issue introduced in YARN-10585

2021-02-03 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10612?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10612:
--
Attachment: YARN-10612.001.patch

> Fix find bugs issue introduced in YARN-10585
> 
>
> Key: YARN-10612
> URL: https://issues.apache.org/jira/browse/YARN-10612
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Priority: Major
> Attachments: YARN-10612.001.patch
>
>




--
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] [Created] (YARN-10612) Fix find bugs issue introduced in YARN-10585

2021-02-03 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10612:
-

 Summary: Fix find bugs issue introduced in YARN-10585
 Key: YARN-10612
 URL: https://issues.apache.org/jira/browse/YARN-10612
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak






--
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-10610) Add queuePath to restful api for CapacityScheduler consistent with FairScheduler queuePath.

2021-02-03 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17277968#comment-17277968
 ] 

Gergely Pollak commented on YARN-10610:
---

[~zhuqi] thank  you for the patch! Nice change, when we introduced the leaf 
queue / full path change we intentionally did not change any external 
interfaces to make sure we don't break any tools relying on these interfaces, 
however this change is very clean, and only extends the response, so there 
should be no such issue with it!

LGTM +1 (Non-binding)

> Add queuePath to restful api for CapacityScheduler consistent with 
> FairScheduler queuePath.
> ---
>
> Key: YARN-10610
> URL: https://issues.apache.org/jira/browse/YARN-10610
> Project: Hadoop YARN
>  Issue Type: Improvement
>Reporter: Qi Zhu
>Assignee: Qi Zhu
>Priority: Major
> Attachments: YARN-10610.001.patch, image-2021-02-03-13-47-13-516.png
>
>
> The cs only have queueName, but not full queuePath.
> !image-2021-02-03-13-47-13-516.png|width=631,height=356!
>  
>  



--
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-10604) Support auto queue creation without mapping rules

2021-02-01 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17276351#comment-17276351
 ] 

Gergely Pollak commented on YARN-10604:
---

[~gandras]thank you for the patch, LGTM+1 (Non-binding)

> Support auto queue creation without mapping rules
> -
>
> Key: YARN-10604
> URL: https://issues.apache.org/jira/browse/YARN-10604
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Andras Gyori
>Assignee: Andras Gyori
>Priority: Major
> Attachments: YARN-10604.001.patch
>
>
> Currently, the Capacity Scheduler skips auto queue creation entirely, if the 
> ApplicationPlacementContext is null, which happens, when the mapping rules 
> are turned off by:
> {noformat}
> 
> yarn.scheduler.capacity.queue-mappings-override.enable
> false
> {noformat}
> We should allow the auto queue creation to be taken into consideration 
> without disrupting the application submission flow.
>  



--
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-10605) Add queue-mappings-override.enable property in FS2CS conversions

2021-02-01 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10605?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17276347#comment-17276347
 ] 

Gergely Pollak commented on YARN-10605:
---

[~gandras]Thank you for the patch! The only thing I've noticed you import 
import static org.junit.Assert.assertFalse, but you don't use it.

Otherwise LGTM+1 (Non-binding)

> Add queue-mappings-override.enable property in FS2CS conversions
> 
>
> Key: YARN-10605
> URL: https://issues.apache.org/jira/browse/YARN-10605
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Andras Gyori
>Assignee: Andras Gyori
>Priority: Major
> Attachments: YARN-10605.001.patch
>
>
> In Capacity Scheduler the
> {noformat}
> queue-mappings-override.enable
> {noformat}
> property is false by default. As this is not set during an FS2CS conversion, 
> the converted placement rules (aka. mapping rules in CS) are ignored during 
> application submission. We should enable this property in the conversion 
> logic if there are placement rules to be converted.



--
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-10585) Create a class which can convert from legacy mapping rule format to the new JSON format

2021-01-26 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10585?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17272072#comment-17272072
 ] 

Gergely Pollak commented on YARN-10585:
---

Latest patch fixed [~gandras] "userGroupMappingRules and 
applicationNameMappingRules might be initialised as empty sets" concern, now 
the null check is centralised and moved to the setters. Also fixed ASF 
warnings, and added a testcase for the "u:%user:root.%user.QUEUE" case.

> Create a class which can convert from legacy mapping rule format to the new 
> JSON format
> ---
>
> Key: YARN-10585
> URL: https://issues.apache.org/jira/browse/YARN-10585
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10585.001.patch, YARN-10585.002.patch, 
> YARN-10585.003.patch
>
>
> To make transition easier we need to create tooling to support the migration 
> effort. The first step is to create a class which can migrate from legacy to 
> the new JSON format.



--
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] [Updated] (YARN-10585) Create a class which can convert from legacy mapping rule format to the new JSON format

2021-01-26 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10585?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10585:
--
Attachment: YARN-10585.003.patch

> Create a class which can convert from legacy mapping rule format to the new 
> JSON format
> ---
>
> Key: YARN-10585
> URL: https://issues.apache.org/jira/browse/YARN-10585
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10585.001.patch, YARN-10585.002.patch, 
> YARN-10585.003.patch
>
>
> To make transition easier we need to create tooling to support the migration 
> effort. The first step is to create a class which can migrate from legacy to 
> the new JSON format.



--
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

2021-01-25 Thread Gergely Pollak (Jira)


[ 
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] [Assigned] (YARN-10597) CSMappingPlacementRule should not create new instance of Groups

2021-01-25 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10597?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak reassigned YARN-10597:
-

Assignee: Gergely Pollak

> CSMappingPlacementRule should not create new instance of Groups
> ---
>
> Key: YARN-10597
> URL: https://issues.apache.org/jira/browse/YARN-10597
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
>
> As [~ahussein] pointed out in YARN-10425, no new Groups instance should be 
> created.



--
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] [Created] (YARN-10597) CSMappingPlacementRule should not create new instance of Groups

2021-01-25 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10597:
-

 Summary: CSMappingPlacementRule should not create new instance of 
Groups
 Key: YARN-10597
 URL: https://issues.apache.org/jira/browse/YARN-10597
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak


As [~ahussein] pointed out in YARN-10425, no new Groups instance should be 
created.



--
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] [Comment Edited] (YARN-10585) Create a class which can convert from legacy mapping rule format to the new JSON format

2021-01-25 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10585?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17271297#comment-17271297
 ] 

Gergely Pollak edited comment on YARN-10585 at 1/25/21, 1:01 PM:
-

[~gandras] thank you for the review and your comments, let me address your 
concerns.
 * Nit: in splitRule#line:213 the classic for loop could be replaced by the 
enhanced loop/stream, which is nicer to readI

It is VERY subjective what is nice / easy to read, but in this case it's quite 
hard to contradict this statement. Literally ANY developer who ever wrote a 
loop, will be able to read a simple for loop, while to understand streams they 
need some deeper java knowledge, so I think it points out very well, why is the 
for loop easier (this nicer) to read.

But let's get technical a bit, because the overuse of the java streams is a pet 
peeve of mine. Java streams with lambdas will create an unnecessary (well for 
streams they ARE necessary) anonymous class in the background, and wrap the 
actual loop core with multiple method calls, which inherently put extra strain 
on the stack, gc and on the performance, for a very subjective little gain.  
{code:java}
java.lang.ArithmeticException: / by zero
at fiddle.main(fiddle.java:7)

java.lang.ArithmeticException: / by zero
at fiddle.lambda$main$0(fiddle.java:14)
at 
java.util.stream.Streams$RangeIntSpliterator.forEachRemaining(Streams.java:110)
at java.util.stream.IntPipeline$Head.forEach(IntPipeline.java:581)
at fiddle.main(fiddle.java:14){code}
 We can see the extra method calls and the created class as well. 

This has an effect on performance as well: 
{code:java}
int sum = 0;
for (int i = 0; i < 10; i++) {
  sum += i;
}
{code}
Runs on my computer about 330ms

 
{code:java}
IntStream.range(0, 10).sum();{code}
 

This has a bit of a range between 380-420 ms, but even in best case it is still 
a 20% performance degradation.

So all in all, I prefer to avoid streams, when they are not strictly needed, 
because they are not to replace regular loops but to add some functional 
programming support to java. They have their place in the language, for example 
when one wants to use parallel streams they can really improve the performance, 
with much less hassle about thread handling and concurrency.

 
 * Nit: in creatUserMappingRule, you replace the match argument with its json 
equivalent (*). Overwriting arguments are generally not a good idea, because it 
could produce hard-to-find errors, also made the debugging somewhat harder.

As a general rule it's true, however I'm converting the argument in place, 
before using it, it is not a repurpose of the variable, but I'm making sure, 
the function receives the input it will properly process, in this case I don't 
really find it problematic, since the goal here is to "hide" the incorrect 
value from the rest of the method, hence the overwrite. 

 
 * I am not sure about this, but is the following format accepted in the legacy 
format:

Yeah, it is a complex question. It is NOT supported by the legacy engine, but 
it works in the new engine with legacy mode... however I've took a look at it, 
and actually the converter DOES support this conversion, it converts it to a 
customRule. I don't want to add any validation to the input, besides it's 
syntax, because the tool will do a best effort conversion for unsupported 
cases. It should be able to convert EVERY use case supported by the legacy 
engine, and all cases supported by the new engine in legacy format, however we 
don't encourage to use the new features in the legacy format. So if someone 
still uses them we might be able to convert those rules as well (probably they 
will become custom rules) so I don't want to reject those just because they are 
not officially supported. 
 * userGroupMappingRules and applicationNameMappingRules might be initialised 
as empty sets. This would eliminate the null checks in convert loops, but the 
empty cases could still be checked in the beginning.

Since I expose the collection setter method then I'd have to move the null 
check there, so we don't really save null checks, but at least we don't create 
unnecessary objects. However I like the idea to have a centralised null check, 
so I'll see the other feedbacks, and if I'll make a new patch set I'll consider 
this suggestion.


was (Author: shuzirra):
[~gandras] thank you for the review and your comments, let me address your 
concerns.
 * Nit: in splitRule#line:213 the classic for loop could be replaced by the 
enhanced loop/stream, which is nicer to readI

It is VERY subjective what is nice / easy to read, but in this case it's quite 
hard to contradict this statement. Literally ANY developer who ever wrote a 
loop, will be able to read a simple for loop, while to understand streams they 
need some deeper java knowledge, so I think it 

[jira] [Commented] (YARN-10585) Create a class which can convert from legacy mapping rule format to the new JSON format

2021-01-25 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10585?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17271297#comment-17271297
 ] 

Gergely Pollak commented on YARN-10585:
---

[~gandras] thank you for the review and your comments, let me address your 
concerns.
 * Nit: in splitRule#line:213 the classic for loop could be replaced by the 
enhanced loop/stream, which is nicer to readI

It is VERY subjective what is nice / easy to read, but in this case it's quite 
hard to contradict this statement. Literally ANY developer who ever wrote a 
loop, will be able to read a simple for loop, while to understand streams they 
need some deeper java knowledge, so I think it points out very well, why is the 
for loop easier (this nicer) to read.

But let's get technical a bit, because the overuse of the java streams is a pet 
peeve of mine. Java streams with lambdas will create an unnecessary (well for 
streams they ARE necessary) anonymous class in the background, and wrap the 
actual loop core with multiple method calls, which inherently put extra strain 
on the stack, gc and on the performance, for a very subjective little gain. 

 
{code:java}
java.lang.ArithmeticException: / by zero
at fiddle.main(fiddle.java:7)

java.lang.ArithmeticException: / by zero
at fiddle.lambda$main$0(fiddle.java:14)
at 
java.util.stream.Streams$RangeIntSpliterator.forEachRemaining(Streams.java:110)
at java.util.stream.IntPipeline$Head.forEach(IntPipeline.java:581)
at fiddle.main(fiddle.java:14){code}
 

We can see the extra method calls and the created class as well. 

This has an effect on performance as well:

 
{code:java}
int sum = 0;
for (int i = 0; i < 10; i++) {
  sum += i;
}
{code}
Runs on my computer about 330ms

 

 
{code:java}
IntStream.range(0, 10).sum();{code}
 

This has a bit of a range between 380-420 ms, but even in best case it is still 
a 20% performance degradation.

So all in all, I prefer to avoid streams, when they are not strictly needed, 
because they are not to replace regular loops but to add some functional 
programming support to java. They have their place in the language, for example 
when one wants to use parallel streams they can really improve the performance, 
with much less hassle about thread handling and concurrency.

 
 * Nit: in creatUserMappingRule, you replace the match argument with its json 
equivalent (*). Overwriting arguments are generally not a good idea, because it 
could produce hard-to-find errors, also made the debugging somewhat harder.

As a general rule it's true, however I'm converting the argument in place, 
before using it, it is not a repurpose of the variable, but I'm making sure, 
the function receives the input it will properly process, in this case I don't 
really find it problematic, since the goal here is to "hide" the incorrect 
value from the rest of the method, hence the overwrite. 

 
 * I am not sure about this, but is the following format accepted in the legacy 
format:

Yeah, it is a complex question. It is NOT supported by the legacy engine, but 
it works in the new engine with legacy mode... however I've took a look at it, 
and actually the converter DOES support this conversion, it converts it to a 
customRule. I don't want to add any validation to the input, besides it's 
syntax, because the tool will do a best effort conversion for unsupported 
cases. It should be able to convert EVERY use case supported by the legacy 
engine, and all cases supported by the new engine in legacy format, however we 
don't encourage to use the new features in the legacy format. So if someone 
still uses them we might be able to convert those rules as well (probably they 
will become custom rules) so I don't want to reject those just because they are 
not officially supported. 
 * userGroupMappingRules and applicationNameMappingRules might be initialised 
as empty sets. This would eliminate the null checks in convert loops, but the 
empty cases could still be checked in the beginning.

Since I expose the collection setter method then I'd have to move the null 
check there, so we don't really save null checks, but at least we don't create 
unnecessary objects. However I like the idea to have a centralised null check, 
so I'll see the other feedbacks, and if I'll make a new patch set I'll consider 
this suggestion.

> Create a class which can convert from legacy mapping rule format to the new 
> JSON format
> ---
>
> Key: YARN-10585
> URL: https://issues.apache.org/jira/browse/YARN-10585
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10585.001.patch, YARN-10585.002.patch
>
>
> To make transition easier 

[jira] [Updated] (YARN-10585) Create a class which can convert from legacy mapping rule format to the new JSON format

2021-01-21 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10585?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10585:
--
Attachment: YARN-10585.002.patch

> Create a class which can convert from legacy mapping rule format to the new 
> JSON format
> ---
>
> Key: YARN-10585
> URL: https://issues.apache.org/jira/browse/YARN-10585
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10585.001.patch, YARN-10585.002.patch
>
>
> To make transition easier we need to create tooling to support the migration 
> effort. The first step is to create a class which can migrate from legacy to 
> the new JSON format.



--
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-10585) Create a class which can convert from legacy mapping rule format to the new JSON format

2021-01-21 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10585?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17269316#comment-17269316
 ] 

Gergely Pollak commented on YARN-10585:
---

Fixed the unit test failures, improved the conversion of rules like 
root.deep.%primary_group.%secondary_group, which was not supported by the 
legacy mapping rule engine, but the new mapping engine supports it even with 
the legacy format, so the converter should too.

> Create a class which can convert from legacy mapping rule format to the new 
> JSON format
> ---
>
> Key: YARN-10585
> URL: https://issues.apache.org/jira/browse/YARN-10585
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10585.001.patch, YARN-10585.002.patch
>
>
> To make transition easier we need to create tooling to support the migration 
> effort. The first step is to create a class which can migrate from legacy to 
> the new JSON format.



--
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] [Created] (YARN-10586) Create a command line tool for converting from legacy CS mapping rule format

2021-01-20 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10586:
-

 Summary: Create a command line tool for converting from legacy CS 
mapping rule format
 Key: YARN-10586
 URL: https://issues.apache.org/jira/browse/YARN-10586
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak
Assignee: Gergely Pollak






--
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] [Created] (YARN-10585) Create a class which can convert from legacy mapping rule format to the new JSON format

2021-01-20 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10585:
-

 Summary: Create a class which can convert from legacy mapping rule 
format to the new JSON format
 Key: YARN-10585
 URL: https://issues.apache.org/jira/browse/YARN-10585
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak
Assignee: Gergely Pollak


To make transition easier we need to create tooling to support the migration 
effort. The first step is to create a class which can migrate from legacy to 
the new JSON format.



--
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] [Created] (YARN-10584) Move the functionality of MappingRuleValidationHelper to CSQueueManager

2021-01-20 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10584:
-

 Summary: Move the functionality of MappingRuleValidationHelper to 
CSQueueManager
 Key: YARN-10584
 URL: https://issues.apache.org/jira/browse/YARN-10584
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak
Assignee: Gergely Pollak


The only reason we created the helper class was to make the features introduced 
in YARN-10506 accessible as soon as possible. Due to our current test helpers 
and frameworks, integrating these changes into the 
CapacitySchedulerQueueManager would have been more time consuming.

However we need to remove this standalone class and merge the functionality 
into either the QueueManager or into CapacityScheduler.



--
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] [Assigned] (YARN-10583) Increase the test coverage for validation in MappingRuleValidationHelper

2021-01-20 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10583?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak reassigned YARN-10583:
-

Assignee: Gergely Pollak

> Increase the test coverage for validation in MappingRuleValidationHelper
> 
>
> Key: YARN-10583
> URL: https://issues.apache.org/jira/browse/YARN-10583
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
>
> MappingRuleValidationHelper introduced new validation cases due to the 
> changes in YARN-10506, we need to increase the coverage before we move the 
> class into QueueManager



--
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] [Created] (YARN-10583) Increase the test coverage for validation in MappingRuleValidationHelper

2021-01-20 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10583:
-

 Summary: Increase the test coverage for validation in 
MappingRuleValidationHelper
 Key: YARN-10583
 URL: https://issues.apache.org/jira/browse/YARN-10583
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak


MappingRuleValidationHelper introduced new validation cases due to the changes 
in YARN-10506, we need to increase the coverage before we move the class into 
QueueManager



--
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] [Created] (YARN-10582) Increase the test coverage for CSMappingPlacementRule

2021-01-20 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10582:
-

 Summary: Increase the test coverage for CSMappingPlacementRule
 Key: YARN-10582
 URL: https://issues.apache.org/jira/browse/YARN-10582
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak
Assignee: Gergely Pollak


We have somewhat limited coverage for cases introduced via YARN-10506, to make 
sure we won't have any regression we need to improve the coverage.



--
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-10578) Fix Auto Queue Creation parent handling

2021-01-19 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17267869#comment-17267869
 ] 

Gergely Pollak commented on YARN-10578:
---

[~gandras] thank you for the patch, my only observation is you are using a lock 
before calling the  _autoQueueHandler.autoCreateQueue(placementContext_); 
however if this operation requires a lock, it would be  better to have that 
lock in the _autoCreateQueue_ method, to make sure no other code path can 
concurrently modify it. (Even if we don't call it elsewhere currently).

Otherwise LGTM+1 (Non-binding)

> Fix Auto Queue Creation parent handling
> ---
>
> Key: YARN-10578
> URL: https://issues.apache.org/jira/browse/YARN-10578
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Andras Gyori
>Assignee: Andras Gyori
>Priority: Major
> Attachments: YARN-10578.001.patch, YARN-10578.002.patch
>
>
> YARN-10506 introduced the new auto queue creation logic, however a parent == 
> null check in CapacityScheduler#autoCreateLeafQueue will prevent a two levels 
> queue to be created. We need to revert it back to the normal logic, also, we 
> should wrap the auto queue handling with a lock.



--
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] [Updated] (YARN-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-18 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10535:
--
Attachment: YARN-10535.006.patch

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch, 
> YARN-10535.003.patch, YARN-10535.004.patch, YARN-10535.005.patch, 
> YARN-10535.006.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-17 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266941#comment-17266941
 ] 

Gergely Pollak commented on YARN-10535:
---

[~snemeth] thank you for the review, a good portion of this work needs to be 
merged into CapacityScheduler or CapacitySchedulerQueueManager, this will be 
done as a separate effort, since it requires a bit more refactoring, and I 
wanted this patch out soon, because without this YARN-10506 won't be able to 
utilize it's new feature set. So for most of your point my answer will be "will 
be fixed in a followup jira". :)

1) I find it overkill to create a separate method for a simple split. It is 
technically a split which puts the result into an ArrayList, but later I'll try 
to find a more centralized place for parsing the queuePath this way.

1.1-1.5) We already have a lot of queue path storing classes (at least 2, but 
there might be 3), so we should create only one if possible, this might be the 
good reason to merge whatever possible. I simply didn't want to introduce a new 
one, but I agree we need to centralize the QueuePath operations in a dedicated 
class.

1.6) I don't want to add a getGrandparent method, because YARN-10506 is written 
in a way, it can support more depth than 2, so I would like to make this code 
flexible during the next iteration. Currently we limit the max depth to 2, but 
the underling code supports more depth, so should we. But we'll need some 
getNthParent method, where n=1 is the parent, n=2 grandparent ... etc, so 
generally I agree with the concept, but we need to increase flexibility, to 
make the code future proof.

2) The confusing part here is that "dynamic" is overused. Originally I used 
called static path when a TARGET path was full static  eg. "root.someting.leaf" 
and dynamic when it had variables which are to be substituted, eg. 
"root.group.%primary_group". But YARN-10506 introduced the dynamic parent 
concept, so dynamic now can mean 2 different thing in this context. The 
validateXXXQueuePath (where XXX STRICTLY for dynamic/static) methods are only 
used for TARGET path validation. And not for validating paths with dynamic 
parents. I hope it makes sense.

3.1) Yes it could be, but still I prefer it dynamic, since it is used within 
the context, so there is not much point in calling one VALIDATION helper method 
without a context. Technically it could be, semantically I don't think it 
should. Also this is something which should be in the QueueManager in the first 
place, or something we should query about the queue, so the best would be this 
to be in the CSQueue or in QueueManager. During refactor I'll remove it from 
here for sure.

3.2) I had only 1 typo? Cool :) Thx for finding it

3.3) The WHOLE CLASS should go. And it will, as soon I can integrate it to QM, 
however that requires some test helper refactors as well, since currently QM is 
mocked in a good portion of the tests, and we need this functionality during 
the tests, so this is the only reason it is in a separate class for now.

3.3.1) Please see my earlier repsonse regarding queue parsing classes.

3.3.2) I don't think an empty check should be a separate method, This is 
literally a hasNext / throw check, I don't want to create a method with the 
only purpose of throwing an exception.

3.3.3) The isPathStatic IS the dedicated method, the 
staticPartBuffer.toString() is the argument. Again I think it is overkill to 
move it to an other method.
{code:java}
THIS:

if (!isPathStatic(staticPartBuffer.toString())) {
  return true;
}

Would become THIS:

if (!checkRoot(staticPartBuffer)) { 
  return true; 
}
{code}
Also the stringBuffer is kind of a special representation here, so probably we 
would still keep the staticPartBuffer.toString() call, so all in all we would 
have an extra call on our stack

3.3.4) This is a special validation method which we need only once, we don't 
need to store the static part of the queue, we do some operation on it, the we 
won't touch it again, I see no reason to extract this, since it's mostly part 
of the validation, and not really reusable.

3.3.5-3.3.6) The whole static part of a queue is a mapping rule specific 
validation thing, we could make methods for all of it, but we would only use 
those methods once, and we'd need a few extra fields. Generally I agree we 
should keep the size of the functions minimal, but these are all steps in the 
very same and quite unique validation process, with almost 0 reusability, hence 
I kept all in one method.

4) Will take a look at it, actually it might result in false positive result, 
thx!

Thank you again for the detailed feedbacks, I'll fix the most urgent issues 
here, and then keep the rest in mind for the followup tasks.

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -

[jira] [Commented] (YARN-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-17 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266771#comment-17266771
 ] 

Gergely Pollak commented on YARN-10535:
---

Patchset#5 fixes the new checkstyle/findbugs/javadoc warnings introduced during 
patchet#4.

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch, 
> YARN-10535.003.patch, YARN-10535.004.patch, YARN-10535.005.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Comment Edited] (YARN-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-17 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266771#comment-17266771
 ] 

Gergely Pollak edited comment on YARN-10535 at 1/17/21, 12:41 PM:
--

Patchset#5 fixes the new checkstyle/findbugs/javadoc warnings introduced during 
patchet#4.

Test failure is unrelated, test seems to be flaky.


was (Author: shuzirra):
Patchset#5 fixes the new checkstyle/findbugs/javadoc warnings introduced during 
patchet#4.

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch, 
> YARN-10535.003.patch, YARN-10535.004.patch, YARN-10535.005.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Updated] (YARN-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-17 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10535:
--
Attachment: YARN-10535.005.patch

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch, 
> YARN-10535.003.patch, YARN-10535.004.patch, YARN-10535.005.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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-10574) Fix the FindBugs warning introduced in YARN-10506

2021-01-17 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10574?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266769#comment-17266769
 ] 

Gergely Pollak commented on YARN-10574:
---

Test failure is unrelated, test seems to be flaky.

Test have not been added, because there is no functionality change, only fixing 
a FindBugs recommendation, everything is expected to work exactly the same way.

> Fix the FindBugs warning introduced in YARN-10506
> -
>
> Key: YARN-10574
> URL: https://issues.apache.org/jira/browse/YARN-10574
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10574.001.patch
>
>
> Findbugs started to give warnings about an unnecessary null check, it's 
> better to get rid of it.



--
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-10574) Fix the FindBugs warning introduced in YARN-10506

2021-01-16 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10574?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266687#comment-17266687
 ] 

Gergely Pollak commented on YARN-10574:
---

I've just removed the null check, because the previous line will throw an 
exception if the variable is null, and it bothered findbugs. 

> Fix the FindBugs warning introduced in YARN-10506
> -
>
> Key: YARN-10574
> URL: https://issues.apache.org/jira/browse/YARN-10574
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10574.001.patch
>
>
> Findbugs started to give warnings about an unnecessary null check, it's 
> better to get rid of it.



--
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-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-16 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266686#comment-17266686
 ] 

Gergely Pollak commented on YARN-10535:
---

Patchset 4 is supposed to fix all findbugs and checkstyle issues, as well as 
the relevant unit tests. TestCapacitySchedulerAutoQueueCreation were due to 
error message changes, while TestFairSchedulerOvercommit is simply flaky.

Fixed the trunk findbugs warning in YARN-10574.

There are still some more tests to add to increase coverage, but otherwise 
patch is near completion.

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch, 
> YARN-10535.003.patch, YARN-10535.004.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Updated] (YARN-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-16 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10535:
--
Attachment: YARN-10535.004.patch

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch, 
> YARN-10535.003.patch, YARN-10535.004.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Assigned] (YARN-10574) Fix the FindBugs warning introduced in YARN-10506

2021-01-16 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10574?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak reassigned YARN-10574:
-

Assignee: Gergely Pollak

> Fix the FindBugs warning introduced in YARN-10506
> -
>
> Key: YARN-10574
> URL: https://issues.apache.org/jira/browse/YARN-10574
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
>
> Findbugs started to give warnings about an unnecessary null check, it's 
> better to get rid of it.



--
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] [Created] (YARN-10574) Fix the FindBugs warning introduced in YARN-10506

2021-01-16 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10574:
-

 Summary: Fix the FindBugs warning introduced in YARN-10506
 Key: YARN-10574
 URL: https://issues.apache.org/jira/browse/YARN-10574
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak


Findbugs started to give warnings about an unnecessary null check, it's better 
to get rid of it.



--
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-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-15 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266413#comment-17266413
 ] 

Gergely Pollak commented on YARN-10535:
---

Dependency merged patchset 3 is a reupload to retrigger the build.

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch, 
> YARN-10535.003.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Updated] (YARN-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-15 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10535:
--
Attachment: YARN-10535.003.patch

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch, 
> YARN-10535.003.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-15 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266139#comment-17266139
 ] 

Gergely Pollak commented on YARN-10535:
---

[~pbacsko] [~gandras] Thank you for the review, I fixed the most issues 
(checkstyle might find some more). Also when jenkins can run the full test 
suite we might get some test failures which needs to be fixed. 

*7. {{MappingRuleValidationContextImpl}}: {{staticPartParent}} is initially 
{{null}}. Any change of it remaining {{null}} after the {{while}} loop? If so, 
a null check is warranted.*

There is an explicit check for null state of staticPartParent, but null is 
totally legit, if there is no parent of the static part. But that case is 
handled.

 

*I can't really comment on the validation logic because there a lot of string 
operations and hopefully the unit tests will do their job.*

Tests only cover the current cases, they won't provide enough coverage, for 
validation I've already added a few test, but I still need a few for the 
CSMappingPlacementRule class.

 

*Talking about unit tests: {{TestMappingRuleValidationContextImpl}} was 
extended with some new cases, does that provide enough coverage? Have you run 
it through a coverage tool?*

Tests are still being written

 

*EDIT: ok, the patch does not compile yet, so probably the answer is "no".*

It won't compile until YARN-10506 is merged.

 

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Updated] (YARN-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-15 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10535:
--
Attachment: YARN-10535.002.patch

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch, YARN-10535.002.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Comment Edited] (YARN-10506) Update queue creation logic to use weight mode and allow the flexible static/dynamic creation

2021-01-15 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266081#comment-17266081
 ] 

Gergely Pollak edited comment on YARN-10506 at 1/15/21, 2:46 PM:
-

Thank you [~gandras] for the fix, here are a few comments, most of them can 
wait for a followup JIRA:

 

*CapacitySchedulerAutoQueueHandler class*:
 Why is it a separate class, this is the functionality of the actual queue, I 
think it would have been better to extend the parentQueue and add this 
functionality there, this is a class which automagicly changes the ParentQueue 
class and enhances it with new functionality, but I think it would be better to 
solve this via inheritance. Also that would make differentiating between 
ParentQueue and the DynamicParentQueue much easier. Probably we should fix it 
in a followup JIRA.

*CapacitySchedulerAutoQueueHandler#getQueue*
 This method is unnecessary, since CSQueueStore.get method handles null paths, 
and returns null. See CapacitySchedulerQueueManager#getQueue and 
CSQueueStore#get

*CapacitySchedulerAutoQueueHandler#autoCreateQueue*
 I might be wrong, but this is concerning:
{code:java}
CSQueue existingQueueCandidate = 
getQueue(queueCandidateContext.getQueue());{code}
 The getQueue will always return the leaf name of the queue, which might be 
ambiguous so this might return null even if the queue exists on the path we are 
checking, and might identify an already existing queue invalid.

As a rule of thumb, wherever possible use the full path of the queues please!

If we have the following structure:
{code:java}
root
  +--alice
  |    +--test
  |    +--dev
  +--bob
  |    +--test
  |    +--dev
{code}
 Where both _dev_ and _test_ queues are dynamic parents, then when I try to 
submit something to root.bob.dev.something, this part of the code 
{code:java}
    ApplicationPlacementContext queueCandidateContext = parentContext; // 
(Parent context is root.bob.dev)
    CSQueue existingQueueCandidate = 
getQueue(queueCandidateContext.getQueue()); //queueCandidateContext.getQueue() 
= 'dev'
    getQueue(queueCandidateContext.getQueue()) == null //because dev is 
ambiguous 
{code}
Will identify root.bob.dev as a not existing queue. 

So please use _ApplciationPlacementContext#getFullQueuePath_ 

*CapacitySchedulerAutoQueueHandler#isDynamicQueue* 
 Is not fool/future proof, while I'm aware currently only the AbstractCSQueue 
class implements the interface, but still it would be better to check before 
casting it to AbstractCSQueue. Or move the isDynamic down to the interface, 
then you don't have to cast

 


was (Author: shuzirra):
Thank you [~gandras] for the fix, here are a few comments, most of them can 
wait for a followup JIRA:

 

*CapacitySchedulerAutoQueueHandler class*:
Why is it a separate class, this is the functionality of the actual queue, I 
think it would have been better to extend the parentQueue and add this 
functionality there, this is a class which automagicly changes the ParentQueue 
class and enhances it with new functionality, but I think it would be better to 
solve this via inheritance. Also that would make differentiating between 
ParentQueue and the DynamicParentQueue much easier. Probably we should fix it 
in a followup JIRA.

*CapacitySchedulerAutoQueueHandler#getQueue*
This method is unnecessary, since CSQueueStore.get method handles null paths, 
and returns null. See CapacitySchedulerQueueManager#getQueue and 
CSQueueStore#get

*CapacitySchedulerAutoQueueHandler#autoCreateQueue*
I might be wrong, but this is concerning:

 
{code:java}
CSQueue existingQueueCandidate = 
getQueue(queueCandidateContext.getQueue());{code}
 

The getQueue will always return the leaf name of the queue, which might be 
ambiguous so this might return null even if the queue exists on the path we are 
checking, and might identify an already existing queue invalid.

As a rule of thumb, wherever possible use the full path of the queues please!

If we have the following structure:

 
{code:java}
root
  +--alice
  |    +--test
  |    +--dev
  +--bob
  |    +--test
  |    +--dev
{code}
 

 Where both _dev_ and _test_ queues are dynamic parents, then when I try to 
submit something to root.bob.dev.something, this part of the code

 
{code:java}
    ApplicationPlacementContext queueCandidateContext = parentContext; // 
(Parent context is root.bob.dev)
    CSQueue existingQueueCandidate = 
getQueue(queueCandidateContext.getQueue()); //queueCandidateContext.getQueue() 
= 'dev'
    getQueue(queueCandidateContext.getQueue()) == null //because dev is 
ambiguous 
{code}
Will identify root.bob.dev as a not existing queue. 

So please use _ApplciationPlacementContext#getFullQueuePath_

 

*CapacitySchedulerAutoQueueHandler#isDynamicQueue* 
Is not fool/future proof, while I'm aware currently only the AbstractCSQueue 
class implements the interface, but still it would be better to check 

[jira] [Commented] (YARN-10506) Update queue creation logic to use weight mode and allow the flexible static/dynamic creation

2021-01-15 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17266081#comment-17266081
 ] 

Gergely Pollak commented on YARN-10506:
---

Thank you [~gandras] for the fix, here are a few comments, most of them can 
wait for a followup JIRA:

 

*CapacitySchedulerAutoQueueHandler class*:
Why is it a separate class, this is the functionality of the actual queue, I 
think it would have been better to extend the parentQueue and add this 
functionality there, this is a class which automagicly changes the ParentQueue 
class and enhances it with new functionality, but I think it would be better to 
solve this via inheritance. Also that would make differentiating between 
ParentQueue and the DynamicParentQueue much easier. Probably we should fix it 
in a followup JIRA.

*CapacitySchedulerAutoQueueHandler#getQueue*
This method is unnecessary, since CSQueueStore.get method handles null paths, 
and returns null. See CapacitySchedulerQueueManager#getQueue and 
CSQueueStore#get

*CapacitySchedulerAutoQueueHandler#autoCreateQueue*
I might be wrong, but this is concerning:

 
{code:java}
CSQueue existingQueueCandidate = 
getQueue(queueCandidateContext.getQueue());{code}
 

The getQueue will always return the leaf name of the queue, which might be 
ambiguous so this might return null even if the queue exists on the path we are 
checking, and might identify an already existing queue invalid.

As a rule of thumb, wherever possible use the full path of the queues please!

If we have the following structure:

 
{code:java}
root
  +--alice
  |    +--test
  |    +--dev
  +--bob
  |    +--test
  |    +--dev
{code}
 

 Where both _dev_ and _test_ queues are dynamic parents, then when I try to 
submit something to root.bob.dev.something, this part of the code

 
{code:java}
    ApplicationPlacementContext queueCandidateContext = parentContext; // 
(Parent context is root.bob.dev)
    CSQueue existingQueueCandidate = 
getQueue(queueCandidateContext.getQueue()); //queueCandidateContext.getQueue() 
= 'dev'
    getQueue(queueCandidateContext.getQueue()) == null //because dev is 
ambiguous 
{code}
Will identify root.bob.dev as a not existing queue. 

So please use _ApplciationPlacementContext#getFullQueuePath_

 

*CapacitySchedulerAutoQueueHandler#isDynamicQueue* 
Is not fool/future proof, while I'm aware currently only the AbstractCSQueue 
class implements the interface, but still it would be better to check before 
casting it to AbstractCSQueue. Or move the isDynamic down to the interface, 
then you don't have to cast

 

> Update queue creation logic to use weight mode and allow the flexible 
> static/dynamic creation
> -
>
> Key: YARN-10506
> URL: https://issues.apache.org/jira/browse/YARN-10506
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Benjamin Teke
>Assignee: Andras Gyori
>Priority: Major
> Attachments: YARN-10506-006-10504-010.patch, 
> YARN-10506-007-10504-010.patch, YARN-10506-008.patch, YARN-10506-010.patch, 
> YARN-10506-012.patch, YARN-10506-013.patch, YARN-10506.001.patch, 
> YARN-10506.002.patch, YARN-10506.003.patch, YARN-10506.004.patch, 
> YARN-10506.005.patch, YARN-10506.006-combined.patch, YARN-10506.006.patch, 
> YARN-10506.007.patch, YARN-10506.009.patch, YARN-10506.011.patch, 
> YARN-10506.014.patch
>
>
> The queue creation logic should be updated to use weight mode and support the 
> flexible creation. 



--
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-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-14 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17265402#comment-17265402
 ] 

Gergely Pollak commented on YARN-10535:
---

The first iteration of the patch includes all modifications needed to be 
compliant with the changes being introduced in YARN-10506. This patch will not 
compile until YARN-10506 is merged, but I upload it for initial reviews. Also 
I'll add some new test cases for the new possible cases introduced in 
YARN-10506.

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Updated] (YARN-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-14 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10535:
--
Attachment: YARN-10535.001.patch

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10535.001.patch
>
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Updated] (YARN-10535) Make queue placement in CapacityScheduler compliant with auto-queue-placement

2021-01-14 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10535:
--
Summary: Make queue placement in CapacityScheduler compliant with 
auto-queue-placement  (was: Make changes in queue placement policy to be 
compliant with auto-queue-placement in CapacityScheduler)

> Make queue placement in CapacityScheduler compliant with auto-queue-placement
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Updated] (YARN-10535) Make changes in queue placement policy to be compliant with auto-queue-placement in CapacityScheduler

2021-01-14 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10535?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10535:
--
Summary: Make changes in queue placement policy to be compliant with 
auto-queue-placement in CapacityScheduler  (was: Make changes in queue 
placement policy to use auto-queue-placement API in CapacityScheduler)

> Make changes in queue placement policy to be compliant with 
> auto-queue-placement in CapacityScheduler
> -
>
> Key: YARN-10535
> URL: https://issues.apache.org/jira/browse/YARN-10535
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: capacity scheduler
>Reporter: Wangda Tan
>Assignee: Gergely Pollak
>Priority: Major
>
> Once YARN-10506 is done, we need to call the API from the queue placement 
> policy to create queues. 



--
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] [Comment Edited] (YARN-10506) Update queue creation logic to use weight mode and allow the flexible static/dynamic creation

2021-01-14 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17265208#comment-17265208
 ] 

Gergely Pollak edited comment on YARN-10506 at 1/14/21, 8:29 PM:
-

Thank you [~wangda] [~pbacsko] for your insights!

Actually the queue creation checks were present in the legacy engine 
([https://github.com/apache/hadoop/blob/9d5144e66d71551ad6e1a8d3f1670e49ba181999/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java#L333-L340]),
 if I see correctly since 3.0. But the first commit seems much older: 
[https://github.com/apache/hadoop/commit/0987a7b8c2c1e4c2095821d98a7db19644df]
 . 

I've just kept it during the refactor, and as Peter pointed out now we actually 
build on it, since we need to be able to determine if a rule will be executed 
successfully (ie. the queue can be created), and if it cannot we can move onto 
the next rule. This fallback behaviour is part of the FS Placement engine as 
well, so we need it in order to server FS customers, so it was somewhat lucky 
that it was already in place. (In the original implementation we just used this 
check to throw exceptions, which was quite pointless, since the CS would have 
rejected the application if the queue was uncreatable anyway)

So this is why I say we can safely remove those flags from the application 
placement context, this way we can reduce the code complexity.

This way we only need what [~pbacsko] mentioned:
{code:java}
1) "create" flag on the rule itself in the JSON - true/false
2) "yarh.scheduler.capacity..auto-queue-creation-v2.enabled" - 
true/false{code}
Also I'm currently adopting this change in the CSMappingPlacementRule, and I 
don't use these flags. What I'll need in the future a better, centralised way 
to determine if a path can be created, and both CS and the MappingEngine should 
use the same methods, to do it. Currently we are implementing the same logic 
differently, and it's much harder to maintain the code this way, but during the 
followup code cleanup jiras we'll take care of this.

About the race condition, I think (and I might be very wrong, because race 
conditions can be really sneaky), we cannot do much about, theoretically it can 
happen that the Mapping engine determines a rule can be executed, because the 
target queue exists or there is a proper dynamic parent, then a config change 
occurs, and by the time we get to the actual creation the queue structure 
changes, at this point the rejection is the proper action to take since the 
submission IS against the configuration. And I don't see a way around it since 
at the time the MappingEngine makes the decision that decision is the correct 
one, but we shouldn't enforce this decision if the configuration changes. But 
this race condition should only occur when the user changes the queue 
configuration, and in this case they should expect failing application 
submissions for the queues which have been changed or removed.


was (Author: shuzirra):
Actually the queue creation checks were present in the legacy engine 
(https://github.com/apache/hadoop/blob/9d5144e66d71551ad6e1a8d3f1670e49ba181999/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java#L333-L340),
 if I see correctly since 3.0. But the first commit seems much older: 
[https://github.com/apache/hadoop/commit/0987a7b8c2c1e4c2095821d98a7db19644df]
 . 

I've just kept it during the refactor, and as Peter pointed out now we actually 
build on it, since we need to be able to determine if a rule will be executed 
successfully (ie. the queue can be created), and if it cannot we can move onto 
the next rule. This fallback behaviour is part of the FS Placement engine as 
well, so we need it in order to server FS customers, so it was somewhat lucky 
that it was already in place. (In the original implementation we just used this 
check to throw exceptions, which was quite pointless, since the CS would have 
rejected the application if the queue was uncreatable anyway)

So this is why I say we can safely remove those flags from the application 
placement context, this way we can reduce the code complexity.

This way we only need what [~pbacsko] mentioned:
{code:java}
1) "create" flag on the rule itself in the JSON - true/false
2) "yarh.scheduler.capacity..auto-queue-creation-v2.enabled" - 
true/false{code}
Also I'm currently adopting this change in the CSMappingPlacementRule, and I 
don't use these flags. What I'll need in the future a better, centralised way 
to determine if a path can be created, and both CS and the MappingEngine should 
use the same methods, to do it. Currently we are implementing the same logic 

[jira] [Commented] (YARN-10506) Update queue creation logic to use weight mode and allow the flexible static/dynamic creation

2021-01-14 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17265208#comment-17265208
 ] 

Gergely Pollak commented on YARN-10506:
---

Actually the queue creation checks were present in the legacy engine 
(https://github.com/apache/hadoop/blob/9d5144e66d71551ad6e1a8d3f1670e49ba181999/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java#L333-L340),
 if I see correctly since 3.0. But the first commit seems much older: 
[https://github.com/apache/hadoop/commit/0987a7b8c2c1e4c2095821d98a7db19644df]
 . 

I've just kept it during the refactor, and as Peter pointed out now we actually 
build on it, since we need to be able to determine if a rule will be executed 
successfully (ie. the queue can be created), and if it cannot we can move onto 
the next rule. This fallback behaviour is part of the FS Placement engine as 
well, so we need it in order to server FS customers, so it was somewhat lucky 
that it was already in place. (In the original implementation we just used this 
check to throw exceptions, which was quite pointless, since the CS would have 
rejected the application if the queue was uncreatable anyway)

So this is why I say we can safely remove those flags from the application 
placement context, this way we can reduce the code complexity.

This way we only need what [~pbacsko] mentioned:
{code:java}
1) "create" flag on the rule itself in the JSON - true/false
2) "yarh.scheduler.capacity..auto-queue-creation-v2.enabled" - 
true/false{code}
Also I'm currently adopting this change in the CSMappingPlacementRule, and I 
don't use these flags. What I'll need in the future a better, centralised way 
to determine if a path can be created, and both CS and the MappingEngine should 
use the same methods, to do it. Currently we are implementing the same logic 
differently, and it's much harder to maintain the code this way, but during the 
followup code cleanup jiras we'll take care of this.

About the race condition, I think (and I might be very wrong, because race 
conditions can be really sneaky), we cannot do much about, theoretically it can 
happen that the Mapping engine determines a rule can be executed, because the 
target queue exists or there is a proper dynamic parent, then a config change 
occurs, and by the time we get to the actual creation the queue structure 
changes, at this point the rejection is the proper action to take since the 
submission IS against the configuration. And I don't see a way around it since 
at the time the MappingEngine makes the decision that decision is the correct 
one, but we shouldn't enforce this decision if the configuration changes. But 
this race condition should only occur when the user changes the queue 
configuration, and in this case they should expect failing application 
submissions for the queues which have been changed or removed.

> Update queue creation logic to use weight mode and allow the flexible 
> static/dynamic creation
> -
>
> Key: YARN-10506
> URL: https://issues.apache.org/jira/browse/YARN-10506
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Benjamin Teke
>Assignee: Andras Gyori
>Priority: Major
> Attachments: YARN-10506-006-10504-010.patch, 
> YARN-10506-007-10504-010.patch, YARN-10506-008.patch, YARN-10506-010.patch, 
> YARN-10506-012.patch, YARN-10506.001.patch, YARN-10506.002.patch, 
> YARN-10506.003.patch, YARN-10506.004.patch, YARN-10506.005.patch, 
> YARN-10506.006-combined.patch, YARN-10506.006.patch, YARN-10506.007.patch, 
> YARN-10506.009.patch, YARN-10506.011.patch
>
>
> The queue creation logic should be updated to use weight mode and support the 
> flexible creation. 



--
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] [Comment Edited] (YARN-10506) Update queue creation logic to use weight mode and allow the flexible static/dynamic creation

2021-01-14 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17264855#comment-17264855
 ] 

Gergely Pollak edited comment on YARN-10506 at 1/14/21, 3:46 PM:
-

Hi [~gandras] [~wangda], [~zhuqi]

*1) How we deal with "create" flag of ApplicationPlacementContext?* ** 

I don't think we need this flag at all. The CapacityScheduler has the 
information where can be dynamic queues created, it is supplied via the 
configuration option {{queue-path.auto-queue-creation-v2.enabled}}

and

{\{queue-path.auto-queue-creation.enabled }}

So at any point the CS will be aware if a supplied queue path is creatable. 
Mapping rules will return a path based on their configuration, but I don't 
think the placement rule should be overriding the CS configuration. This can 
cause some undesired issues, and we really don't get anything with it. It will 
be a little more inconvenient for the user since they have to configure the 
mapping rules AND the CS queues properly, but it doesn't look like a big deal.

On the other hand there are multiple places in the code which check if a queue 
can be created and all these places use the queue hierarchy to determine 
whether a queue is a managed parent, or not, however if the Mapping Rules are 
allowed to circumvent this, it might lead to unforeseen inconsistencies. (As a 
matter of fact the CSMappingPlacementRule currently uses the information from 
CS to determine if a queue can be created or not, so it wouldn't set any of 
these flags differently than CS is configured anyway)

 

So I would say let's omit the creation flags from the 
ApoplicationPlacementContext, let the CSMappingPlacementRule rely on the 
information provided by the CS about queue creation eligibility, and let it 
pass back a "regular" ApplicationPlacementContext with the desired queue path, 
then the CS will be able to create it if the configuration allows. This is a 
much easier and stable solution, while we don't really put extra administration 
overhead to the user, and also can be easily extended in the future.

As I see circumventing protections are never a good idea even if we are talking 
about internal interfaces, and these flags would exactly do that, they would 
force the CS to create queues which are against it's configuration.


was (Author: shuzirra):
Hi [~gandras] [~wangda], [~zhuqi] 

*1) How we deal with "create" flag of ApplicationPlacementContext?* ** 

I don't think we need this flag at all. The CapacityScheduler has the 
information where can be dynamic queues created, it is supplied via the 
configuration option {{queue-path.auto-queue-creation-v2.enabled}}

and

{\{queue-path.auto-queue-creation.enabled }}

So at any point the CS will be aware if a supplied queue path is creatable. 
Mapping rules will return a path based on their configuration, but I don't 
think the placement rule should be overriding the CS configuration. This can 
cause some undesired issues, and we really don't get anything with it. It will 
be a little more inconvenient for the user since they have to configure the 
mapping rules AND the CS queues properly, but it doesn't look like a big deal.

On the other hand there are multiple places in the code which check if a queue 
can be created and all these places use the queue hierarchy to determine 
whether a queue is a managed parent, or not, however if the Mapping Rules are 
allowed to circumvent this, it might lead to unforeseen inconsistencies. (As a 
matter of fact the CSMappingPlacementRule currently uses the information from 
CS to determine if a queue can be created or not, so it wouldn't set any of 
these flags differently then CS is configured anyway)

 

So I would say let's omit the creation flags from the 
ApoplicationPlacementContext, let the CSMappingPlacementRule rely on the 
information provided by the CS about queue creation eligibility, and let it 
pass back a "regular" ApplicationPlacementContext with the desired queue path, 
then the CS will be able to create it if the configuration allows. This is a 
much easier and stable solution, while we don't really put extra administration 
overhead to the user, and also can be easily extended in the future.

As I see circumventing protections are never a good idea even if we are talking 
about internal interfaces, and these flags would exactly do that, they would 
force the CS to create queues which are against it's configuration.

> Update queue creation logic to use weight mode and allow the flexible 
> static/dynamic creation
> -
>
> Key: YARN-10506
> URL: https://issues.apache.org/jira/browse/YARN-10506
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Benjamin Teke
>Assignee: Andras Gyori
>

[jira] [Comment Edited] (YARN-10506) Update queue creation logic to use weight mode and allow the flexible static/dynamic creation

2021-01-14 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17264855#comment-17264855
 ] 

Gergely Pollak edited comment on YARN-10506 at 1/14/21, 3:46 PM:
-

Hi [~gandras] [~wangda], [~zhuqi]

*1) How we deal with "create" flag of ApplicationPlacementContext?* ** 

I don't think we need this flag at all. The CapacityScheduler has the 
information where can be dynamic queues created, it is supplied via the 
configuration option {{queue-path.auto-queue-creation-v2.enabled}}

and

{\{queue-path.auto-queue-creation.enabled }}

So at any point the CS will be aware if a supplied queue path is creatable. 
Mapping rules will return a path based on their configuration, but I don't 
think the placement rule should be overriding the CS configuration. This can 
cause some undesired issues, and we really don't get anything with it. It will 
be a little more inconvenient for the user since they have to configure the 
mapping rules AND the CS queues properly, but it doesn't look like a big deal.

On the other hand there are multiple places in the code which check if a queue 
can be created and all these places use the queue hierarchy to determine 
whether a queue is a managed parent, or not, however if the Mapping Rules are 
allowed to circumvent this, it might lead to unforeseen inconsistencies. (As a 
matter of fact the CSMappingPlacementRule currently uses the information from 
CS to determine if a queue can be created or not, so it wouldn't set any of 
these flags differently than CS is configured anyway)

 

So I would say let's omit the creation flags from the 
ApplicationPlacementContext, let the CSMappingPlacementRule rely on the 
information provided by the CS about queue creation eligibility, and let it 
pass back a "regular" ApplicationPlacementContext with the desired queue path, 
then the CS will be able to create it if the configuration allows. This is a 
much easier and stable solution, while we don't really put extra administration 
overhead to the user, and also can be easily extended in the future.

As I see circumventing protections are never a good idea even if we are talking 
about internal interfaces, and these flags would exactly do that, they would 
force the CS to create queues which are against it's configuration.


was (Author: shuzirra):
Hi [~gandras] [~wangda], [~zhuqi]

*1) How we deal with "create" flag of ApplicationPlacementContext?* ** 

I don't think we need this flag at all. The CapacityScheduler has the 
information where can be dynamic queues created, it is supplied via the 
configuration option {{queue-path.auto-queue-creation-v2.enabled}}

and

{\{queue-path.auto-queue-creation.enabled }}

So at any point the CS will be aware if a supplied queue path is creatable. 
Mapping rules will return a path based on their configuration, but I don't 
think the placement rule should be overriding the CS configuration. This can 
cause some undesired issues, and we really don't get anything with it. It will 
be a little more inconvenient for the user since they have to configure the 
mapping rules AND the CS queues properly, but it doesn't look like a big deal.

On the other hand there are multiple places in the code which check if a queue 
can be created and all these places use the queue hierarchy to determine 
whether a queue is a managed parent, or not, however if the Mapping Rules are 
allowed to circumvent this, it might lead to unforeseen inconsistencies. (As a 
matter of fact the CSMappingPlacementRule currently uses the information from 
CS to determine if a queue can be created or not, so it wouldn't set any of 
these flags differently than CS is configured anyway)

 

So I would say let's omit the creation flags from the 
ApoplicationPlacementContext, let the CSMappingPlacementRule rely on the 
information provided by the CS about queue creation eligibility, and let it 
pass back a "regular" ApplicationPlacementContext with the desired queue path, 
then the CS will be able to create it if the configuration allows. This is a 
much easier and stable solution, while we don't really put extra administration 
overhead to the user, and also can be easily extended in the future.

As I see circumventing protections are never a good idea even if we are talking 
about internal interfaces, and these flags would exactly do that, they would 
force the CS to create queues which are against it's configuration.

> Update queue creation logic to use weight mode and allow the flexible 
> static/dynamic creation
> -
>
> Key: YARN-10506
> URL: https://issues.apache.org/jira/browse/YARN-10506
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Benjamin Teke
>Assignee: Andras Gyori
>Priority: 

[jira] [Comment Edited] (YARN-10506) Update queue creation logic to use weight mode and allow the flexible static/dynamic creation

2021-01-14 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17264855#comment-17264855
 ] 

Gergely Pollak edited comment on YARN-10506 at 1/14/21, 12:15 PM:
--

Hi [~gandras] [~wangda], [~zhuqi] 

*1) How we deal with "create" flag of ApplicationPlacementContext?* ** 

I don't think we need this flag at all. The CapacityScheduler has the 
information where can be dynamic queues created, it is supplied via the 
configuration option {{queue-path.auto-queue-creation-v2.enabled}}

and

{\{queue-path.auto-queue-creation.enabled }}

So at any point the CS will be aware if a supplied queue path is creatable. 
Mapping rules will return a path based on their configuration, but I don't 
think the placement rule should be overriding the CS configuration. This can 
cause some undesired issues, and we really don't get anything with it. It will 
be a little more inconvenient for the user since they have to configure the 
mapping rules AND the CS queues properly, but it doesn't look like a big deal.

On the other hand there are multiple places in the code which check if a queue 
can be created and all these places use the queue hierarchy to determine 
whether a queue is a managed parent, or not, however if the Mapping Rules are 
allowed to circumvent this, it might lead to unforeseen inconsistencies. (As a 
matter of fact the CSMappingPlacementRule currently uses the information from 
CS to determine if a queue can be created or not, so it wouldn't set any of 
these flags differently then CS is configured anyway)

 

So I would say let's omit the creation flags from the 
ApoplicationPlacementContext, let the CSMappingPlacementRule rely on the 
information provided by the CS about queue creation eligibility, and let it 
pass back a "regular" ApplicationPlacementContext with the desired queue path, 
then the CS will be able to create it if the configuration allows. This is a 
much easier and stable solution, while we don't really put extra administration 
overhead to the user, and also can be easily extended in the future.

As I see circumventing protections are never a good idea even if we are talking 
about internal interfaces, and these flags would exactly do that, they would 
force the CS to create queues which are against it's configuration.


was (Author: shuzirra):
Hi [~gandras] [~wangda],

*1) How we deal with "create" flag of ApplicationPlacementContext?* ** 

I don't think we need this flag at all. The CapacityScheduler has the 
information where can be dynamic queues created, it is supplied via the 
configuration option {{queue-path.auto-queue-creation-v2.enabled}}

and

{{queue-path.auto-queue-creation.enabled }}

So at any point the CS will be aware if a supplied queue path is creatable. 
Mapping rules will return a path based on their configuration, but I don't 
think the placement rule should be overriding the CS configuration. This can 
cause some undesired issues, and we really don't get anything with it. It will 
be a little more inconvenient for the user since they have to configure the 
mapping rules AND the CS queues properly, but it doesn't look like a big deal.

On the other hand there are multiple places in the code which check if a queue 
can be created and all these places use the queue hierarchy to determine 
whether a queue is a managed parent, or not, however if the Mapping Rules are 
allowed to circumvent this, it might lead to unforeseen inconsistencies. (As a 
matter of fact the CSMappingPlacementRule currently uses the information from 
CS to determine if a queue can be created or not, so it wouldn't set any of 
these flags differently then CS is configured anyway)

 

So I would say let's omit the creation flags from the 
ApoplicationPlacementContext, let the CSMappingPlacementRule rely on the 
information provided by the CS about queue creation eligibility, and let it 
pass back a "regular" ApplicationPlacementContext with the desired queue path, 
then the CS will be able to create it if the configuration allows. This is a 
much easier and stable solution, while we don't really put extra administration 
overhead to the user, and also can be easily extended in the future.

As I see circumventing protections are never a good idea even if we are talking 
about internal interfaces, and these flags would exactly do that, they would 
force the CS to create queues which are against it's configuration.

> Update queue creation logic to use weight mode and allow the flexible 
> static/dynamic creation
> -
>
> Key: YARN-10506
> URL: https://issues.apache.org/jira/browse/YARN-10506
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Benjamin Teke
>Assignee: Andras Gyori
>Priority: Major

[jira] [Commented] (YARN-10506) Update queue creation logic to use weight mode and allow the flexible static/dynamic creation

2021-01-14 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17264855#comment-17264855
 ] 

Gergely Pollak commented on YARN-10506:
---

Hi [~gandras] [~wangda],

*1) How we deal with "create" flag of ApplicationPlacementContext?* ** 

I don't think we need this flag at all. The CapacityScheduler has the 
information where can be dynamic queues created, it is supplied via the 
configuration option {{queue-path.auto-queue-creation-v2.enabled}}

and

{{queue-path.auto-queue-creation.enabled }}

So at any point the CS will be aware if a supplied queue path is creatable. 
Mapping rules will return a path based on their configuration, but I don't 
think the placement rule should be overriding the CS configuration. This can 
cause some undesired issues, and we really don't get anything with it. It will 
be a little more inconvenient for the user since they have to configure the 
mapping rules AND the CS queues properly, but it doesn't look like a big deal.

On the other hand there are multiple places in the code which check if a queue 
can be created and all these places use the queue hierarchy to determine 
whether a queue is a managed parent, or not, however if the Mapping Rules are 
allowed to circumvent this, it might lead to unforeseen inconsistencies. (As a 
matter of fact the CSMappingPlacementRule currently uses the information from 
CS to determine if a queue can be created or not, so it wouldn't set any of 
these flags differently then CS is configured anyway)

 

So I would say let's omit the creation flags from the 
ApoplicationPlacementContext, let the CSMappingPlacementRule rely on the 
information provided by the CS about queue creation eligibility, and let it 
pass back a "regular" ApplicationPlacementContext with the desired queue path, 
then the CS will be able to create it if the configuration allows. This is a 
much easier and stable solution, while we don't really put extra administration 
overhead to the user, and also can be easily extended in the future.

As I see circumventing protections are never a good idea even if we are talking 
about internal interfaces, and these flags would exactly do that, they would 
force the CS to create queues which are against it's configuration.

> Update queue creation logic to use weight mode and allow the flexible 
> static/dynamic creation
> -
>
> Key: YARN-10506
> URL: https://issues.apache.org/jira/browse/YARN-10506
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Benjamin Teke
>Assignee: Andras Gyori
>Priority: Major
> Attachments: YARN-10506-006-10504-010.patch, 
> YARN-10506-007-10504-010.patch, YARN-10506-008.patch, YARN-10506-010.patch, 
> YARN-10506-012.patch, YARN-10506.001.patch, YARN-10506.002.patch, 
> YARN-10506.003.patch, YARN-10506.004.patch, YARN-10506.005.patch, 
> YARN-10506.006-combined.patch, YARN-10506.006.patch, YARN-10506.007.patch, 
> YARN-10506.009.patch, YARN-10506.011.patch
>
>
> The queue creation logic should be updated to use weight mode and support the 
> flexible creation. 



--
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-10526) RMAppManager CS Placement ignores parent path

2020-12-14 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10526?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17249001#comment-17249001
 ] 

Gergely Pollak commented on YARN-10526:
---

Latest version is supposed to be final, added a test and a few comments. Also 
please ignore the fail in TestDelegationTokenRenewer, it seems to be flaky and 
is unrelated.

> RMAppManager CS Placement ignores parent path
> -
>
> Key: YARN-10526
> URL: https://issues.apache.org/jira/browse/YARN-10526
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10526.001.patch, YARN-10526.002.patch, 
> YARN-10526.003.patch, YARN-10526.004.patch
>
>
> When RMAppManager creates the RMApp object using the placementContext's 
> results, it only uses the getQueue method which will return only the name of 
> the leaf queue in the case of CapacityScheduler.
> If a queue exists with this name, then the application will be placed into 
> the queue. If the queue does not exists, then CS will take the parent path 
> into consideration during the auto queue creation, however this only happens 
> if there is no queue with the leaf name.
>  



--
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] [Updated] (YARN-10526) RMAppManager CS Placement ignores parent path

2020-12-14 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10526?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10526:
--
Attachment: YARN-10526.004.patch

> RMAppManager CS Placement ignores parent path
> -
>
> Key: YARN-10526
> URL: https://issues.apache.org/jira/browse/YARN-10526
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10526.001.patch, YARN-10526.002.patch, 
> YARN-10526.003.patch, YARN-10526.004.patch
>
>
> When RMAppManager creates the RMApp object using the placementContext's 
> results, it only uses the getQueue method which will return only the name of 
> the leaf queue in the case of CapacityScheduler.
> If a queue exists with this name, then the application will be placed into 
> the queue. If the queue does not exists, then CS will take the parent path 
> into consideration during the auto queue creation, however this only happens 
> if there is no queue with the leaf name.
>  



--
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] [Updated] (YARN-10526) RMAppManager CS Placement ignores parent path

2020-12-11 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10526?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10526:
--
Attachment: YARN-10526.003.patch

> RMAppManager CS Placement ignores parent path
> -
>
> Key: YARN-10526
> URL: https://issues.apache.org/jira/browse/YARN-10526
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10526.001.patch, YARN-10526.002.patch, 
> YARN-10526.003.patch
>
>
> When RMAppManager creates the RMApp object using the placementContext's 
> results, it only uses the getQueue method which will return only the name of 
> the leaf queue in the case of CapacityScheduler.
> If a queue exists with this name, then the application will be placed into 
> the queue. If the queue does not exists, then CS will take the parent path 
> into consideration during the auto queue creation, however this only happens 
> if there is no queue with the leaf name.
>  



--
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] [Updated] (YARN-10526) RMAppManager CS Placement ignores parent path

2020-12-11 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10526?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10526:
--
Attachment: YARN-10526.002.patch

> RMAppManager CS Placement ignores parent path
> -
>
> Key: YARN-10526
> URL: https://issues.apache.org/jira/browse/YARN-10526
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10526.001.patch, YARN-10526.002.patch
>
>
> When RMAppManager creates the RMApp object using the placementContext's 
> results, it only uses the getQueue method which will return only the name of 
> the leaf queue in the case of CapacityScheduler.
> If a queue exists with this name, then the application will be placed into 
> the queue. If the queue does not exists, then CS will take the parent path 
> into consideration during the auto queue creation, however this only happens 
> if there is no queue with the leaf name.
>  



--
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] [Updated] (YARN-10526) RMAppManager CS Placement ignores parent path

2020-12-09 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10526?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10526:
--
Attachment: YARN-10526.001.patch

> RMAppManager CS Placement ignores parent path
> -
>
> Key: YARN-10526
> URL: https://issues.apache.org/jira/browse/YARN-10526
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10526.001.patch
>
>
> When RMAppManager creates the RMApp object using the placementContext's 
> results, it only uses the getQueue method which will return only the name of 
> the leaf queue in the case of CapacityScheduler.
> If a queue exists with this name, then the application will be placed into 
> the queue. If the queue does not exists, then CS will take the parent path 
> into consideration during the auto queue creation, however this only happens 
> if there is no queue with the leaf name.
>  



--
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] [Created] (YARN-10526) RMAppManager CS Placement ignores parent path

2020-12-09 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10526:
-

 Summary: RMAppManager CS Placement ignores parent path
 Key: YARN-10526
 URL: https://issues.apache.org/jira/browse/YARN-10526
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak
Assignee: Gergely Pollak


When RMAppManager creates the RMApp object using the placementContext's 
results, it only uses the getQueue method which will return only the name of 
the leaf queue in the case of CapacityScheduler.

If a queue exists with this name, then the application will be placed into the 
queue. If the queue does not exists, then CS will take the parent path into 
consideration during the auto queue creation, however this only happens if 
there is no queue with the leaf name.

 



--
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

2020-11-30 Thread Gergely Pollak (Jira)


[ 
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-10497) Fix an issue in CapacityScheduler which fails to delete queues

2020-11-25 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10497?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17238683#comment-17238683
 ] 

Gergely Pollak commented on YARN-10497:
---

[~wangda] you didn't use getStringCollection, but the method which is 
responsible for the root cause did. The original implementation of 
getSiblingQueues used getStringCollection instead of 
getTrimmedStringCollection, and this caused the trimming issue. So if we only 
update that method to use getTrimmedStringCollection, then the issue would be 
fixed I think.

> Fix an issue in CapacityScheduler which fails to delete queues
> --
>
> Key: YARN-10497
> URL: https://issues.apache.org/jira/browse/YARN-10497
> Project: Hadoop YARN
>  Issue Type: Improvement
>Reporter: Wangda Tan
>Assignee: Wangda Tan
>Priority: Major
> Attachments: YARN-10497.001.patch, YARN-10497.002.patch, 
> YARN-10497.003.patch
>
>
> We saw an exception when using queue mutation APIs:
> {code:java}
> 2020-11-13 16:47:46,327 WARN 
> org.apache.hadoop.yarn.server.resourcemanager.webapp.RMWebServices: 
> CapacityScheduler configuration validation failed:java.io.IOException: Queue 
> root.am2cmQueueSecond not found
> {code}
> Which comes from this code:
> {code:java}
> List siblingQueues = getSiblingQueues(queueToRemove,
> proposedConf);
> if (!siblingQueues.contains(queueName)) {
>   throw new IOException("Queue " + queueToRemove + " not found");
> } 
> {code}
> (Inside MutableCSConfigurationProvider)
> If you look at the method:
> {code:java}
>  
>   private List getSiblingQueues(String queuePath, Configuration conf) 
> {
> String parentQueue = queuePath.substring(0, queuePath.lastIndexOf('.'));
> String childQueuesKey = CapacitySchedulerConfiguration.PREFIX +
> parentQueue + CapacitySchedulerConfiguration.DOT +
> CapacitySchedulerConfiguration.QUEUES;
> return new ArrayList<>(conf.getStringCollection(childQueuesKey));
>   }
> {code}
> And here's capacity-scheduler.xml I got
> {code:java}
> yarn.scheduler.capacity.root.queuesdefault, q1, 
> q2
> {code}
> You can notice there're spaces between default, q1, a2
> So conf.getStringCollection returns:
> {code:java}
> default
> q1
> ...
> {code}
> Which causes match issue when we try to delete the queue.



--
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-10497) Fix an issue in CapacityScheduler which fails to delete queues

2020-11-23 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10497?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17237479#comment-17237479
 ] 

Gergely Pollak commented on YARN-10497:
---

[~wangda]Thank you for the patch!

There is a Configuration#getTrimmedStringCollection, which could be used in 
MutableCSConfigurationProvider#getSiblingQueues instead of the 
getStringCollection call, reducing the whole change to 1 line. However that 
uses some regexp magic for trimming, which might be not the most effective way, 
but I guess this is not time critical part of the code, since the most of the 
load won't come from queue deletion.

 

> Fix an issue in CapacityScheduler which fails to delete queues
> --
>
> Key: YARN-10497
> URL: https://issues.apache.org/jira/browse/YARN-10497
> Project: Hadoop YARN
>  Issue Type: Improvement
>Reporter: Wangda Tan
>Assignee: Wangda Tan
>Priority: Major
> Attachments: YARN-10497.001.patch
>
>
> We saw an exception when using queue mutation APIs:
> {code:java}
> 2020-11-13 16:47:46,327 WARN 
> org.apache.hadoop.yarn.server.resourcemanager.webapp.RMWebServices: 
> CapacityScheduler configuration validation failed:java.io.IOException: Queue 
> root.am2cmQueueSecond not found
> {code}
> Which comes from this code:
> {code:java}
> List siblingQueues = getSiblingQueues(queueToRemove,
> proposedConf);
> if (!siblingQueues.contains(queueName)) {
>   throw new IOException("Queue " + queueToRemove + " not found");
> } 
> {code}
> (Inside MutableCSConfigurationProvider)
> If you look at the method:
> {code:java}
>  
>   private List getSiblingQueues(String queuePath, Configuration conf) 
> {
> String parentQueue = queuePath.substring(0, queuePath.lastIndexOf('.'));
> String childQueuesKey = CapacitySchedulerConfiguration.PREFIX +
> parentQueue + CapacitySchedulerConfiguration.DOT +
> CapacitySchedulerConfiguration.QUEUES;
> return new ArrayList<>(conf.getStringCollection(childQueuesKey));
>   }
> {code}
> And here's capacity-scheduler.xml I got
> {code:java}
> yarn.scheduler.capacity.root.queuesdefault, q1, 
> q2
> {code}
> You can notice there're spaces between default, q1, a2
> So conf.getStringCollection returns:
> {code:java}
> default
> q1
> ...
> {code}
> Which causes match issue when we try to delete the queue.



--
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] [Updated] (YARN-10457) Add a configuration switch to change between legacy and JSON placement rule format

2020-11-17 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10457?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10457:
--
Attachment: YARN-10457.002.patch

> Add a configuration switch to change between legacy and JSON placement rule 
> format
> --
>
> Key: YARN-10457
> URL: https://issues.apache.org/jira/browse/YARN-10457
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10457.001.patch, YARN-10457.002.patch
>
>




--
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-7200) SLS generates a realtimetrack.json file but that file is missing the closing ']'

2020-11-16 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-7200?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17232862#comment-17232862
 ] 

Gergely Pollak commented on YARN-7200:
--

[~akshink] It does matter.

1) You break backwards compatibility, since you cannot guarantee no one uses 
multiple instances of the same class. Eg 2 CapacitySchedulerMetrics objects.

2) The class is not singleton, there are no indication that only one instance 
should exist (based on its name, actually it might make sense to have multiple 
instances), and you were able to use multiple instances, until this change. 

3) The SLSCapacityScheduler#setConf creates a new instance of this class, so 
there might be a code path, which results in multiple instances.

4) You can access the metrics class via
{code:java}
SchedulerWrapper wrapper = (SchedulerWrapper)rm.getResourceScheduler();
SchedulerMetrics metrics = wrapper.getSchedulerMetrics();
{code}

> SLS generates a realtimetrack.json file but that file is missing the closing 
> ']'
> 
>
> Key: YARN-7200
> URL: https://issues.apache.org/jira/browse/YARN-7200
> Project: Hadoop YARN
>  Issue Type: Bug
>  Components: scheduler-load-simulator
>Reporter: Grant Sohn
>Assignee: Agshin Kazimli
>Priority: Minor
>  Labels: newbie, newbie++
> Attachments: YARN-7200-branch-trunk.patch, realtimetrack.json, 
> snemeth-testing-20201113.zip
>
>
> File 
> hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/scheduler/SchedulerMetrics.java
>  shows:
> {noformat}
>   void tearDown() throws Exception {
> if (metricsLogBW != null)  {
>   metricsLogBW.write("]");
>   metricsLogBW.close();
> }
> 
> {noformat}
> So the exit logic is flawed.



--
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-7200) SLS generates a realtimetrack.json file but that file is missing the closing ']'

2020-11-16 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-7200?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17232770#comment-17232770
 ] 

Gergely Pollak commented on YARN-7200:
--

[~akshink] Thank you for the patch.

I'm not entirely aware of the context, but I don't really like the static 
BufferedWriter. It is a call for error if anyone wants to create two instances 
of the same class. Also please note: this is an abstract class, which further 
increases the chance to have multiple classes which extends this class at the 
same time, and all will use the same buffer. 

> SLS generates a realtimetrack.json file but that file is missing the closing 
> ']'
> 
>
> Key: YARN-7200
> URL: https://issues.apache.org/jira/browse/YARN-7200
> Project: Hadoop YARN
>  Issue Type: Bug
>  Components: scheduler-load-simulator
>Reporter: Grant Sohn
>Assignee: Agshin Kazimli
>Priority: Minor
>  Labels: newbie, newbie++
> Attachments: YARN-7200-branch-trunk.patch, realtimetrack.json, 
> snemeth-testing-20201113.zip
>
>
> File 
> hadoop-tools/hadoop-sls/src/main/java/org/apache/hadoop/yarn/sls/scheduler/SchedulerMetrics.java
>  shows:
> {noformat}
>   void tearDown() throws Exception {
> if (metricsLogBW != null)  {
>   metricsLogBW.write("]");
>   metricsLogBW.close();
> }
> 
> {noformat}
> So the exit logic is flawed.



--
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] [Updated] (YARN-10425) Replace the legacy placement engine in CS with the new one

2020-11-10 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10425:
--
Attachment: YARN-10425.007.patch

> 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

2020-11-10 Thread Gergely Pollak (Jira)


[ 
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] [Updated] (YARN-10425) Replace the legacy placement engine in CS with the new one

2020-11-10 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10425:
--
Attachment: YARN-10425.006.patch

> 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

2020-10-30 Thread Gergely Pollak (Jira)


[ 
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] [Updated] (YARN-10425) Replace the legacy placement engine in CS with the new one

2020-10-30 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10425:
--
Attachment: YARN-10425.005.patch

> 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

2020-10-29 Thread Gergely Pollak (Jira)


[ 
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] [Updated] (YARN-10425) Replace the legacy placement engine in CS with the new one

2020-10-29 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10425:
--
Attachment: YARN-10425.004.patch

> 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

2020-10-28 Thread Gergely Pollak (Jira)


[ 
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] [Updated] (YARN-10425) Replace the legacy placement engine in CS with the new one

2020-10-28 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10425:
--
Attachment: YARN-10425.003.patch

> 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

2020-10-27 Thread Gergely Pollak (Jira)


[ 
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] [Updated] (YARN-10425) Replace the legacy placement engine in CS with the new one

2020-10-27 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10425:
--
Attachment: YARN-10425.002.patch

> 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] [Assigned] (YARN-10457) Add a configuration switch to change between legacy and JSON placement rule format.

2020-10-12 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10457?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak reassigned YARN-10457:
-

Assignee: Gergely Pollak

> Add a configuration switch to change between legacy and JSON placement rule 
> format.
> ---
>
> Key: YARN-10457
> URL: https://issues.apache.org/jira/browse/YARN-10457
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
>




--
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] [Created] (YARN-10457) Add a configuration switch to change between legacy and JSON placement rule format.

2020-10-12 Thread Gergely Pollak (Jira)
Gergely Pollak created YARN-10457:
-

 Summary: Add a configuration switch to change between legacy and 
JSON placement rule format.
 Key: YARN-10457
 URL: https://issues.apache.org/jira/browse/YARN-10457
 Project: Hadoop YARN
  Issue Type: Sub-task
Reporter: Gergely Pollak






--
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] [Updated] (YARN-10425) Replace the legacy placement engine in CS with the new one

2020-10-07 Thread Gergely Pollak (Jira)


 [ 
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Gergely Pollak updated YARN-10425:
--
Attachment: YARN-10425.001.patch

> 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-10413) Change fs2cs to generate mapping rules in the new format

2020-09-22 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10413?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17200070#comment-17200070
 ] 

Gergely Pollak commented on YARN-10413:
---

[~pbacsko] thank you for the update. LGTM+1 (Non-binding)

> Change fs2cs to generate mapping rules in the new format
> 
>
> Key: YARN-10413
> URL: https://issues.apache.org/jira/browse/YARN-10413
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-10413-001.patch, YARN-10413-002.patch, 
> YARN-10413-003.patch, YARN-10413-004.patch, YARN-10413-005.patch
>
>
> Currently, the converter tool {{fs2cs}} can convert placement rules to 
> mapping rules, but the differences are too big.
> It should be modified to generate placement rules to the new engine and 
> output it to a separate JSON file.



--
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-10413) Change fs2cs to generate mapping rules in the new format

2020-09-18 Thread Gergely Pollak (Jira)


[ 
https://issues.apache.org/jira/browse/YARN-10413?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17198361#comment-17198361
 ] 

Gergely Pollak commented on YARN-10413:
---

Thank you [~pbacsko] for the patch, I don't see any major issues, conversion 
rules seem solid.

I think the default rule conversion is a bit more complex than it should be. 
You change the default to X, create a rule to place to the new default (X), and 
then change the default back to its original value. It is technically 
equivalent of a placement rule to X without having to change the default. So we 
might reduce the confusion and the number of rules. But please correct me if 
some FS specific detail have eluded me, because I'm not super familiar with the 
default handing in FS nested rules.

Besides this I've found only two minor things, which you might want to double 
check:

1) QueuePlacementConverter#handleNestedRule The rule creations and additions 
are very similar, and the fail branch returns from the method anyway, so you 
might consider moving the rule creation, and addition to the list to the end of 
the method, since all paths use the same arguments except for policy and 
default, which can be passed through variables, it would reduce the size of the 
method, and for me it would be a bit more readable, but it's not a big deal.

2) QueuePlacementConverter#createNestedRule and 
QueuePlacementConverter#createRule are very similar, with some very similar 
handling position, I was wondering if it could be restructured in a way that 
createNestedRule uses createRule, or both use a same common method for the 
similar parts, again it's not a big deal, bit less code, and perhaps a bit more 
clarity.

 

 

> Change fs2cs to generate mapping rules in the new format
> 
>
> Key: YARN-10413
> URL: https://issues.apache.org/jira/browse/YARN-10413
> Project: Hadoop YARN
>  Issue Type: Sub-task
>Reporter: Peter Bacsko
>Assignee: Peter Bacsko
>Priority: Major
> Attachments: YARN-10413-001.patch, YARN-10413-002.patch, 
> YARN-10413-003.patch, YARN-10413-004.patch
>
>
> Currently, the converter tool {{fs2cs}} can convert placement rules to 
> mapping rules, but the differences are too big.
> It should be modified to generate placement rules to the new engine and 
> output it to a separate JSON file.



--
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



  1   2   3   4   >