hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aaron Kimball (JIRA)" <j...@apache.org>
Subject [jira] Commented: (MAPREDUCE-706) Support for FIFO pools in the fair scheduler
Date Fri, 31 Jul 2009 05:56:14 GMT

    [ https://issues.apache.org/jira/browse/MAPREDUCE-706?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12737425#action_12737425

Aaron Kimball commented on MAPREDUCE-706:


Great documentation -- that really helps! :) Also good that you added a lot of tests. +1 overall
on this patch, subject to the following (relatively minor) questions and suggestions:

Task task = new ReduceTask("", attemptId, 0, maps.length, 1) <-- shouldn't this be "reduces.length"

TestFairScheduler.getLocalityLevel(): These locality level constants are used throughout the
FairScheduler; they should be converted to an Enum. (Magic constants are evil.)

TestComputeFairShares.testEmptyList() -- should this call verifyShares() after computeFairShares()
to assert that the list length is zero?

PoolManager.parseSchedulingMode(): why case sensitive 'fifo' and 'fair' ? maybe use toLower()

PoolSchedulable c'tor: scheduler.getClock().getTime() should be called only once to guarantee
this.lastTimeAtMinShare == this.lastTimeAtHalfFairShare on start?

assignTask(): Is SchedulingMode guaranteed to never be extended by another internal algorithm?
If not, turn "else" into "else if" and have an "else throw InvalidArgumentException" at the
end of the case.

JobSchedulable.updateDemand(): why does this use System.currentTimeMillis() instead of getting
the time from a Clock object?

Schedulable's class javadoc: typo "algoirthms"

SchuldingAlgorithms.LOG: rather than use a string, use SchedulingAlgorithms.class.getName()

FairScheduler.UpdateThread.run(): why is preemptTasksIfNecessary() commented out? Needs a
comment for rationale.

FairScheduler.assignTasks() -- Should convert System.out.println to log msg.

This method is also getting pretty long. Consider refactoring the inner loop into shorter
methods if you need to add anything else to it in the future.

You have the comment:  // Job not in infos (shouldn't happen)- 
... So throw an exception if it does, or at least log this event with level ERROR, rather
than returning an in-bounds value? When you get to switch(info.lastMapLocalityLevel), you'll
naturally throw an NPE, so the caller should just deal with that and clean up its own mess.

> Support for FIFO pools in the fair scheduler
> --------------------------------------------
>                 Key: MAPREDUCE-706
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-706
>             Project: Hadoop Map/Reduce
>          Issue Type: New Feature
>          Components: contrib/fair-share
>            Reporter: Matei Zaharia
>            Assignee: Matei Zaharia
>         Attachments: fsdesigndoc.pdf, fsdesigndoc.tex, mapreduce-706.patch, mapreduce-706.v1.patch,
> The fair scheduler should support making the internal scheduling algorithm for some pools
be FIFO instead of fair sharing in order to work better for batch workloads. FIFO pools will
behave exactly like the current default scheduler, sorting jobs by priority and then submission
time. Pools will have their scheduling algorithm set through the pools config file, and it
will be changeable at runtime.
> To support this feature, I'm also changing the internal logic of the fair scheduler to
no longer use deficits. Instead, for fair sharing, we will assign tasks to the job farthest
below its share as a ratio of its share. This is easier to combine with other scheduling algorithms
and leads to a more stable sharing situation, avoiding unfairness issues brought up in MAPREDUCE-543
and MAPREDUCE-544 that happen when some jobs have long tasks. The new preemption (MAPREDUCE-551)
will ensure that critical jobs can gain their fair share within a bounded amount of time.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message