> On July 25, 2013, 1:36 a.m., Vinod Kone wrote: > > hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java, line 310 > > <https://reviews.apache.org/r/12924/diff/1/?file=327619#file327619line310> > > > > 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/ ? > >
My understanding of Java patterns is that doing things like returning a pair/tuple from a function is bad. The usual java pattern seems to be to write a container class. Then again, I'm not well versed in Java. I'll see if I can reconstruct this to be a little better. > On July 25, 2013, 1:36 a.m., Vinod Kone wrote: > > hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java, line 487 > > <https://reviews.apache.org/r/12924/diff/1/?file=327619#file327619line487> > > > > Why not make this class and this method abstract if it has to be > > overridden? I wanted to do some sort of partial inheritance (like C++ pure virtual), but I think that's not possible with Java. Maybe I need to factor the common parts out to a separate class. - Brenden ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12924/#review23804 ----------------------------------------------------------- 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 > >
