[
https://issues.apache.org/jira/browse/YARN-10043?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17035442#comment-17035442
]
Szilard Nemeth edited comment on YARN-10043 at 2/12/20 3:27 PM:
----------------------------------------------------------------
Hi [[email protected]],
Some comments:
1. The javadoc of FairOrderingPolicy could be enhanched with specifying
"Compare using job submit time" a bit more: As per the implementation (compare
method), the app which is submitted before another app is preferred. This fact
should be included in the javadoc as well.
2. Still in the same javadoc: "3. Compare demands. SchedulableEntities without
resource demand get lower priority than ones who have demands." --> I would
change "who" to "which" or "that".
3. In TestFairOrderingPolicy: Multiple occurrences of statements like:
{code}
FairOrderingPolicy<MockSchedulableEntity> policy =
new FairOrderingPolicy<MockSchedulableEntity>();
{code}
You can remove the type argument to FairOrderingPolicy.
4. In TestFairOrderingPolicy: Multiple occurrences of statements like:
{code}
Assert.assertTrue(policy.getComparator().compare(r1, r2) == 0);
{code}
Should be changed to:
{code}
assertEquals(0, policy.getComparator().compare(r1, r2));
{code}
Please also add a message to all the assert statements.
5. In TestFairOrderingPolicy#testOrderingUsingAppDemand: You have multiple
occurrences of statements like:
{code}
r1.setUsed(Resources.createResource(0 * GB));
{code}
0*GB can be changed to 0.
6. In TestFairOrderingPolicy#testOrderingUsingAppDemand: I think you don't need
this statement:
{code}
// R1, R2 has been started at same time
assertEquals(r1.getStartTime(), r2.getStartTime());
{code}
as this assertion was already included in testOrderingUsingAppSubmitTime
was (Author: snemeth):
Hi [[email protected]],
Some comments:
# The javadoc of FairOrderingPolicy could be enhanched with specifying
"Compare using job submit time" a bit more: As per the implementation (compare
method), the app which is submitted before another app is preferred. This fact
should be included in the javadoc as well.
# Still in the same javadoc: "3. Compare demands. SchedulableEntities without
resource demand get lower priority than ones who have demands." --> I would
change "who" to "which" or "that".
# In TestFairOrderingPolicy: Multiple occurrences of statements like:
{code}
FairOrderingPolicy<MockSchedulableEntity> policy =
new FairOrderingPolicy<MockSchedulableEntity>();
{code}
You can remove the type argument to FairOrderingPolicy.
# In TestFairOrderingPolicy: Multiple occurrences of statements like:
{code}
Assert.assertTrue(policy.getComparator().compare(r1, r2) == 0);
{code}
Should be changed to:
{code}
assertEquals(0, policy.getComparator().compare(r1, r2));
{code}
Please also add a message to all the assert statements.
# In TestFairOrderingPolicy#testOrderingUsingAppDemand: You have multiple
occurrences of statements like:
{code}
r1.setUsed(Resources.createResource(0 * GB));
{code}
0*GB can be changed to 0.
# In TestFairOrderingPolicy#testOrderingUsingAppDemand: I think you don't need
this statement:
{code}
// R1, R2 has been started at same time
assertEquals(r1.getStartTime(), r2.getStartTime());
{code}
as this assertion was already included in testOrderingUsingAppSubmitTime
> FairOrderingPolicy Improvements
> -------------------------------
>
> Key: YARN-10043
> URL: https://issues.apache.org/jira/browse/YARN-10043
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Manikandan R
> Assignee: Manikandan R
> Priority: Major
> Attachments: YARN-10043.001.patch, YARN-10043.002.patch,
> YARN-10043.003.patch
>
>
> FairOrderingPolicy can be improved by using some of the approaches (only
> relevant) implemented in FairSharePolicy of FS. This improvement has
> significance in FS to CS migration context.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
