[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mesos-commits
Subject:    (mesos) branch master updated: Improved performance of RatesCollector in network/port_mapping isolat
From:       bmahler () apache ! org
Date:       2024-01-22 20:57:35
Message-ID: 170595705509.2104670.1684406881794564871 () gitbox2-he-fi ! apache ! org
[Download RAW message or body]

This is an automated email from the ASF dual-hosted git repository.

bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git


The following commit(s) were added to refs/heads/master by this push:
     new 03aba5411 Improved performance of RatesCollector in network/port_mapping \
isolator. 03aba5411 is described below

commit 03aba54119dc82053991747d80e7c40d9bf1d132
Author: Ilya Pronin <ipronin@twitter.com>
AuthorDate: Mon Feb 12 16:58:59 2018 -0800

    Improved performance of RatesCollector in network/port_mapping isolator.
    
    To get a single link info utilities in routing::link pull all configured
    links infos from the kernel. With this change RatesCollector pulls the
    link cache only once per sampling round.
---
 src/linux/routing/link/internal.hpp                | 38 ++++++------
 .../mesos/isolators/network/port_mapping.cpp       | 68 +++++++++++++++++-----
 .../mesos/isolators/network/port_mapping.hpp       | 11 ++--
 3 files changed, 76 insertions(+), 41 deletions(-)

diff --git a/src/linux/routing/link/internal.hpp \
b/src/linux/routing/link/internal.hpp index 38100efe0..b834a582a 100644
--- a/src/linux/routing/link/internal.hpp
+++ b/src/linux/routing/link/internal.hpp
@@ -52,9 +52,8 @@ inline void cleanup(struct rtnl_link* link)
 namespace link {
 namespace internal {
 
-// Returns the netlink link object associated with a given link by its
-// name. Returns None if the link is not found.
-inline Result<Netlink<struct rtnl_link>> get(const std::string& link)
+// Returns the cache of all link objects from the kernel.
+inline Try<Netlink<struct nl_cache>> get()
 {
   Try<Netlink<struct nl_sock>> socket = routing::socket();
   if (socket.isError()) {
@@ -69,8 +68,20 @@ inline Result<Netlink<struct rtnl_link>> get(const std::string& \
link)  return Error(nl_geterror(error));
   }
 
-  Netlink<struct nl_cache> cache(c);
-  struct rtnl_link* l = rtnl_link_get_by_name(cache.get(), link.c_str());
+  return Netlink<struct nl_cache>(c);
+}
+
+
+// Returns the netlink link object associated with a given link by its
+// name. Returns None if the link is not found.
+inline Result<Netlink<struct rtnl_link>> get(const std::string& link)
+{
+  Try<Netlink<struct nl_cache>> cache = get();
+  if (cache.isError()) {
+    return Error(cache.error());
+  }
+
+  struct rtnl_link* l = rtnl_link_get_by_name(cache->get(), link.c_str());
   if (l == nullptr) {
     return None();
   }
@@ -83,21 +94,12 @@ inline Result<Netlink<struct rtnl_link>> get(const std::string& \
link)  // interface index. Returns None if the link is not found.
 inline Result<Netlink<struct rtnl_link>> get(int index)
 {
-  Try<Netlink<struct nl_sock>> socket = routing::socket();
-  if (socket.isError()) {
-    return Error(socket.error());
-  }
-
-  // Dump all the netlink link objects from kernel. Note that the flag
-  // AF_UNSPEC means all available families.
-  struct nl_cache* c = nullptr;
-  int error = rtnl_link_alloc_cache(socket->get(), AF_UNSPEC, &c);
-  if (error != 0) {
-    return Error(nl_geterror(error));
+  Try<Netlink<struct nl_cache>> cache = get();
+  if (cache.isError()) {
+    return Error(cache.error());
   }
 
-  Netlink<struct nl_cache> cache(c);
-  struct rtnl_link* l = rtnl_link_get(cache.get(), index);
+  struct rtnl_link* l = rtnl_link_get(cache->get(), index);
   if (l == nullptr) {
     return None();
   }
diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp \
b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp index \
                f5e74eb9e..c0dcb0ae7 100644
--- a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
@@ -62,6 +62,7 @@
 #include "linux/fs.hpp"
 #include "linux/ns.hpp"
 
+#include "linux/routing/internal.hpp"
 #include "linux/routing/route.hpp"
 #include "linux/routing/utils.hpp"
 
@@ -73,6 +74,7 @@
 
 #include "linux/routing/handle.hpp"
 
+#include "linux/routing/link/internal.hpp"
 #include "linux/routing/link/link.hpp"
 #include "linux/routing/link/veth.hpp"
 
@@ -1588,20 +1590,13 @@ Option<Statistics<uint64_t>> \
PercentileRatesCollector::txErrorRate() const  }
 
 
-void PercentileRatesCollector::sample()
-{
-  const Time ts = Clock::now();
-
-  Result<hashmap<string, uint64_t>> stats = link::statistics(link);
-  if (stats.isSome()) {
-    sample(ts, std::move(stats.get()));
-  }
-}
-
-
 void PercentileRatesCollector::sample(
     const Time& ts, hashmap<string, uint64_t>&& statistics)
 {
+  if (statistics.empty()) {
+    return;
+  }
+
   if (previous.isSome() && previous.get() < ts) {
     // We sample statistics on the host end of the veth pair, so we
     // need to reverse RX and TX to get statistics inside the
@@ -1652,7 +1647,7 @@ public:
 
   void initialize() override
   {
-    schedule();
+    sample();
   }
 
   Future<ResourceStatistics> usage(const ContainerID& containerId)
@@ -1724,13 +1719,54 @@ public:
   }
 
 private:
-  void schedule()
+  // This method is mostly a copy of routing::link::statistics(), but
+  // with the ability to use the existing links cache. It was copied
+  // here to preserve libnl-agnostic interface of routing package and
+  // ease the cleanup when this collector will be replaced with eBPF.
+  static hashmap<string, uint64_t> statistics(
+      const Netlink<struct nl_cache>& cache,
+      const string& link)
   {
-    foreachvalue (PercentileRatesCollector& collector, collectors) {
-      collector.sample();
+    hashmap<string, uint64_t> results;
+
+    struct rtnl_link* l = rtnl_link_get_by_name(cache.get(), link.c_str());
+    if (l) {
+      Netlink<struct rtnl_link> _link(l);
+
+      rtnl_link_stat_id_t stats[] = {
+        RTNL_LINK_RX_PACKETS,
+        RTNL_LINK_RX_BYTES,
+        RTNL_LINK_RX_ERRORS,
+        RTNL_LINK_RX_DROPPED,
+        RTNL_LINK_TX_PACKETS,
+        RTNL_LINK_TX_BYTES,
+        RTNL_LINK_TX_ERRORS,
+        RTNL_LINK_TX_DROPPED,
+      };
+
+      char buf[32];
+      size_t size = sizeof(stats) / sizeof(stats[0]);
+      for (size_t i = 0; i < size; i++) {
+        rtnl_link_stat2str(stats[i], buf, 32);
+        results[buf] = rtnl_link_get_stat(_link.get(), stats[i]);
+      }
+    }
+
+    return results;
+  }
+
+  void sample()
+  {
+    const Time timestamp = Clock::now();
+
+    Try<Netlink<struct nl_cache>> cache = link::internal::get();
+    if (cache.isSome()) {
+      foreachvalue (PercentileRatesCollector& collector, collectors) {
+        collector.sample(timestamp, statistics(cache.get(), collector.link));
+      }
     }
 
-    delay(interval, self(), &Self::schedule);
+    delay(interval, self(), &Self::sample);
   }
 
   void copyRate(
diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.hpp \
b/src/slave/containerizer/mesos/isolators/network/port_mapping.hpp index \
                c073c6d9c..df0584b97 100644
--- a/src/slave/containerizer/mesos/isolators/network/port_mapping.hpp
+++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.hpp
@@ -173,12 +173,12 @@ public:
   Option<process::Statistics<uint64_t>> txDropRate() const;
   Option<process::Statistics<uint64_t>> txErrorRate() const;
 
-  // Sample statistics from the interface.
-  void sample();
-
-  // Register the statistics sample. Exposed for testing.
+  // Register the statistics sample.
   void sample(const process::Time& ts, hashmap<std::string, uint64_t>&& stats);
 
+  // Name of the link to sample metrics from.
+  const std::string link;
+
 private:
   void sampleRate(
       const hashmap<std::string, uint64_t>& statistics,
@@ -187,9 +187,6 @@ private:
       double timeDelta,
       process::TimeSeries<uint64_t>& rates);
 
-  // Name of the link to sample metrics from.
-  const std::string link;
-
   process::TimeSeries<uint64_t> rxRates;
   process::TimeSeries<uint64_t> rxPackets;
   process::TimeSeries<uint64_t> rxDrops;


[prev in list] [next in list] [prev in thread] [next in thread]