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/06/26 22:20:47 UTC

Re: Review Request 11130: Allow some JVM memory vars to be configured.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11130/
-----------------------------------------------------------

(Updated June 26, 2013, 8:20 p.m.)


Review request for mesos.


Changes
-------

Dropped the overhead percentage config option as per Ben's comments.


Summary (updated)
-----------------

Allow some JVM memory vars to be configured.


Repository: mesos


Description (updated)
-------

Allow some JVM memory vars to be configured.

Review: https://reviews.apache.org/r/11130


Diffs (updated)
-----

  hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java c5a63bc7a172c44cc11d6bdd6b2eb4bc3df35cb7 

Diff: https://reviews.apache.org/r/11130/diff/


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request 11130: Allowed some JVM memory vars to be configured.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11130/#review23990
-----------------------------------------------------------

Ship it!



hadoop/mapred-site.xml.patch
<https://reviews.apache.org/r/11130/#comment47806>

    Thanks!


- Ben Mahler


On July 25, 2013, 1:27 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11130/
> -----------------------------------------------------------
> 
> (Updated July 25, 2013, 1:27 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Allowed some JVM memory vars to be configured.
> 
> Also modified the slot CPU/mem values to be more sane defaults.
> 
> Review: https://reviews.apache.org/r/11130
> 
> 
> Diffs
> -----
> 
>   hadoop/mapred-site.xml.patch 8b3997944f60a9d91c81089a4450e4820526dde3 
>   hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java 279f84e0f0c43ad3cfd9e4442010e706ee3565d9 
> 
> Diff: https://reviews.apache.org/r/11130/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11130: Allowed some JVM memory vars to be configured.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11130/
-----------------------------------------------------------

(Updated July 30, 2013, 7:45 p.m.)


Review request for mesos.


Changes
-------

Add dependency.


Repository: mesos


Description
-------

Allowed some JVM memory vars to be configured.

Also modified the slot CPU/mem values to be more sane defaults.

Review: https://reviews.apache.org/r/11130


Diffs
-----

  hadoop/mapred-site.xml.patch 8b3997944f60a9d91c81089a4450e4820526dde3 
  hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java 279f84e0f0c43ad3cfd9e4442010e706ee3565d9 

Diff: https://reviews.apache.org/r/11130/diff/


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request 11130: Allowed some JVM memory vars to be configured.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11130/
-----------------------------------------------------------

(Updated July 25, 2013, 1:27 a.m.)


Review request for mesos.


Changes
-------

Updated as per Vinod's comments.


Summary (updated)
-----------------

Allowed some JVM memory vars to be configured.


Repository: mesos


Description (updated)
-------

Allowed some JVM memory vars to be configured.

Also modified the slot CPU/mem values to be more sane defaults.

Review: https://reviews.apache.org/r/11130


Diffs (updated)
-----

  hadoop/mapred-site.xml.patch 8b3997944f60a9d91c81089a4450e4820526dde3 
  hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java 279f84e0f0c43ad3cfd9e4442010e706ee3565d9 

Diff: https://reviews.apache.org/r/11130/diff/


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request 11130: Allowed some JVM memory vars to be configured.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On July 25, 2013, 12:26 a.m., Vinod Kone wrote:
> > hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java, lines 453-454
> > <https://reviews.apache.org/r/11130/diff/8/?file=327647#file327647line453>
> >
> >     how about
> >     
> >     Math.round((1-JVM_MEM_OVERHEAD_PERCENT_DEFAULT)) * slotMem
> >     
> >     same for taskTrackerMem below?

I did this in a later commit.


- Brenden


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11130/#review23802
-----------------------------------------------------------


On July 25, 2013, 1:27 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11130/
> -----------------------------------------------------------
> 
> (Updated July 25, 2013, 1:27 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Allowed some JVM memory vars to be configured.
> 
> Also modified the slot CPU/mem values to be more sane defaults.
> 
> Review: https://reviews.apache.org/r/11130
> 
> 
> Diffs
> -----
> 
>   hadoop/mapred-site.xml.patch 8b3997944f60a9d91c81089a4450e4820526dde3 
>   hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java 279f84e0f0c43ad3cfd9e4442010e706ee3565d9 
> 
> Diff: https://reviews.apache.org/r/11130/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11130: Allow some JVM memory vars to be configured.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11130/#review23802
-----------------------------------------------------------

Ship it!



hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java
<https://reviews.apache.org/r/11130/#comment47638>

    Make this 1 to match the config?



hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java
<https://reviews.apache.org/r/11130/#comment47639>

    ditto



hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java
<https://reviews.apache.org/r/11130/#comment47640>

    how about
    
    Math.round((1-JVM_MEM_OVERHEAD_PERCENT_DEFAULT)) * slotMem
    
    same for taskTrackerMem below?


- Vinod Kone


On July 24, 2013, 11 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11130/
> -----------------------------------------------------------
> 
> (Updated July 24, 2013, 11 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Allow some JVM memory vars to be configured.
> 
> Also modified the slot CPU/mem values to be more sane defaults.
> 
> Review: https://reviews.apache.org/r/11130
> 
> 
> Diffs
> -----
> 
>   hadoop/mapred-site.xml.patch 8b3997944f60a9d91c81089a4450e4820526dde3 
>   hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java 279f84e0f0c43ad3cfd9e4442010e706ee3565d9 
> 
> Diff: https://reviews.apache.org/r/11130/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11130: Allow some JVM memory vars to be configured.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11130/
-----------------------------------------------------------

(Updated July 24, 2013, 11 p.m.)


Review request for mesos.


Changes
-------

Rebasing on master, updating as per Ben's comments.


Repository: mesos


Description (updated)
-------

Allow some JVM memory vars to be configured.

Also modified the slot CPU/mem values to be more sane defaults.

Review: https://reviews.apache.org/r/11130


Diffs (updated)
-----

  hadoop/mapred-site.xml.patch 8b3997944f60a9d91c81089a4450e4820526dde3 
  hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java 279f84e0f0c43ad3cfd9e4442010e706ee3565d9 

Diff: https://reviews.apache.org/r/11130/diff/


Testing
-------

Used in production at airbnb.

make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3


Thanks,

Brenden Matthews


Re: Review Request 11130: Allow some JVM memory vars to be configured.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On June 27, 2013, 7:07 p.m., Ben Mahler wrote:
> > hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java, lines 452-453
> > <https://reviews.apache.org/r/11130/diff/7/?file=311244#file311244line452>
> >
> >     Include the overhead in the default computation. To get the heap value you would subtract the overhead from slotMem (as done previously).
> >     
> >     This is because 'cpus', 'disk', and 'mem' are the raw resources allocated by mesos, so it would be nice to keep that meaning consistent. We could hide 'mem' and introduce 'heap', 'jvm_overhead', etc, should the need arise later.

I'm not sure I understand why you raised this as an issue.  What's there to change here?  As far as I can tell, it behaves as you describe.


> On June 27, 2013, 7:07 p.m., Ben Mahler wrote:
> > hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java, lines 457-460
> > <https://reviews.apache.org/r/11130/diff/7/?file=311244#file311244line457>
> >
> >     I think we want to keep the overhead as part of the default computation. So that 'mem' continues to mean the resources allocated by mesos.
> >     
> >     Also, we should add a default value in mapred-site.xml for this new knob. Explaining that 10% of 'mem' is used as overhead for the JVM.
> >     
> >     As you said, we may need to expose the overhead percent knob, but let's hope not! :)

See comment above.


- Brenden


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11130/#review22479
-----------------------------------------------------------


On June 26, 2013, 8:20 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11130/
> -----------------------------------------------------------
> 
> (Updated June 26, 2013, 8:20 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Allow some JVM memory vars to be configured.
> 
> Review: https://reviews.apache.org/r/11130
> 
> 
> Diffs
> -----
> 
>   hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java c5a63bc7a172c44cc11d6bdd6b2eb4bc3df35cb7 
> 
> Diff: https://reviews.apache.org/r/11130/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 11130: Allow some JVM memory vars to be configured.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11130/#review22479
-----------------------------------------------------------


Cool, this is looking pretty good to me now, I like adding the new knob for task tracker memory!

My comments are for keeping the meaning of 'mem' to be the directly related to 'mem' as a resource in mesos. This will keep it clear as to what memory limit will be used for the TaskTracker, and administrators will be forced to think about the JVM overhead should they need to change the values.

The downside is that we're decreeing 10% as the overhead. If that turns out to be a bad thing, we can add that as well and revisit the knobs.


hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java
<https://reviews.apache.org/r/11130/#comment46023>

    Can you update the comment in mapred-site.xml to reflect that 10% of mapred.mesos.tasktracker.mem will be allocated only for JVM overhead?



hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java
<https://reviews.apache.org/r/11130/#comment46024>

    Include the overhead in the default computation. To get the heap value you would subtract the overhead from slotMem (as done previously).
    
    This is because 'cpus', 'disk', and 'mem' are the raw resources allocated by mesos, so it would be nice to keep that meaning consistent. We could hide 'mem' and introduce 'heap', 'jvm_overhead', etc, should the need arise later.



hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java
<https://reviews.apache.org/r/11130/#comment46025>

    I think we want to keep the overhead as part of the default computation. So that 'mem' continues to mean the resources allocated by mesos.
    
    Also, we should add a default value in mapred-site.xml for this new knob. Explaining that 10% of 'mem' is used as overhead for the JVM.
    
    As you said, we may need to expose the overhead percent knob, but let's hope not! :)


- Ben Mahler


On June 26, 2013, 8:20 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11130/
> -----------------------------------------------------------
> 
> (Updated June 26, 2013, 8:20 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Allow some JVM memory vars to be configured.
> 
> Review: https://reviews.apache.org/r/11130
> 
> 
> Diffs
> -----
> 
>   hadoop/mesos/src/java/org/apache/hadoop/mapred/MesosScheduler.java c5a63bc7a172c44cc11d6bdd6b2eb4bc3df35cb7 
> 
> Diff: https://reviews.apache.org/r/11130/diff/
> 
> 
> Testing
> -------
> 
> Used in production at airbnb.
> 
> make -j10 check && cd hadoop && make hadoop-2.0.0-mr1-cdh4.2.1 && make hadoop-0.20.205.0 && make hadoop-0.20.2-cdh3u3
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>