[
https://issues.apache.org/jira/browse/YARN-10374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17187844#comment-17187844
]
Gergely Pollak commented on YARN-10374:
---------------------------------------
Test failure is unrelated, fixed checkstyle warnings.
[~pbacsko]
4. {{setFallbackReject()}} / {{setFallbackSkip()}} /
{{setFallbackDefaultPlacement()}} – these methods all return
{{MappingRuleActionBase}}. In the implementation I can see it's because they
all return "this", but is chained invocation ever used? It's also weird to see
in {{MappingRuleAction}} that {{MappingRuleActionBase}} is returned, which is
the actual implementation of the interface. I suggest making these methods void.
Changed the interface to return {{MappingRuleAction, }}{{MappingRuleActionBase
was weird indeed. Currently it is not used since it is a convenience solution
for the parser, and those classes which create actions. But it can be used if
needed. Added documentation for the returns.}}{{}}
6. {{MappingRuleResult}}: there are separate methods for returning
{{RESULT_REJECT}}, {{RESULT_SKIP}}, {{RESULT_DEFAULT_PLACEMENT}}. They're also
called {{create*}} but they don't really create anything. I think it's simpler
to just make these constants public.
This is for consistency's sake. All helper methods are called createXXX, with
the current implementation REJECT/SKIP/DEFAULT don't have to be created each
time, so to save up some minimal GC time I create the constants. But it is the
internal behaviour of the MappingRuleResult, external callers should NOT rely
on REJECT object is always being the same, this might change in the future,
thats why I've created a separate interface with the createXXX methods, and
just return the constants. Should the implementation behind this change we
don't need to hunt down all uses of these constants, we just change the create
method's behaviour and everything should be working. Also I find it confusing
if some of the results are created using createXXX method (eg. place to queue),
while others are just using internal constants.
Other comments have been fixed.
> Create Actions for CS mapping rules
> -----------------------------------
>
> Key: YARN-10374
> URL: https://issues.apache.org/jira/browse/YARN-10374
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: yarn
> Reporter: Gergely Pollak
> Assignee: Gergely Pollak
> Priority: Major
> Attachments: YARN-10374.001.patch, YARN-10374.002.patch
>
>
> As per the design document attached to the umbrella Jira (YARN-10370), we
> need to create the classes which represent the different actions we can take
> when a mapping rule applies to an application placement. There are multiple
> actions to be implemented, like place to queue or reject application.
--
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
