-----------------------------------------------------------
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
> 
>

Reply via email to