----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12924/#review23804 -----------------------------------------------------------
hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java <https://reviews.apache.org/r/12924/#comment47646> I think this class is doing a bit too much. The fact that it needs access to the driver and is a give away. Also, I don't like inner classes mutating the parent class variables. How about ResourcePolicy.computeSlots() just returning a pair of map and reduce slots that needs to be allocated (according to the policy)? Also, s/ResourcePolicy/SlotAllocationPolicy/ ? hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java <https://reviews.apache.org/r/12924/#comment47643> final? hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java <https://reviews.apache.org/r/12924/#comment47642> can you move these to the beginning of the class? hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java <https://reviews.apache.org/r/12924/#comment47644> Why not make this class and this method abstract if it has to be overridden? - Vinod Kone On July 24, 2013, 10:16 p.m., Brenden Matthews wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12924/ > ----------------------------------------------------------- > > (Updated July 24, 2013, 10:16 p.m.) > > > Review request for mesos. > > > Repository: mesos-git > > > Description > ------- > > Refactor Mesos scheduler into fixed/variable policies. > > > Diffs > ----- > > hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java > 279f84e0f0c43ad3cfd9e4442010e706ee3565d9 > > Diff: https://reviews.apache.org/r/12924/diff/ > > > Testing > ------- > > make check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 > > > Thanks, > > Brenden Matthews > >
