Thanks for the review, Aaron! Here is a new patch taking into account your comments:
Task task = new ReduceTask("", attemptId, 0, maps.length, 1) <-- shouldn't this be "reduces.length" ?
This actually needs to be the number of maps according to the ReduceTask API; it lets it create a data structure for each map.
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.
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.
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.
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()
Fixed. Also fixed this in PoolSchedulable.
FairScheduler.UpdateThread.run(): why is preemptTasksIfNecessary() commented out? Needs a comment for rationale.
Oops! Fixed. I had commented that out for debugging.
FairScheduler.assignTasks() - Should convert System.out.println to log msg.
Removed the println, it was also for debugging.
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.
You have the comment: // Job not in infos (shouldn't happen)-
... So throw an exception if it does, or at least log this e
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).