> On May 6, 2015, 11:51 a.m., Jie Yu wrote: > > src/slave/monitor.cpp, lines 89-90 > > <https://reviews.apache.org/r/33876/diff/1/?file=950753#file950753line89> > > > > 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.
Good catch! Thanks! > On May 6, 2015, 11:51 a.m., Jie Yu wrote: > > src/slave/monitor.cpp, lines 91-100 > > <https://reviews.apache.org/r/33876/diff/1/?file=950753#file950753line91> > > > > We typically put continuations below their parent (i.e., `usages()`). > > See `statistics` and its continuations. Reordered in latest patch > On May 6, 2015, 11:51 a.m., Jie Yu wrote: > > src/slave/monitor.cpp, line 148 > > <https://reviews.apache.org/r/33876/diff/1/?file=950753#file950753line148> > > > > 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`? It would be an extra lookup, but have removed the parameter and moved the lookup into usage() > On May 6, 2015, 11:51 a.m., Jie Yu wrote: > > src/slave/monitor.cpp, lines 151-152 > > <https://reviews.apache.org/r/33876/diff/1/?file=950753#file950753line151> > > > > Use lambda here:) To match the semantics of the previous version, do > > you also want to install onDiscarded callback here? Looks much better in a lambda! Thanks Added the onDiscarded callback too > On May 6, 2015, 11:51 a.m., Jie Yu wrote: > > src/slave/monitor.cpp, lines 153-158 > > <https://reviews.apache.org/r/33876/diff/1/?file=950753#file950753line153> > > > > We usually put onXXX at the end of the chain: > > ``` > > containerizer->usage(...) > > .then(...) > > .onFailed(...); > > ``` > > > > Also, could you please move the continuation `_usage` below? Done - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33876/#review82704 ----------------------------------------------------------- On May 11, 2015, 11:20 a.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33876/ > ----------------------------------------------------------- > > (Updated May 11, 2015, 11:20 a.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 > >
