[jira] [Comment Edited] (YARN-10372) Create MappingRule class to represent each CS mapping rule

2020-09-03 Thread Peter Bacsko (Jira)


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

Peter Bacsko edited comment on YARN-10372 at 9/3/20, 1:08 PM:
--

Thanks [~shuzirra] for the patch and [~snemeth] for the review, changed has 
been submitted to trunk.


was (Author: pbacsko):
Thanks [~shuzirra], changed has been submitted to trunk.

> Create MappingRule class to represent each CS mapping rule
> --
>
> Key: YARN-10372
> URL: https://issues.apache.org/jira/browse/YARN-10372
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: yarn
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Fix For: 3.4.0
>
> Attachments: YARN-10372.001.patch, YARN-10372.002.patch, 
> YARN-10372.003.patch, YARN-10372.004.patch
>
>
> As per the design document attached to the umbrella Jira (YARN-10370), we 
> need a class which represents a mapping rule, which can be evaluated, and 
> determines if the rule applies to the current application placement and 
> determines the action to be taken.



--
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-10372) Create MappingRule class to represent each CS mapping rule

2020-09-01 Thread Peter Bacsko (Jira)


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

Peter Bacsko edited comment on YARN-10372 at 9/1/20, 7:38 PM:
--

[~shuzirra] please fix the indentation-related warnings.

Also please do the following:
1. In {{createLegacyRule()}}, it is important to call 
{{action.setFallbackDefaultPlacement()}} to ensure proper backward compatibility
2. Add javadoc to {{createLegacyRule()}} methods to describe what "legacy" 
means in this context
3. Consider extending the unit tests which verifies the fallback setting


was (Author: pbacsko):
[~shuzirra] please fix the indentation-related warnings.

Also, in {{createLegacyRule()}}, it is important to call 
{{action.setFallbackDefaultPlacement()}} to ensure proper backward 
compatibility.

> Create MappingRule class to represent each CS mapping rule
> --
>
> Key: YARN-10372
> URL: https://issues.apache.org/jira/browse/YARN-10372
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: yarn
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10372.001.patch, YARN-10372.002.patch
>
>
> As per the design document attached to the umbrella Jira (YARN-10370), we 
> need a class which represents a mapping rule, which can be evaluated, and 
> determines if the rule applies to the current application placement and 
> determines the action to be taken.



--
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-10372) Create MappingRule class to represent each CS mapping rule

2020-09-01 Thread Peter Bacsko (Jira)


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

Peter Bacsko edited comment on YARN-10372 at 9/1/20, 7:35 PM:
--

[~shuzirra] please fix the indentation-related warnings.

Also, in {{createLegacyRule()}}, it is important to call 
{{action.setFallbackDefaultPlacement()}} to ensure proper backward 
compatibility.


was (Author: pbacsko):
[~shuzirra] please fix the indentation-related warnings.

> Create MappingRule class to represent each CS mapping rule
> --
>
> Key: YARN-10372
> URL: https://issues.apache.org/jira/browse/YARN-10372
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: yarn
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10372.001.patch, YARN-10372.002.patch
>
>
> As per the design document attached to the umbrella Jira (YARN-10370), we 
> need a class which represents a mapping rule, which can be evaluated, and 
> determines if the rule applies to the current application placement and 
> determines the action to be taken.



--
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-10372) Create MappingRule class to represent each CS mapping rule

2020-08-26 Thread Peter Bacsko (Jira)


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

Peter Bacsko edited comment on YARN-10372 at 8/26/20, 2:12 PM:
---

In this code:

{noformat}
  public static MappingRule createLegacyRule(
  String type, String source, String path) {
MappingRuleMatcher matcher;
MappingRuleAction action = new 
MappingRuleActions.PlaceToQueueAction(path);
switch (type) {
  case "u":
matcher = MappingRuleMatchers.createUserMatcher(source);
  break;
  case "g":
matcher = MappingRuleMatchers.createGroupMatcher(source);
  break;
  default:
matcher = 
MappingRuleMatchers.createApplicationNameMatcher(source);
}
{noformat}

I would definitely add a separate case for "a" and then fail in in the 
{{default}} branch with {{IllegalArgumentException}}. This adds an extra level 
of safety net.

It's perhaps even better to extract "u", "g" and "a" into final String 
constants instead of using them as literals (this also applies to YARN-10375).


was (Author: pbacsko):
In this code:

{noformat}
  public static MappingRule createLegacyRule(
  String type, String source, String path) {
MappingRuleMatcher matcher;
MappingRuleAction action = new 
MappingRuleActions.PlaceToQueueAction(path);
switch (type) {
  case "u":
matcher = MappingRuleMatchers.createUserMatcher(source);
  break;
  case "g":
matcher = MappingRuleMatchers.createGroupMatcher(source);
  break;
  default:
matcher = 
MappingRuleMatchers.createApplicationNameMatcher(source);
}
{noformat}

I would definitely add a separate case for "a" and then fail in in the 
{{default}} branch with {{IllegalArgumentException}}. This adds an extra level 
of safety net.

> Create MappingRule class to represent each CS mapping rule
> --
>
> Key: YARN-10372
> URL: https://issues.apache.org/jira/browse/YARN-10372
> Project: Hadoop YARN
>  Issue Type: Sub-task
>  Components: yarn
>Reporter: Gergely Pollak
>Assignee: Gergely Pollak
>Priority: Major
> Attachments: YARN-10372.001.patch
>
>
> As per the design document attached to the umbrella Jira (YARN-10370), we 
> need a class which represents a mapping rule, which can be evaluated, and 
> determines if the rule applies to the current application placement and 
> determines the action to be taken.



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