----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35229/#review88246 -----------------------------------------------------------
src/slave/containerizer/isolators/network/port_mapping.cpp (line 683) <https://reviews.apache.org/r/35229/#comment140669> 1 extra line please. src/slave/containerizer/isolators/network/port_mapping.cpp (line 686) <https://reviews.apache.org/r/35229/#comment140676> How about s/CopyNetworkStatsToProtobuf/addTrafficControlStatistics/ src/slave/containerizer/isolators/network/port_mapping.cpp (lines 688 - 689) <https://reviews.apache.org/r/35229/#comment140672> See my comments below. 'statistics' -> 'result' 'stats' -> 'statistics' src/slave/containerizer/isolators/network/port_mapping.cpp (line 694) <https://reviews.apache.org/r/35229/#comment140670> Can you include `using namespace routing::queueing::statistics` so that we don't need to specify the prefix. src/slave/containerizer/isolators/network/port_mapping.cpp (lines 694 - 711) <https://reviews.apache.org/r/35229/#comment140673> ``` // TODO: Use protobuf reflection here. if (statistics.contains(BACKLOG)) { tc->set_backlog(statistics[BACKLOG]); } ... ``` src/slave/containerizer/isolators/network/port_mapping.cpp (line 713) <https://reviews.apache.org/r/35229/#comment140668> Extra line please. src/slave/containerizer/isolators/network/port_mapping.cpp (lines 858 - 859) <https://reviews.apache.org/r/35229/#comment140667> Can you pass in eth0_name as we did for PortMappingUpdate? `link::eth0` is pretty heavy wait (involing reading the routing table, etc.) src/slave/containerizer/isolators/network/port_mapping.cpp (line 861) <https://reviews.apache.org/r/35229/#comment140671> Hum, we have too many 'stat' here in this file. How about 'statistics' -> 'result' 'stats' -> 'statistics' src/slave/containerizer/isolators/network/port_mapping.cpp (lines 863 - 866) <https://reviews.apache.org/r/35229/#comment140674> Do you want to print the error to stderr? src/slave/containerizer/isolators/network/port_mapping.cpp (lines 869 - 873) <https://reviews.apache.org/r/35229/#comment140675> Ditto on printing the error to stderr. - Jie Yu On June 16, 2015, 10 p.m., Paul Brett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35229/ > ----------------------------------------------------------- > > (Updated June 16, 2015, 10 p.m.) > > > Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang. > > > Bugs: MESOS-2836 > https://issues.apache.org/jira/browse/MESOS-2836 > > > Repository: mesos > > > Description > ------- > > Report per-container metrics for network bandwidth throttling to the slave. > > > Diffs > ----- > > src/slave/containerizer/isolators/network/port_mapping.hpp > 4c719b186b519fad0c3869dbdae8b60c3a2c20cc > src/slave/containerizer/isolators/network/port_mapping.cpp > e55e7b62bc29125458f9c0fb5477057ecc5a90df > > Diff: https://reviews.apache.org/r/35229/diff/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Paul Brett > >
