You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2015/06/17 22:49:56 UTC

mesos git commit: Replaced adhoc JSON conversion functions for ResourceStatistics in port mapping isolator with the general protocol buffer to JSON converter.

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 <pa...@twopensource.com>
Authored: Wed Jun 17 11:28:34 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
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));