[
https://issues.apache.org/jira/browse/YARN-9298?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16779358#comment-16779358
]
Wilfred Spiegelenburg edited comment on YARN-9298 at 2/28/19 2:11 AM:
----------------------------------------------------------------------
1) oops copy past error, fixed now
2) yep, you're right replaced the text
3) added
4) The tests we have in YARN-8967 are up a level: they test the rules as part
of a list of rules and not really every rule independently. They do do not
check the rule config/init parts. I have added new tests for all rules in the
{{TestPlacementRuleFS}} class for config and init. I would like to leave the
placement checks in the policy for clarity.
5) You cannot use a switch with an Object as the input as per the [java
docs|https://docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.html]. To
do that we would need to switch on a string object compared to the Class name
which I don't think is a good idea as it is discouraged due to false
positives/negatives and class loader dependencies.
6) For the {{setConfig()}}:
* moving the Object check out will pollute the abstract class with
FairScheduler dependencies and two extra {{setConfig()}} methods. Those 2
methods will be _noop_ implementations in the abstract class. I think more
confusing when you look at it from other schedulers.
* The only part that could possibly be pulled out is getting the create flag
out that is done in this version of the patch.
6) I looked at {{initialize()}} but that is not really possible:
* Moving the scheduler check out is not possible, especially not into the
abstract class.
* The check for the parent rule outside the class itself does not make it any
cleaner. Two different cases are handled in the same code lines (not allowed
and not the same class). Moving them makes it really messy.
was (Author: wilfreds):
1) oops copy past error, fixed now
2) yep, you're right replaced the text
3) added
4) The tests we have in YARN-8967 are up a level: they test the rules as part
of a list of rules and not really every rule independently. They do do not
check the rule config/init parts. I have added new tests for all rules in the
{{TestPlacementRuleFS}} class for config and init. I would like to leave the
placement checks in the policy for clarity.
5) You cannot use a switch with an Object as the input as per the [java
docs|https://docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.html]. To
do that we would need to switch on a string object compared to the Class name
which I don't think is a good idea as it is discouraged due to false
positives/negatives and class loader dependencies.
6) For the {{setConfig()}}:
* moving the Object check out will pollute the abstract class with
FairScheduler dependencies and two extra {{setConfig()}} methods. Those 2
methods will be _noop_ implementations in the abstract class. I think more
confusing when you look at it from other schedulers.
* The only part that could possibly be pulled out is getting the create flag
out that is done in this version of the patch.
6) I looked at {{initialize()}} but that is not really possible:
* Moving the scheduler check out is not possible, especially not into the
abstract class.
* The check for the parent rule outside the class itself does not make it any
cleaner. Two different cases are handled in the same code lines (not allowed
and not the same class). Moving them makes it really messy.
> Implement FS placement rules using PlacementRule interface
> ----------------------------------------------------------
>
> Key: YARN-9298
> URL: https://issues.apache.org/jira/browse/YARN-9298
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: scheduler
> Reporter: Wilfred Spiegelenburg
> Assignee: Wilfred Spiegelenburg
> Priority: Major
> Attachments: YARN-9298.001.patch, YARN-9298.002.patch,
> YARN-9298.003.patch
>
>
> Implement existing placement rules of the FS using the PlacementRule
> interface.
> Preparation for YARN-8967
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
