----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44260/#review122176 -----------------------------------------------------------
Fix it, then Ship it! I like the idea of making this more consistent with the metrics within `master/` and `slave/`, thanks! A couple of minor things here but otherwise looks great! src/master/allocator/mesos/hierarchical.hpp (line 284) <https://reviews.apache.org/r/44260/#comment184092> Hm.. I don't know that the comment here adds any clarity for the reader. At least for me, it's not clear what it means for a dispatch event to be "waiting", this is just the number of dispatches in the event queue. "wait" seems to imply some form of blocking. It seems pretty clear to me from the way the code is written here and the metric name, no? Also, keep in mind that this very name is used in a number of places, so the documentation should ideally be consistent. src/master/allocator/mesos/metrics.hpp (lines 1 - 47) <https://reviews.apache.org/r/44260/#comment184103> The directory layout / file naming here is a little odd, because this is coupled to the hierarchical allocator, but the following layout doesn't express this: `allocator/mesos/allocator.hpp` `allocator/mesos/metrics.hpp` `allocator/mesos/hierarchical.hpp` Ideally the hierarchical allocator and metrics are more clearly coupled, since there is the allocator wrapper also contained here. For now it's ok since we can move things around here (not part of the public interfaces). src/master/allocator/mesos/metrics.cpp (lines 17 - 21) <https://reviews.apache.org/r/44260/#comment184104> Any reason for the unusual ordering? - Ben Mahler On March 3, 2016, 4:17 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44260/ > ----------------------------------------------------------- > > (Updated March 3, 2016, 4:17 p.m.) > > > Review request for mesos, Alexander Rukletsov and Ben Mahler. > > > Bugs: MESOS-4718 > https://issues.apache.org/jira/browse/MESOS-4718 > > > Repository: mesos > > > Description > ------- > > Moved metrics of the hierarchical allocator to its own file. > > > Diffs > ----- > > src/CMakeLists.txt 0eabfad66f43adff12d08dbf34ca3db1b801c49e > src/Makefile.am b30cc25f61856d6417437547baaa0bb338a30d63 > src/master/allocator/mesos/hierarchical.hpp > 3043888630b066505410d3b32c5b3f813cc458c1 > src/master/allocator/mesos/metrics.hpp PRE-CREATION > src/master/allocator/mesos/metrics.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/44260/diff/ > > > Testing > ------- > > `make distcheck` on OS X. > > > Thanks, > > Benjamin Bannier > >
