> On March 6, 2014, 4:20 p.m., Benjamin Hindman wrote: > > Before I dive too deep into this review, can you give an overview of your > > proposed next steps? In particular: > > > > (1) What's the concurrency story here? It appears as though multiple actors > > adding or removing metrics could causing conflicting writes and reads. > > > > (2) How do you plan on exposing the JSON data as a REST endpoint? > > > > (3) Do you plan to integrate into libprocess statistics? Were you thinking > > of changing the name of libprocess statistics to metrics? The advantage of > > using statistics is that you'll have solved (1) and you'll get time series > > that can be useful for people wanting to plot things without needing some > > other place to store the metrics.
1. i wanted to ensure the API was as expected for the current use-cases. You're correct that there is no concurrency yet, but I expected to be able to add that without changing the API significantly - ie by adding a Process to which operations are dispatched (as per process's Statistics). 2. The same way it is currently done in process Snapshot, which brings me to.. 3. I should actually roll this down into process, i suppose, though I was considering having the basic non-concurrent Metrics[Process] be implemented in stout with a concurrent wrapper in process that dispatches to the stout implementation. This would make the metrics subsystem available more widely in stout. Regarding using statistics directly - it's unclear to me why we're not already doing that if it provides the API we need. Perhaps just changing that to match this API (using Metric objects directly, etc) is enough? I was imagining TimeSeries to be implemented on top of this system - ie, you could imagine a TimeSeries object that is given a context and name and can poll the Metrics system for values where it makes sense. This could also just be left to the UI. It's not clear to me that a TimeSeries (or Histogram, same argument) should be part of the core API. - Dominic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18718/#review36455 ----------------------------------------------------------- On March 11, 2014, 11:46 a.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18718/ > ----------------------------------------------------------- > > (Updated March 11, 2014, 11:46 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-1036 > https://issues.apache.org/jira/browse/MESOS-1036 > > > Repository: mesos-git > > > Description > ------- > > See summary > > > Diffs > ----- > > src/Makefile.am 384b3122b61294401ba4a894c06e985d9fc2fb1e > src/metrics/counter.hpp PRE-CREATION > src/metrics/gauge.hpp PRE-CREATION > src/metrics/metric.hpp PRE-CREATION > src/metrics/metric.cpp PRE-CREATION > src/metrics/metrics.hpp PRE-CREATION > src/metrics/metrics.cpp PRE-CREATION > src/tests/metrics_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/18718/diff/ > > > Testing > ------- > > Added unit tests. make check. > > > Thanks, > > Dominic Hamon > >
