> 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?
>
> Ben Mahler wrote:
> 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.
This subtlety wasn't obvious. May be a comment here.
More importantly, what semantics do you want when someone adds a meter to a stat? Would it
be preferable to meter to the whole time series rather than just the future values? I'm not
sure, but good to think about.
> 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 &
>
> Ben Mahler wrote:
> 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.
i guess you are going to use const now.
> 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?
>
> Ben Mahler wrote:
> 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.
definitely do not align the "+"s?
> 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().
>
> Ben Mahler wrote:
> That would require a prototype of the function since usage needs _usage.
aha. i see that _usage is not a method on the class.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9094/#review15771
-----------------------------------------------------------
On Jan. 29, 2013, 1:38 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9094/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2013, 1:38 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
>
>
|