> On Jan. 15, 2014, 1:50 a.m., Benjamin Hindman wrote:
> > src/slave/monitor.hpp, line 92
> > <https://reviews.apache.org/r/13604/diff/2/?file=422664#file422664line92>
> >
> >     I think it's probably time for s/ResourceMonitor/Monitor/ ;).

If it's just 'Monitor', I'm not sure whether it's clear as to what 'thing' is 
being monitored.


> On Jan. 15, 2014, 1:50 a.m., Benjamin Hindman wrote:
> > src/slave/monitor.cpp, line 99
> > <https://reviews.apache.org/r/13604/diff/2/?file=422665#file422665line99>
> >
> >     Why 'insert'?

Ah, this was an artifact of when I had make MonitoringInfo const, but I had 
loosened this to allow assignability and forgot to update this to use the [] 
operator, thanks!


> On Jan. 15, 2014, 1:50 a.m., Benjamin Hindman wrote:
> > src/tests/monitor_tests.cpp, lines 165-166
> > <https://reviews.apache.org/r/13604/diff/2/?file=422666#file422666line165>
> >
> >     Could you wrap the entire JSON string in 'string(...)' instead?


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13604/#review31840
-----------------------------------------------------------


On Jan. 15, 2014, 2 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13604/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2014, 2 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-375
>     https://issues.apache.org/jira/browse/MESOS-375
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Dumping data into Statistics leads to high memory consumption for various 
> reasons. We had to encode the string values of the statistical information 
> into a unique string key ("frameworkId/ExecutorId/statistic_name"), all of 
> these combinations consume a lot of memory. This also made it clunky in terms 
> of retrieving the latest information. This now uses 
> TimeSeries<ResourceStatistics> to efficiently store the monitoring 
> information.
> 
> My subsequent change in this chain of reviews returns immediate values to 
> simplify the statistics.json endpoint.
> 
> This also uses JSON::Protobuf I recently added to simplify the JSON 
> generation substantially!
> 
> 
> Diffs
> -----
> 
>   src/slave/monitor.hpp 52568ad8ec566f7cf36c249c76d798d44eacb578 
>   src/slave/monitor.cpp a931c4f35a8793c66ee03de82f0e0a21b92f8ffa 
>   src/tests/monitor_tests.cpp a341893b16fbe502fa32704fcd1f3f85519ad253 
> 
> Diff: https://reviews.apache.org/r/13604/diff/
> 
> 
> Testing
> -------
> 
> make check (no test modifications needed for this change as it preserves the 
> API)
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Reply via email to