----------------------------------------------------------- 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 > >
