-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22316/#review45261
-----------------------------------------------------------



src/Makefile.am
<https://reviews.apache.org/r/22316/#comment79996>

    whitespace .. you need a tab.



src/master/master.hpp
<https://reviews.apache.org/r/22316/#comment79997>

    Are we going to add more of these? It might make sense to combine them:
    
    struct PerFrameworkPrincipalMetrics
    {
      process::metrics::Counter received;
      process::metrics::Counter processed;
    };
    
    hashmap<std::string, process::Owned<PerFrameworkPrincipalMetrics> > 
perFrameworkPrincipalMetrics;
    
    what do you think?



src/master/master.cpp
<https://reviews.apache.org/r/22316/#comment79998>

    this does a double look-up. Can you use:
    
    Option<string> principal = frameworks.principals.get(event.message->from);
    if (principal.isSome()) {
      ...
    }
    
    



src/master/master.cpp
<https://reviews.apache.org/r/22316/#comment79999>

    see above re: double lookup.



src/master/master.cpp
<https://reviews.apache.org/r/22316/#comment80000>

    const string?



src/master/master.cpp
<https://reviews.apache.org/r/22316/#comment80001>

    s/and but/but/



src/master/master.cpp
<https://reviews.apache.org/r/22316/#comment80002>

    s/its the/the/



src/tests/rate_limiting_tests.cpp
<https://reviews.apache.org/r/22316/#comment80003>

    nit: s/is/are/



src/tests/rate_limiting_tests.cpp
<https://reviews.apache.org/r/22316/#comment80004>

    to be complete, you should check that the metrics aren't already added 
before the framework is registered.


- Dominic Hamon


On June 10, 2014, 11:04 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22316/
> -----------------------------------------------------------
> 
> (Updated June 10, 2014, 11:04 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone.
> 
> 
> Bugs: MESOS-1339
>     https://issues.apache.org/jira/browse/MESOS-1339
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> - Multiple frameworks use the same principal use the same counter.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 4a3f2e12643a1f02284587ebcbe9374f416b3d60 
>   src/master/master.hpp 26af1139a43a62b91712acd158b24a8977c81d3f 
>   src/master/master.cpp c18ccc4a1770cd68e4c3cb4b5f8ab912515ab613 
>   src/tests/rate_limiting_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22316/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Reply via email to