----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29033/#review66855 -----------------------------------------------------------
include/mesos/mesos.proto <https://reviews.apache.org/r/29033/#comment110519> // Total number of active tcp connections in a container. include/mesos/mesos.proto <https://reviews.apache.org/r/29033/#comment110520> Total number of ... in a container. include/mesos/mesos.proto <https://reviews.apache.org/r/29033/#comment110521> s/tw/timed_wait/ src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/29033/#comment110522> static please. Also, we seldom use non const references. Please use the following signature: Try<JSON::Object> getTCPSocketsCount(); src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/29033/#comment110538> Kill this. See my comments below. src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/29033/#comment110523> We don't use this style. Please use two lines: // ... size_t index = 0; size_t found = 0; // ... src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/29033/#comment110536> This is a little hard to read and follow, and we seldom use std iterators or non const references. How about the following. ``` foreach (const string& line, strings::tokenize(value.get(), "\n")) { if (!strings::startsWith(line, "TCP")) { continue; } vector<string> tokens = strings::tokenize(line, " "); for (int i = 0; i < tokens.size(); i++) { if (tokens[i] == "inuse") { if (i + 1 >= tokens.size()) { return Error("Unexpected output from /proc/net/sockstat"); } Try<size_t> inuse = numify<size_t>(tokens[i+1]); if (inuse.isError()) { return Error(...); } object.values["..."] = inuse.get(); } else if (tokens[i] == "tw") { // Similar to above. // ... } } } return object; ``` src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/29033/#comment110541> Ditto above. Actually, given that you need to merge JSON::Object below. I would suggest simply just kill these helper functions and put all logics in PortMappingStatistics::execute so that people can read your code without jump back and forth. - Jie Yu On Jan. 6, 2015, 12:01 a.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29033/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2015, 12:01 a.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 > >
