You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gr...@apache.org on 2018/07/13 06:40:49 UTC

mesos git commit: Revert "Optimized the generation of metrics snapshots."

Repository: mesos
Updated Branches:
  refs/heads/master 133379439 -> 13fb36ec0


Revert "Optimized the generation of metrics snapshots."

This reverts commit 42becf22f07831ef8c3d4fbc8c1b80eb9d5b959a.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/13fb36ec
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/13fb36ec
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/13fb36ec

Branch: refs/heads/master
Commit: 13fb36ec04de52b1825a68ffd7a7f471aac68f7c
Parents: 1333794
Author: Greg Mann <gr...@gmail.com>
Authored: Thu Jul 12 23:28:25 2018 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Thu Jul 12 23:28:25 2018 -0700

----------------------------------------------------------------------
 .../include/process/metrics/metrics.hpp         |  8 +--
 3rdparty/libprocess/src/metrics/metrics.cpp     | 71 +++++++++-----------
 2 files changed, 34 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/13fb36ec/3rdparty/libprocess/include/process/metrics/metrics.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/metrics/metrics.hpp b/3rdparty/libprocess/include/process/metrics/metrics.hpp
index 3dc6bbf..f9b7202 100644
--- a/3rdparty/libprocess/include/process/metrics/metrics.hpp
+++ b/3rdparty/libprocess/include/process/metrics/metrics.hpp
@@ -25,6 +25,7 @@
 
 #include <process/metrics/metric.hpp>
 
+#include <stout/hashmap.hpp>
 #include <stout/nothing.hpp>
 #include <stout/option.hpp>
 
@@ -70,12 +71,11 @@ private:
   // capture with C++14.
   Future<std::map<std::string, double>> __snapshot(
       const Option<Duration>& timeout,
-      std::vector<std::string>&& keys,
-      std::vector<Future<double>>&& metrics,
-      std::vector<Option<Statistics<double>>>&& statistics);
+      hashmap<std::string, Future<double>>&& metrics,
+      hashmap<std::string, Option<Statistics<double>>>&& statistics);
 
   // The Owned<Metric> is an explicit copy of the Metric passed to 'add'.
-  std::map<std::string, Owned<Metric>> metrics;
+  hashmap<std::string, Owned<Metric>> metrics;
 
   // Used to rate limit the snapshot endpoint.
   Option<Owned<RateLimiter>> limiter;

http://git-wip-us.apache.org/repos/asf/mesos/blob/13fb36ec/3rdparty/libprocess/src/metrics/metrics.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/metrics/metrics.cpp b/3rdparty/libprocess/src/metrics/metrics.cpp
index 8220470..4883c9a 100644
--- a/3rdparty/libprocess/src/metrics/metrics.cpp
+++ b/3rdparty/libprocess/src/metrics/metrics.cpp
@@ -28,6 +28,7 @@
 #include <stout/duration.hpp>
 #include <stout/error.hpp>
 #include <stout/foreach.hpp>
+#include <stout/hashmap.hpp>
 #include <stout/numify.hpp>
 #include <stout/option.hpp>
 #include <stout/os.hpp>
@@ -120,7 +121,7 @@ string MetricsProcess::help()
 
 Future<Nothing> MetricsProcess::add(Owned<Metric> metric)
 {
-  if (metrics.count(metric->name()) > 0) {
+  if (metrics.contains(metric->name())) {
     return Failure("Metric '" + metric->name() + "' was already added");
   }
 
@@ -131,7 +132,7 @@ Future<Nothing> MetricsProcess::add(Owned<Metric> metric)
 
 Future<Nothing> MetricsProcess::remove(const string& name)
 {
-  if (metrics.count(name) == 0) {
+  if (!metrics.contains(name)) {
     return Failure("Metric '" + name + "' not found");
   }
 
@@ -144,35 +145,29 @@ Future<Nothing> MetricsProcess::remove(const string& name)
 Future<map<string, double>> MetricsProcess::snapshot(
     const Option<Duration>& timeout)
 {
-  // To avoid creating a new vector when calling `await()` below, we use three
-  // ordered vectors, where the Nth key in `keys` is associated with the Nth
-  // items in each of `futures` and `statistics`.
-  vector<string> keys;
-  vector<Future<double>> futures;
-  vector<Option<Statistics<double>>> statistics;
-
-  keys.reserve(metrics.size());
-  futures.reserve(metrics.size());
-  statistics.reserve(metrics.size());
-
-  for (auto iter = metrics.begin(); iter != metrics.end(); ++iter) {
-    keys.emplace_back(iter->first);
-    futures.emplace_back(iter->second->value());
-    statistics.emplace_back(iter->second->statistics());
+  hashmap<string, Future<double>> futures;
+  hashmap<string, Option<Statistics<double>>> statistics;
+
+  foreachkey (const string& name, metrics) {
+    const Owned<Metric>& metric = metrics.at(name);
+    futures[name] = metric->value();
+    // TODO(dhamon): It would be nice to compute these asynchronously.
+    statistics[name] = metric->statistics();
   }
 
   Future<Nothing> timedout =
     after(timeout.getOrElse(Duration::max()));
 
+  vector<Future<double>> values = futures.values();
+
   // Return the response once it finishes or we time out.
   return select<Nothing>({
       timedout,
-      await(futures).then([]{ return Nothing(); }) })
+      await(std::move(values)).then([]{ return Nothing(); }) })
     .onAny([=]() mutable { timedout.discard(); }) // Don't accumulate timers.
     .then(defer(self(),
                 &Self::__snapshot,
                 timeout,
-                std::move(keys),
                 std::move(futures),
                 std::move(statistics)));
 }
@@ -214,40 +209,34 @@ Future<http::Response> MetricsProcess::_snapshot(
 
 Future<map<string, double>> MetricsProcess::__snapshot(
     const Option<Duration>& timeout,
-    vector<string>&& keys,
-    vector<Future<double>>&& metrics,
-    vector<Option<Statistics<double>>>&& statistics)
+    hashmap<string, Future<double>>&& metrics,
+    hashmap<string, Option<Statistics<double>>>&& statistics)
 {
   map<string, double> snapshot;
 
-  for (size_t i = 0; i < metrics.size(); ++i) {
+  foreachpair (const string& key, const Future<double>& value, metrics) {
     // TODO(dhamon): Maybe add the failure message for this metric to the
     // response if value.isFailed().
-    const string& key = keys[i];
-    const Future<double>& value = metrics[i];
-
     if (value.isPending()) {
       CHECK_SOME(timeout);
       VLOG(1) << "Exceeded timeout of " << timeout.get()
               << " when attempting to get metric '" << key << "'";
     } else if (value.isReady()) {
-      snapshot.emplace_hint(snapshot.end(), key, value.get());
+      snapshot[key] = value.get();
     }
 
-    if (statistics[i].isSome()) {
-      Statistics<double>& statistics_ = statistics[i].get();
-      snapshot.emplace_hint(
-          snapshot.end(),
-          key + "/count",
-          static_cast<double>(statistics_.count));
-      snapshot.emplace_hint(snapshot.end(), key + "/max", statistics_.max);
-      snapshot.emplace_hint(snapshot.end(), key + "/min", statistics_.min);
-      snapshot.emplace_hint(snapshot.end(), key + "/p50", statistics_.p50);
-      snapshot.emplace_hint(snapshot.end(), key + "/p90", statistics_.p90);
-      snapshot.emplace_hint(snapshot.end(), key + "/p95", statistics_.p95);
-      snapshot.emplace_hint(snapshot.end(), key + "/p99", statistics_.p99);
-      snapshot.emplace_hint(snapshot.end(), key + "/p999", statistics_.p999);
-      snapshot.emplace_hint(snapshot.end(), key + "/p9999", statistics_.p9999);
+    Option<Statistics<double>> statistics_ = statistics.get(key).get();
+
+    if (statistics_.isSome()) {
+      snapshot[key + "/count"] = static_cast<double>(statistics_->count);
+      snapshot[key + "/min"] = statistics_->min;
+      snapshot[key + "/max"] = statistics_->max;
+      snapshot[key + "/p50"] = statistics_->p50;
+      snapshot[key + "/p90"] = statistics_->p90;
+      snapshot[key + "/p95"] = statistics_->p95;
+      snapshot[key + "/p99"] = statistics_->p99;
+      snapshot[key + "/p999"] = statistics_->p999;
+      snapshot[key + "/p9999"] = statistics_->p9999;
     }
   }