You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Brenden Matthews <br...@diddyinc.com> on 2013/07/25 00:16:51 UTC
Review Request 12924: Refactor Mesos scheduler into fixed/variable policies.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12924/
-----------------------------------------------------------
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
Re: Review Request 12924: Refactor Mesos scheduler into fixed/variable
policies.
Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12924/#review24499
-----------------------------------------------------------
I'm abandoning this review in favour of:
https://reviews.apache.org/r/13213/
- Brenden Matthews
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
>
>
Re: Review Request 12924: Refactor Mesos scheduler into fixed/variable
policies.
Posted by Brenden Matthews <br...@diddyinc.com>.
> 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
>
>
Re: Review Request 12924: Refactor Mesos scheduler into fixed/variable
policies.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
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
>
>