----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33876/#review82704 -----------------------------------------------------------
src/slave/monitor.cpp <https://reviews.apache.org/r/33876/#comment133492> We've already done `using std::list` above. No need to have the `std::` prefix. ALso, no need for `process::` prefix as well. Here and everywhere else. src/slave/monitor.cpp <https://reviews.apache.org/r/33876/#comment133497> We typically put continuations below their parent (i.e., `usages()`). See `statistics` and its continuations. src/slave/monitor.cpp <https://reviews.apache.org/r/33876/#comment133502> No need to pass in `executorInfo` since you can get it from `monitored`? Do you want to return a Failure (or add a CHECK) to make sure containerId is in `monitored`? src/slave/monitor.cpp <https://reviews.apache.org/r/33876/#comment133499> Use lambda here:) To match the semantics of the previous version, do you also want to install onDiscarded callback here? src/slave/monitor.cpp <https://reviews.apache.org/r/33876/#comment133501> We usually put onXXX at the end of the chain: ``` containerizer->usage(...) .then(...) .onFailed(...); ``` Also, could you please move the continuation `_usage` below? - Jie Yu On May 6, 2015, 4:01 p.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33876/ > ----------------------------------------------------------- > > (Updated May 6, 2015, 4:01 p.m.) > > > Review request for mesos, Jie Yu and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > Added usages() to resource monitor to enable internal components tapping into > resource statistics. > > > Diffs > ----- > > src/slave/monitor.hpp 69c60a10187f8ea617c6be9738b28e8103e0ed27 > src/slave/monitor.cpp 398af010564e999b46e38560ba1e652261a9420c > > Diff: https://reviews.apache.org/r/33876/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Niklas Nielsen > >
