----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44474/#review122423 -----------------------------------------------------------
Please see the first comment... I think this unexpectedly introduces a fundamental change in how the metrics are counted. src/master/master.cpp (line 6461) <https://reviews.apache.org/r/44474/#comment184411> I'm not familiar with this code but it appears to be changing the behavior substantially. The original code sums across all slaves that are registered at the time of the metric call. The new code tracks counters for the various states when a task on a registered slave changes. IIUC, this is a fundamental change in how the metrics are determined because the slave registration state is considered at different times. For example, a task may change state on a registered slave and it gets counted, then the slave becomes unregistered, then the metric endpoint is queried. Old and new code will give different numbers? If my understanding is correct, then perhaps (somewhat) more performant code could be achieved by maintaining counters at the slave level, and then aggregating the counters from *registered* slaves? src/master/metrics.cpp (lines 38 - 46) <https://reviews.apache.org/r/44474/#comment184414> Please keep the original order which more closely matches the progression of states, i.e., staging before starting. src/master/metrics.cpp (line 193) <https://reviews.apache.org/r/44474/#comment184416> foreach? - Ian Downes On March 7, 2016, 2:51 p.m., Cong Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44474/ > ----------------------------------------------------------- > > (Updated March 7, 2016, 2:51 p.m.) > > > Review request for mesos, Ian Downes and Vinod Kone. > > > Bugs: MESOS-4740 > https://issues.apache.org/jira/browse/MESOS-4740 > > > Repository: mesos > > > Description > ------- > > Avoid iterate the list of slaves, instead just maintain some counters. > > > Diffs > ----- > > src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 > src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f > src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 > src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 > > Diff: https://reviews.apache.org/r/44474/diff/ > > > Testing > ------- > > Manual check > > > Thanks, > > Cong Wang > >