[
https://issues.apache.org/jira/browse/MAPREDUCE-706?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12739268#action_12739268
]
Matei Zaharia commented on MAPREDUCE-706:
-----------------------------------------
Thanks for the review, Aaron! Here is a new patch taking into account your
comments:
{quote}
TestFairScheduler.obtainNewReduceTask():
Task task = new ReduceTask("", attemptId, 0, maps.length, 1) <-- shouldn't this
be "reduces.length" ?
{quote}
This actually needs to be the number of maps according to the ReduceTask API;
it lets it create a data structure for each map.
bq. TestFairScheduler.getLocalityLevel(): These locality level constants are
used throughout the FairScheduler; they should be converted to an Enum. (Magic
constants are evil.)
Fixed. I added a LocalityLevel enum with methods for getting the locality level
of a given task and converting a locality level to a cache level cap for
obtainNewMapTask.
bq. TestComputeFairShares.testEmptyList() - should this call verifyShares()
after computeFairShares() to assert that the list length is zero?
Fixed.
bq. PoolManager.parseSchedulingMode(): why case sensitive 'fifo' and 'fair' ?
maybe use toLower() ?
Fixed.
bq. PoolSchedulable c'tor: scheduler.getClock().getTime() should be called only
once to guarantee this.lastTimeAtMinShare == this.lastTimeAtHalfFairShare on
start?
Fixed.
bq. 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.
Fixed. Throws RuntimeException if there's another mode, which should cause the
JobTracker to exit and make it obvious that there's something deeply wrong.
bq. JobSchedulable.updateDemand(): why does this use System.currentTimeMillis()
instead of getting the time from a Clock object?
Fixed.
bq. Schedulable's class javadoc: typo "algoirthms"
Fixed.
bq. SchuldingAlgorithms.LOG: rather than use a string, use
SchedulingAlgorithms.class.getName()
Fixed. Also fixed this in PoolSchedulable.
bq. FairScheduler.UpdateThread.run(): why is preemptTasksIfNecessary()
commented out? Needs a comment for rationale.
Oops! Fixed. I had commented that out for debugging.
bq. FairScheduler.assignTasks() - Should convert System.out.println to log msg.
Removed the println, it was also for debugging.
bq. 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.
I've refactored it to take out the delay scheduling code and the cap
calculation code. It's now closer to fitting on one screenful.
{quote}
getAllowedLocalityLevel():
You have the comment: // Job not in infos (shouldn't happen)-
... So throw an exception if it does, or at least log this e
{quote}
Fixed. I log an error so the scheduler doesn't crash if for some reason a bug
does cause a job to go through there when it has no info set.
In addition to these fixes, I've added a bit more documentation on the delay
scheduling methods in this version of the patch, and I've changed
Schedulable.assignTasks to take the current time as a parameter so that we
don't have multiple calls to System.currentTimeMillis on each heartbeat (in
case that hurts performance).
> 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, mapreduce-706.v2.patch, mapreduce-706.v3.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.
