> On April 22, 2014, 12:09 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/include/process/statistics.hpp, lines 64-65 > > <https://reviews.apache.org/r/20018/diff/10/?file=564597#file564597line64> > > > > Looking around at other libraries, "p999" and "p9999" seem to be pretty > > standard so I take back that this might confuse people. Let's revert, sound > > good?
no problem - i'm surprised that it's standard, but i'll go with it :) > On April 22, 2014, 12:09 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/include/process/statistics.hpp, lines 90-93 > > <https://reviews.apache.org/r/20018/diff/10/?file=564597#file564597line90> > > > > Looks like there is a regression here! What happens on line 76 when > > position is negative? Likely the CHECK_LT on line 86 will fail! > > > > This seems more difficult to understand and this change introduces a > > bug, let's revert! i just noticed you removed the clamping. > On April 22, 2014, 12:09 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/src/tests/metrics_tests.cpp, lines 144-162 > > <https://reviews.apache.org/r/20018/diff/10/?file=564599#file564599line144> > > > > Is this rebased off master? This test looks to be already committed. it is .. and i'm showing it as being different to master too locally. i'll rebase again, but not sure why it's not showing up in my master (which i pulled this morning). Are you thinking about the statistics_test.cpp? > On April 22, 2014, 12:09 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/src/tests/metrics_tests.cpp, line 149 > > <https://reviews.apache.org/r/20018/diff/10/?file=564599#file564599line149> > > > > size_t? http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Integer_Types#Integer_Types > On April 22, 2014, 12:09 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/src/tests/metrics_tests.cpp, lines 199-216 > > <https://reviews.apache.org/r/20018/diff/10/?file=564599#file564599line199> > > > > Is there any reason to overlap the removal of gauge and counter? Could > > we remove the gauge first to make this a bit easier to follow? there is, and i'll add a comment explaining it. > On April 22, 2014, 12:09 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/src/tests/metrics_tests.cpp, lines 259-263 > > <https://reviews.apache.org/r/20018/diff/10/?file=564599#file564599line259> > > > > Can we come up with an interval that generates more intuitive > > percentiles? One that a reader can intuit? it's tricky because i want to reset and end up with a non-zero value that isn't the maximum. try this? - Dominic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20018/#review41044 ----------------------------------------------------------- On April 22, 2014, 11:20 a.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20018/ > ----------------------------------------------------------- > > (Updated April 22, 2014, 11:20 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-1036 > https://issues.apache.org/jira/browse/MESOS-1036 > > > Repository: mesos-git > > > Description > ------- > > see summary. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/metrics/metric.hpp > 6a384ded8a4b57fd6aee819d0337773018c45669 > 3rdparty/libprocess/include/process/metrics/metrics.hpp > c20bb639e8ef79de63f0d0d56c2ea40a15a1f995 > 3rdparty/libprocess/include/process/statistics.hpp > d046bffa00fa839909112f65d19dfb9af46ad2b3 > 3rdparty/libprocess/src/metrics/metrics.cpp > 391295aea91e837bb856a40ef51d1c33d44371d8 > 3rdparty/libprocess/src/tests/metrics_tests.cpp > abe1588c931b45a09294812974788aa74de44dd4 > 3rdparty/libprocess/src/tests/statistics_tests.cpp > 478453fd60056603cf2eb96e56ac2df7e47a3e99 > > Diff: https://reviews.apache.org/r/20018/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Dominic Hamon > >
