----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21166/#review43025 -----------------------------------------------------------
Looks good, just a few minor items below. src/master/master.hpp <https://reviews.apache.org/r/21166/#comment77020> Do we want to flip the names on these to be a bit more consistent with how they are being exposed? i.e. resources_total resource_used resources_percent src/master/master.hpp <https://reviews.apache.org/r/21166/#comment77022> Feel free to put the definition in the .cpp file to keep it consistent with the other two. src/master/master.hpp <https://reviews.apache.org/r/21166/#comment77021> Can't this just re-use 'total' instead of calling _total_resources again? src/master/master.cpp <https://reviews.apache.org/r/21166/#comment77023> s/std::// here and below src/master/master.cpp <https://reviews.apache.org/r/21166/#comment77025> Rather than a CHECK here, can we do the same thing that was done in Resources::cpus() and Resources::mem()? if (resources.name() == name && resources.type() == Value::SCALAR) Because it's not obvious here that we'll prevent "cpus" from being specified as a non-SCALAR in the slave, and having a slave with strange resources crash the master would be bad! Ditto below. - Ben Mahler On May 14, 2014, 7:23 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21166/ > ----------------------------------------------------------- > > (Updated May 14, 2014, 7:23 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-1132 > https://issues.apache.org/jira/browse/MESOS-1132 > > > Repository: mesos-git > > > Description > ------- > > see summary > > > Diffs > ----- > > src/master/http.cpp fe74d5b417adf20d5c72416398446cc93924c317 > src/master/master.hpp 12111cf7b183438f540bb0aad17e50e3f367558a > src/master/master.cpp 2f0e9027c5bea8066ac13996451d4905a422336e > src/tests/master_tests.cpp dcda0c79a6693bc3c0b69b4c9b148f9608342738 > > Diff: https://reviews.apache.org/r/21166/diff/ > > > Testing > ------- > > make check > > ran local instance and manually verified resource statistics show up and > match stats.json. > > > Thanks, > > Dominic Hamon > >
