> On Jan. 28, 2013, 8:30 p.m., Vinod Kone wrote: > > third_party/libprocess/src/statistics.cpp, line 45 > > <https://reviews.apache.org/r/9092/diff/1/?file=251533#file251533line45> > > > > Put the initializer list on its own line.
I'd prefer to keep it on one line if it's a trivial initializer list. I've seen this done in many places: stout/json.hpp stout/try.hpp stout/option.hpp stout/result.hpp stout/duration.hpp etc > On Jan. 28, 2013, 8:30 p.m., Vinod Kone wrote: > > third_party/libprocess/src/statistics.cpp, line 184 > > <https://reviews.apache.org/r/9092/diff/1/?file=251533#file251533line184> > > > > was this necessary to suppress gcc warning? I added it for consistency. I prefer breaks or returns in switch cases that aren't expected to fall through. (Even though this one has a LOG(FATAL)). > On Jan. 28, 2013, 8:30 p.m., Vinod Kone wrote: > > third_party/libprocess/src/statistics.cpp, line 67 > > <https://reviews.apache.org/r/9092/diff/1/?file=251533#file251533line67> > > > > Why virtual? See my comment about virtual destructors from the previous review. > On Jan. 28, 2013, 8:30 p.m., Vinod Kone wrote: > > third_party/libprocess/src/statistics.cpp, line 167 > > <https://reviews.apache.org/r/9092/diff/1/?file=251533#file251533line167> > > > > s/for existing meter/for an existing meter that matches the meteredName > > and meteredType/ > On Jan. 28, 2013, 8:30 p.m., Vinod Kone wrote: > > third_party/libprocess/src/statistics.cpp, line 171 > > <https://reviews.apache.org/r/9092/diff/1/?file=251533#file251533line171> > > > > What if meterType doesn't match? > > > > As I mentioned above, meter->value() would be useful to have. For checking type, I think the only useful thing would be to ensure the type matches when a duplicate is found via a CHECK. This prevents the mistake of: meter (context, name, TYPE1, meterName) meter (context, name, TYPE2, meterName) // Was a no-op, but I now CHECK for this.. since it's a mistake by the caller. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9092/#review15764 ----------------------------------------------------------- On Jan. 24, 2013, 9:11 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9092/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2013, 9:11 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > This add metering into the statistics model. The metering was inspired by > jie's monitoring reviews. > > We'll need to compute metered data on top incoming raw data. The challenge is > that some meters will require previous data: > For example: > 1. cpu_time = 10 @ time 10; > 2. cpu_time = 20 @ time 20; --> cpu_usage = 1.0 (100%) this is 20-10 / 20-10 > 3. cpu_time = 25 @ time 30; --> cpu_usage = 0.5 (50%) this is 25-20 / 30-20 > > > This addresses bug MESOS-324. > https://issues.apache.org/jira/browse/MESOS-324 > > > Diffs > ----- > > third_party/libprocess/include/process/statistics.hpp > 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 > third_party/libprocess/src/statistics.cpp > 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f > > Diff: https://reviews.apache.org/r/9092/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
