> On April 24, 2014, 2:38 p.m., Benjamin Hindman wrote: > > src/master/master.hpp, line 519 > > <https://reviews.apache.org/r/20638/diff/1/?file=566420#file566420line519> > > > > Why 'counters' instead of 'metrics'? I like the namespacing, but in the > > past we used 'stats' for everything, and I can't see the benefit in > > 'counters', 'gauges', and 'timers', since they're all just metrics. > > Something else I'm missing? > > Ben Mahler wrote: > Well, in the past all 'stats' were counters. Now we have different kinds > of metrics to deal with and I think being able to immediately be aware of the > difference when reading the code is nice! Especially when dealing with > 'counters' vs. 'timers' vs. 'gauges'. > > The downside to this approach as I see it is if we had metric types with > names that are overloaded. These might actually lead to confusion instead of > us realizing that we're dealing with a metric: > > timers.slave_admission.time(...); // Is this a metric Timer or a > libprocess Timer? > > vs. > > metrics.slave_admission.time(...); // Clearly a metric, but what kind? > > Dominic and I had this discussion on r/20581. I feel both approaches are > fine but I find 'counters', 'timers', ... more readable in that we need to > "think less" when reading the code. > > Benjamin Hindman wrote: > Why do you need to know the type? Seems like the type system will enforce > that for you (i.e., you can't use a timer like a counter and vice versa). > What about a class that wants to have a collection of libprocess Timers for > something else? If we introduce this precedent, we're taking up the name of > something that honestly seems useful in other settings (where as 'metrics' is > pretty clearly going to be for metrics). Finally, it's nice to have all the > metrics in one place in a class IMHO.
I agree that 'timers' is unfortunately overloaded. :( You don't need to know the type, it just makes it easier to read (less thought required for the human reader). I suppose one has enough information from the usage to know the type of the metric. Alright, I will update this to be 'metrics' along with the Registrar ones to keep things consistent. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20638/#review41301 ----------------------------------------------------------- On April 24, 2014, 1:19 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20638/ > ----------------------------------------------------------- > > (Updated April 24, 2014, 1:19 a.m.) > > > Review request for mesos and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > See above. > > > Diffs > ----- > > src/master/master.hpp f567a4364e8edbf3a2f7bf5a3ebca45fbb64d252 > src/master/master.cpp 0335b3416ee1c4d14a70e018ad9174b465035c5f > src/tests/master_tests.cpp c429432b51ded9cdd7ac9abd5b4e98017c531d7e > > Diff: https://reviews.apache.org/r/20638/diff/ > > > Testing > ------- > > Added to the existing test. > > > Thanks, > > Ben Mahler > >
