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



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/29033/#comment108033>

    just because we can't read /proc/net/sockstat, we might still want to do 
the expensive socket stats below. if this was a different method you could keep 
this logic.



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/29033/#comment108034>

    this is going to go through the entire output once and then you're going to 
go through each line. this could be much faster if you just read the string up 
to each '\n' once.



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/29033/#comment108036>

    const_iterator



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/29033/#comment108037>

    why is this initialization separate to the declaration?



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/29033/#comment108035>

    why do you need to find (which is a linear search) twice?
    
    instead, just loop over the tokens and check for both "inuse" or "tw" and 
then numify the next token. then you only search through the tokens once.



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/29033/#comment108032>

    consider separating this into a separate function for independent testing 
and code readability.



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/29033/#comment108038>

    you need a test that ensures this gives the right result when this is false.



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/29033/#comment108040>

    do you really need 5 seconds? this is overhead on every test run.



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/29033/#comment108039>

    should be a timeout on here


- Dominic Hamon


On Dec. 14, 2014, 7:26 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29033/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2014, 7:26 p.m.)
> 
> 
> Review request for mesos, Dominic Hamon, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> While we are still working to make RTT less expensive to get, expose total 
> number of tcp connections in use and in TIME_WAIT. 
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 540071d 
>   src/slave/containerizer/isolators/network/port_mapping.hpp f1e2352 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 2d81336 
>   src/slave/flags.hpp 670997d 
>   src/tests/port_mapping_tests.cpp eb82993 
> 
> Diff: https://reviews.apache.org/r/29033/diff/
> 
> 
> Testing
> -------
> 
> expanded a test case to test PortMappingStatistics.
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>

Reply via email to