Repository: mesos Updated Branches: refs/heads/master ca66e13db -> 1fe548562
Replaced adhoc JSON conversion functions for ResourceStatistics in port mapping isolator with the general protocol buffer to JSON converter. Review: https://reviews.apache.org/r/35536 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1fe54856 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1fe54856 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1fe54856 Branch: refs/heads/master Commit: 1fe54856278feafb3caa631895b63c5403b98983 Parents: ca66e13 Author: Paul Brett <[email protected]> Authored: Wed Jun 17 11:28:34 2015 -0700 Committer: Jie Yu <[email protected]> Committed: Wed Jun 17 13:49:27 2015 -0700 ---------------------------------------------------------------------- .../isolators/network/port_mapping.cpp | 73 +++++++------------- .../isolators/network/port_mapping.hpp | 9 --- src/tests/port_mapping_tests.cpp | 65 ++++++++--------- 3 files changed, 59 insertions(+), 88 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe54856/src/slave/containerizer/isolators/network/port_mapping.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/network/port_mapping.cpp b/src/slave/containerizer/isolators/network/port_mapping.cpp index e55e7b6..f97e960 100644 --- a/src/slave/containerizer/isolators/network/port_mapping.cpp +++ b/src/slave/containerizer/isolators/network/port_mapping.cpp @@ -113,13 +113,6 @@ namespace mesos { namespace internal { namespace slave { -const char NET_TCP_ACTIVE_CONNECTIONS[] = "net_tcp_active_connections"; -const char NET_TCP_TIME_WAIT_CONNECTIONS[] = "net_tcp_time_wait_connections"; -const char NET_TCP_RTT_MICROSECS_P50[] = "net_tcp_rtt_microsecs_p50"; -const char NET_TCP_RTT_MICROSECS_P90[] = "net_tcp_rtt_microsecs_p90"; -const char NET_TCP_RTT_MICROSECS_P95[] = "net_tcp_rtt_microsecs_p95"; -const char NET_TCP_RTT_MICROSECS_P99[] = "net_tcp_rtt_microsecs_p99"; - using mesos::slave::ExecutorRunState; using mesos::slave::Isolator; using mesos::slave::IsolatorProcess; @@ -709,7 +702,11 @@ int PortMappingStatistics::execute() return 1; } - JSON::Object object; + ResourceStatistics result; + + // NOTE: We use a dummy value here since this field will be cleared + // before the result is sent to the containerizer. + result.set_timestamp(0); if (flags.enable_socket_statistics_summary) { // Collections for socket statistics summary are below. @@ -757,7 +754,7 @@ int PortMappingStatistics::execute() continue; } - object.values[NET_TCP_ACTIVE_CONNECTIONS] = inuse.get(); + result.set_net_tcp_active_connections(inuse.get()); } else if (tokens[i] == "tw") { if (i + 1 >= tokens.size()) { cerr << "Unexpected output from /proc/net/sockstat" << endl; @@ -774,7 +771,7 @@ int PortMappingStatistics::execute() continue; } - object.values[NET_TCP_TIME_WAIT_CONNECTIONS] = tw.get(); + result.set_net_tcp_time_wait_connections(tw.get()); } } } @@ -821,14 +818,14 @@ int PortMappingStatistics::execute() size_t p95 = RTTs.size() * 95 / 100; size_t p99 = RTTs.size() * 99 / 100; - object.values[NET_TCP_RTT_MICROSECS_P50] = RTTs[p50]; - object.values[NET_TCP_RTT_MICROSECS_P90] = RTTs[p90]; - object.values[NET_TCP_RTT_MICROSECS_P95] = RTTs[p95]; - object.values[NET_TCP_RTT_MICROSECS_P99] = RTTs[p99]; + result.set_net_tcp_rtt_microsecs_p50(RTTs[p50]); + result.set_net_tcp_rtt_microsecs_p90(RTTs[p90]); + result.set_net_tcp_rtt_microsecs_p95(RTTs[p95]); + result.set_net_tcp_rtt_microsecs_p99(RTTs[p99]); } } - cout << stringify(object); + cout << stringify(JSON::Protobuf(result)); return 0; } @@ -2774,45 +2771,25 @@ Future<ResourceStatistics> PortMappingIsolatorProcess::__usage( Try<JSON::Object> object = JSON::parse<JSON::Object>(out.get()); if (object.isError()) { - return Failure("Failed to parse the output from the process that gets the " - "network statistics: " + object.error()); - } - - Result<JSON::Number> active = - object.get().find<JSON::Number>(NET_TCP_ACTIVE_CONNECTIONS); - if (active.isSome()) { - result.set_net_tcp_active_connections(active.get().value); - } - - Result<JSON::Number> tw = - object.get().find<JSON::Number>(NET_TCP_TIME_WAIT_CONNECTIONS); - if (tw.isSome()) { - result.set_net_tcp_time_wait_connections(tw.get().value); + return Failure( + "Failed to parse the output from the process that gets the " + "network statistics: " + object.error()); } - Result<JSON::Number> p50 = - object.get().find<JSON::Number>(NET_TCP_RTT_MICROSECS_P50); - if (p50.isSome()) { - result.set_net_tcp_rtt_microsecs_p50(p50.get().value); - } + Result<ResourceStatistics> _result = + protobuf::parse<ResourceStatistics>(object.get()); - Result<JSON::Number> p90 = - object.get().find<JSON::Number>(NET_TCP_RTT_MICROSECS_P90); - if (p90.isSome()) { - result.set_net_tcp_rtt_microsecs_p90(p90.get().value); + if (_result.isError()) { + return Failure( + "Failed to parse the output from the process that gets the " + "network statistics: " + object.error()); } - Result<JSON::Number> p95 = - object.get().find<JSON::Number>(NET_TCP_RTT_MICROSECS_P95); - if (p95.isSome()) { - result.set_net_tcp_rtt_microsecs_p95(p95.get().value); - } + result.MergeFrom(_result.get()); - Result<JSON::Number> p99 = - object.get().find<JSON::Number>(NET_TCP_RTT_MICROSECS_P99); - if (p99.isSome()) { - result.set_net_tcp_rtt_microsecs_p99(p99.get().value); - } + // NOTE: We unset the 'timestamp' field here because otherwise it + // will overwrite the timestamp set in the containerizer. + result.clear_timestamp(); return result; } http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe54856/src/slave/containerizer/isolators/network/port_mapping.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/isolators/network/port_mapping.hpp b/src/slave/containerizer/isolators/network/port_mapping.hpp index 4c719b1..2a988b3 100644 --- a/src/slave/containerizer/isolators/network/port_mapping.hpp +++ b/src/slave/containerizer/isolators/network/port_mapping.hpp @@ -65,15 +65,6 @@ inline std::string PORT_MAPPING_VETH_PREFIX() { return "mesos"; } // NOTE: This constant is exposed for testing. inline std::string PORT_MAPPING_BIND_MOUNT_ROOT() { return "/var/run/netns"; } -// These field names are used in the output of the mesos network helper -// and in the port mapping tests to read the output. -extern const char NET_TCP_ACTIVE_CONNECTIONS[]; -extern const char NET_TCP_TIME_WAIT_CONNECTIONS[]; -extern const char NET_TCP_RTT_MICROSECS_P50[]; -extern const char NET_TCP_RTT_MICROSECS_P90[]; -extern const char NET_TCP_RTT_MICROSECS_P95[]; -extern const char NET_TCP_RTT_MICROSECS_P99[]; - // The root directory where we keep all the namespace handle // symlinks. This is introduced in 0.23.0. // NOTE: This constant is exposed for testing. http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe54856/src/tests/port_mapping_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/port_mapping_tests.cpp b/src/tests/port_mapping_tests.cpp index f8372df..835a0f2 100644 --- a/src/tests/port_mapping_tests.cpp +++ b/src/tests/port_mapping_tests.cpp @@ -50,6 +50,8 @@ #include "master/master.hpp" +#include "mesos/mesos.hpp" + #include "slave/flags.hpp" #include "slave/slave.hpp" @@ -339,7 +341,7 @@ protected: return pid; } - JSON::Object statisticsHelper( + Result<ResourceStatistics> statisticsHelper( pid_t pid, bool enable_summary, bool enable_details) @@ -376,7 +378,7 @@ protected: Try<JSON::Object> object = JSON::parse<JSON::Object>(out.get()); CHECK_SOME(object); - return object.get(); + return ::protobuf::parse<ResourceStatistics>(object.get()); } slave::Flags flags; @@ -1569,32 +1571,24 @@ TEST_F(PortMappingIsolatorTest, ROOT_SmallEgressLimit) } -bool HasTCPSocketsCount(const JSON::Object& object) +bool HasTCPSocketsCount(const ResourceStatistics& statistics) { - return object.find<JSON::Number>(NET_TCP_ACTIVE_CONNECTIONS).isSome() && - object.find<JSON::Number>(NET_TCP_TIME_WAIT_CONNECTIONS).isSome(); + return statistics.has_net_tcp_active_connections() && + statistics.has_net_tcp_time_wait_connections(); } -bool HasTCPSocketsRTT(const JSON::Object& object) +bool HasTCPSocketsRTT(const ResourceStatistics& statistics) { - Result<JSON::Number> p50 = - object.find<JSON::Number>(NET_TCP_RTT_MICROSECS_P50); - Result<JSON::Number> p90 = - object.find<JSON::Number>(NET_TCP_RTT_MICROSECS_P90); - Result<JSON::Number> p95 = - object.find<JSON::Number>(NET_TCP_RTT_MICROSECS_P95); - Result<JSON::Number> p99 = - object.find<JSON::Number>(NET_TCP_RTT_MICROSECS_P99); - // We either have all of the following metrics or we have nothing. - if (!p50.isSome() && !p90.isSome() && !p95.isSome() && !p99.isSome()) { + if (statistics.has_net_tcp_rtt_microsecs_p50() && + statistics.has_net_tcp_rtt_microsecs_p90() && + statistics.has_net_tcp_rtt_microsecs_p95() && + statistics.has_net_tcp_rtt_microsecs_p99()) { + return true; + } else { return false; } - - EXPECT_TRUE(p50.isSome() && p90.isSome() && p95.isSome() && p99.isSome()); - - return true; } @@ -1718,17 +1712,26 @@ TEST_F(PortMappingIsolatorTest, ROOT_PortMappingStatistics) // While the connection is still active, try out different flag // combinations. - JSON::Object object = statisticsHelper(pid.get(), true, true); - ASSERT_TRUE(HasTCPSocketsCount(object) && HasTCPSocketsRTT(object)); - - object = statisticsHelper(pid.get(), true, false); - ASSERT_TRUE(HasTCPSocketsCount(object) && !HasTCPSocketsRTT(object)); - - object = statisticsHelper(pid.get(), false, true); - ASSERT_TRUE(!HasTCPSocketsCount(object) && HasTCPSocketsRTT(object)); - - object = statisticsHelper(pid.get(), false, false); - ASSERT_TRUE(!HasTCPSocketsCount(object) && !HasTCPSocketsRTT(object)); + Result<ResourceStatistics> statistics = + statisticsHelper(pid.get(), true, true); + ASSERT_SOME(statistics); + EXPECT_TRUE(HasTCPSocketsCount(statistics.get())); + EXPECT_TRUE(HasTCPSocketsRTT(statistics.get())); + + statistics = statisticsHelper(pid.get(), true, false); + ASSERT_SOME(statistics); + EXPECT_TRUE(HasTCPSocketsCount(statistics.get())); + EXPECT_FALSE(HasTCPSocketsRTT(statistics.get())); + + statistics = statisticsHelper(pid.get(), false, true); + ASSERT_SOME(statistics); + EXPECT_FALSE(HasTCPSocketsCount(statistics.get())); + EXPECT_TRUE(HasTCPSocketsRTT(statistics.get())); + + statistics = statisticsHelper(pid.get(), false, false); + ASSERT_SOME(statistics); + EXPECT_FALSE(HasTCPSocketsCount(statistics.get())); + EXPECT_FALSE(HasTCPSocketsRTT(statistics.get())); // Wait for the command to finish. ASSERT_TRUE(waitForFileCreation(container1Ready));
