> On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote: > > src/slave/monitor.cpp, lines 109-110 > > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line109> > > > > Why not use foreachpair() above?
I wanted to, but it seems theres an issue with the macro, when dealing with commas in templates. > On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote: > > src/slave/monitor.cpp, lines 167-169 > > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line167> > > > > Why are the "="s aligned here and not below? Hm.. I think because the right hand sides are so closely related: all pulling from statistics, the similar prefix operation. I tried aligning the other bit below but it doesn't seem to help much, since the right hand sides below are not related. > On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote: > > src/slave/monitor.cpp, line 185 > > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line185> > > > > I would put this function before _usage(). That would require a prototype of the function since usage needs _usage. > On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote: > > src/slave/slave.cpp, line 75 > > <https://reviews.apache.org/r/9094/diff/1/?file=251552#file251552line75> > > > > woah..magic! ;) > On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote: > > third_party/libprocess/src/statistics.cpp, line 106 > > <https://reviews.apache.org/r/9094/diff/1/?file=251555#file251555line106> > > > > I'm confused. Why does this get() return a Future, but not the next > > get() and timeseries()? Because I needed to represent the missing case. Here when stats[context][name] is empty I return a failed future. Whereas since the others return maps, an empty map is equivalent to the stats[context] or stats[context][name] being empty. The alternative here was to return an Option<double> instead of a Future<double>, but that would bubble up to returning a Future<Option<double>> instead of just a Future<double>. > On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote: > > src/slave/monitor.cpp, lines 154-155 > > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line154> > > > > const & They can't be const& because I'm using the [] operators, I tried to avoid the [] operators but the resulting code is considerably more clunky. > On Jan. 28, 2013, 10:16 p.m., Vinod Kone wrote: > > src/slave/monitor.cpp, lines 142-149 > > <https://reviews.apache.org/r/9094/diff/1/?file=251548#file251548line142> > > > > Swap these two to match the doc? I first apply the meter to track the stat, so that the first value is given to the meter. If I meter afterwards, then the meter won't apply until the second value arrives. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9094/#review15771 ----------------------------------------------------------- On Jan. 24, 2013, 9:21 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9094/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2013, 9:21 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > This is the meat of the resource monitoring design. > > -Added a ResourceUsage protobuf. > -Added a ResourceMonitor process that periodically hits the isolation module > for usage information. > -Usage is exported to the Statistics process. > -Usage is available via a JSON endpoint. > > Implementation of the isolation module code will follow in subsequent reviews. > > > This addresses bug MESOS-324. > https://issues.apache.org/jira/browse/MESOS-324 > > > Diffs > ----- > > include/mesos/executor.hpp 0ea70528a74bb9ba7d2aaac85d2ff85928363869 > include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 > src/Makefile.am 0ab59b75b2955c532d0833f132bdaffe323838d0 > src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 > src/slave/cgroups_isolation_module.hpp > 669efa14ba2603764aa68ae19a44e79dbfdec192 > src/slave/cgroups_isolation_module.cpp > 63cefc33cf34eebb82db5d8448b751be8652fa36 > src/slave/constants.hpp ddf02570caf3793106b3c48e158a5bb48c1ae80c > src/slave/constants.cpp 1735a6b55a93e6537a5a119e5345961f3d84a000 > src/slave/isolation_module.hpp b962365ebeddd047896a66b02a327aa26ae323d3 > src/slave/lxc_isolation_module.hpp 2bc844f491befbe588965da2ada7cfcef0b6f0a4 > src/slave/lxc_isolation_module.cpp 30cff2a49339bb07030727d30352536a0a22d58c > src/slave/monitor.hpp PRE-CREATION > src/slave/monitor.cpp PRE-CREATION > src/slave/process_based_isolation_module.hpp > f1817192582e3646f8dcf17934ba7998829e8fd6 > src/slave/process_based_isolation_module.cpp > 3d50a4b652e4e09dd57e744e408c8fb79ff3fbf5 > src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 > src/slave/slave.cpp 9755b46f97173d6fcc9ab1fd63e0e4814b3bc018 > src/tests/utils.hpp be457117515ee727af101370b26bf9188afb8f45 > third_party/libprocess/include/process/statistics.hpp > 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 > third_party/libprocess/src/statistics.cpp > 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f > third_party/libprocess/src/statistics_tests.cpp > 0aaab3526618171c7cfbd11d40d614344bcbfd0a > > Diff: https://reviews.apache.org/r/9094/diff/ > > > Testing > ------- > > make check > > Although I didn't add tests, I've manually tested end-to-end with my future > reviews. > > > Thanks, > > Ben Mahler > >
