----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13604/#review31840 -----------------------------------------------------------
Ship it! src/slave/monitor.hpp <https://reviews.apache.org/r/13604/#comment60572> I think it's probably time for s/ResourceMonitor/Monitor/ ;). src/slave/monitor.hpp <https://reviews.apache.org/r/13604/#comment60573> Not needed? src/slave/monitor.hpp <https://reviews.apache.org/r/13604/#comment60574> s/history/archive/? src/slave/monitor.hpp <https://reviews.apache.org/r/13604/#comment60575> s/history.json/archive.json/? ;) Also, you can now get path parameters for routes! src/slave/monitor.cpp <https://reviews.apache.org/r/13604/#comment60576> For std::pair again? src/slave/monitor.cpp <https://reviews.apache.org/r/13604/#comment60577> Revert? ;) src/slave/monitor.cpp <https://reviews.apache.org/r/13604/#comment60578> Why 'insert'? src/slave/monitor.cpp <https://reviews.apache.org/r/13604/#comment60579> s/Future<Nothing>::failed/Failure/ src/slave/monitor.cpp <https://reviews.apache.org/r/13604/#comment60580> Good riddance! src/slave/monitor.cpp <https://reviews.apache.org/r/13604/#comment60581> entry.values["statistics"] = JSON::Protobuf(statistics); src/tests/monitor_tests.cpp <https://reviews.apache.org/r/13604/#comment60582> Could you wrap the entire JSON string in 'string(...)' instead? - Benjamin Hindman On Jan. 14, 2014, 7:08 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13604/ > ----------------------------------------------------------- > > (Updated Jan. 14, 2014, 7:08 a.m.) > > > Review request for mesos, Benjamin Hindman and Jie Yu. > > > 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 > >
