You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2015/06/11 00:10:21 UTC

[2/2] mesos git commit: Minor cleanup to master/slave metric logic.

Minor cleanup to master/slave metric logic.

Review: https://reviews.apache.org/r/35312


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

Branch: refs/heads/master
Commit: 2761bbab923db40ceac4a8582dcb415c33a9f7a0
Parents: c7ba41e
Author: Benjamin Mahler <be...@gmail.com>
Authored: Wed Jun 10 11:35:54 2015 -0700
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Wed Jun 10 15:05:09 2015 -0700

----------------------------------------------------------------------
 src/master/master.cpp  | 24 ++++++------
 src/master/metrics.cpp | 94 ++++++++++++++++++++++++++-------------------
 src/master/metrics.hpp |  7 ++--
 src/slave/metrics.cpp  | 34 +++++++++-------
 src/slave/slave.cpp    | 12 +++---
 5 files changed, 97 insertions(+), 74 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2761bbab/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 15f2132..95ca2e5 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -5372,7 +5372,7 @@ double Master::_tasks_running()
 }
 
 
-double Master::_resources_total(const std::string& name)
+double Master::_resources_total(const string& name)
 {
   double total = 0.0;
 
@@ -5388,7 +5388,7 @@ double Master::_resources_total(const std::string& name)
 }
 
 
-double Master::_resources_used(const std::string& name)
+double Master::_resources_used(const string& name)
 {
   double used = 0.0;
 
@@ -5406,19 +5406,19 @@ double Master::_resources_used(const std::string& name)
 }
 
 
-double Master::_resources_percent(const std::string& name)
+double Master::_resources_percent(const string& name)
 {
   double total = _resources_total(name);
 
   if (total == 0.0) {
-    return total;
-  } else {
-    return _resources_used(name) / total;
+    return 0.0;
   }
+
+  return _resources_used(name) / total;
 }
 
 
-double Master::_resources_revocable_total(const std::string& name)
+double Master::_resources_revocable_total(const string& name)
 {
   double total = 0.0;
 
@@ -5434,7 +5434,7 @@ double Master::_resources_revocable_total(const std::string& name)
 }
 
 
-double Master::_resources_revocable_used(const std::string& name)
+double Master::_resources_revocable_used(const string& name)
 {
   double used = 0.0;
 
@@ -5452,15 +5452,15 @@ double Master::_resources_revocable_used(const std::string& name)
 }
 
 
-double Master::_resources_revocable_percent(const std::string& name)
+double Master::_resources_revocable_percent(const string& name)
 {
   double total = _resources_revocable_total(name);
 
   if (total == 0.0) {
-    return total;
-  } else {
-    return _resources_revocable_used(name) / total;
+    return 0.0;
   }
+
+  return _resources_revocable_used(name) / total;
 }
 
 } // namespace master {

http://git-wip-us.apache.org/repos/asf/mesos/blob/2761bbab/src/master/metrics.cpp
----------------------------------------------------------------------
diff --git a/src/master/metrics.cpp b/src/master/metrics.cpp
index d2489c8..10e2937 100644
--- a/src/master/metrics.cpp
+++ b/src/master/metrics.cpp
@@ -16,11 +16,22 @@
  * limitations under the License.
  */
 
+#include <string>
+
+#include <process/metrics/counter.hpp>
+#include <process/metrics/gauge.hpp>
+#include <process/metrics/metrics.hpp>
+
 #include <stout/foreach.hpp>
 
 #include "master/master.hpp"
 #include "master/metrics.hpp"
 
+using process::metrics::Counter;
+using process::metrics::Gauge;
+
+using std::string;
+
 namespace mesos {
 namespace internal {
 namespace master {
@@ -244,48 +255,50 @@ Metrics::Metrics(const Master& master)
   // Create resource gauges.
   // TODO(dhamon): Set these up dynamically when adding a slave based on the
   // resources the slave exposes.
-  const std::string resources[] = {"cpus", "mem", "disk"};
+  const string resources[] = {"cpus", "mem", "disk"};
 
-  // Regular (non-revocable) resources.
-  foreach (const std::string& resource, resources) {
-    process::metrics::Gauge totalGauge(
+  foreach (const string& resource, resources) {
+    Gauge total(
         "master/" + resource + "_total",
         defer(master, &Master::_resources_total, resource));
-    resources_total.push_back(totalGauge);
-    process::metrics::add(totalGauge);
 
-    process::metrics::Gauge usedGauge(
+    Gauge used(
         "master/" + resource + "_used",
         defer(master, &Master::_resources_used, resource));
-    resources_used.push_back(usedGauge);
-    process::metrics::add(usedGauge);
 
-    process::metrics::Gauge percentGauge(
+    Gauge percent(
         "master/" + resource + "_percent",
         defer(master, &Master::_resources_percent, resource));
-    resources_percent.push_back(percentGauge);
-    process::metrics::add(percentGauge);
+
+    resources_total.push_back(total);
+    resources_used.push_back(used);
+    resources_percent.push_back(percent);
+
+    process::metrics::add(total);
+    process::metrics::add(used);
+    process::metrics::add(percent);
   }
 
-  // Revocable resources.
-  foreach (const std::string& resource, resources) {
-    process::metrics::Gauge totalGauge(
+  foreach (const string& resource, resources) {
+    Gauge total(
         "master/" + resource + "_revocable_total",
         defer(master, &Master::_resources_revocable_total, resource));
-    resources_revocable_total.push_back(totalGauge);
-    process::metrics::add(totalGauge);
 
-    process::metrics::Gauge usedGauge(
+    Gauge used(
         "master/" + resource + "_revocable_used",
         defer(master, &Master::_resources_revocable_used, resource));
-    resources_revocable_used.push_back(usedGauge);
-    process::metrics::add(usedGauge);
 
-    process::metrics::Gauge percentGauge(
+    Gauge percent(
         "master/" + resource + "_revocable_percent",
         defer(master, &Master::_resources_revocable_percent, resource));
-    resources_revocable_percent.push_back(percentGauge);
-    process::metrics::add(percentGauge);
+
+    resources_revocable_total.push_back(total);
+    resources_revocable_used.push_back(used);
+    resources_revocable_percent.push_back(percent);
+
+    process::metrics::add(total);
+    process::metrics::add(used);
+    process::metrics::add(percent);
   }
 }
 
@@ -370,39 +383,39 @@ Metrics::~Metrics()
   process::metrics::remove(slave_shutdowns_completed);
   process::metrics::remove(slave_shutdowns_canceled);
 
-  foreach (const process::metrics::Gauge& gauge, resources_total) {
+  foreach (const Gauge& gauge, resources_total) {
     process::metrics::remove(gauge);
   }
   resources_total.clear();
 
-  foreach (const process::metrics::Gauge& gauge, resources_used) {
+  foreach (const Gauge& gauge, resources_used) {
     process::metrics::remove(gauge);
   }
   resources_used.clear();
 
-  foreach (const process::metrics::Gauge& gauge, resources_percent) {
+  foreach (const Gauge& gauge, resources_percent) {
     process::metrics::remove(gauge);
   }
   resources_percent.clear();
 
-  foreach (const process::metrics::Gauge& gauge, resources_revocable_total) {
+  foreach (const Gauge& gauge, resources_revocable_total) {
     process::metrics::remove(gauge);
   }
   resources_revocable_total.clear();
 
-  foreach (const process::metrics::Gauge& gauge, resources_revocable_used) {
+  foreach (const Gauge& gauge, resources_revocable_used) {
     process::metrics::remove(gauge);
   }
   resources_revocable_used.clear();
 
-  foreach (const process::metrics::Gauge& gauge, resources_revocable_percent) {
+  foreach (const Gauge& gauge, resources_revocable_percent) {
     process::metrics::remove(gauge);
   }
   resources_revocable_percent.clear();
 
   foreachvalue (const auto& source_reason, tasks_states) {
     foreachvalue (const auto& reason_counter, source_reason) {
-      foreachvalue (const process::metrics::Counter& counter, reason_counter) {
+      foreachvalue (const Counter& counter, reason_counter) {
         process::metrics::remove(counter);
       }
     }
@@ -412,27 +425,30 @@ Metrics::~Metrics()
 
 
 void Metrics::incrementTasksStates(
-    TaskState state, TaskStatus::Source source, TaskStatus::Reason reason)
+    const TaskState& state,
+    const TaskStatus::Source& source,
+    const TaskStatus::Reason& reason)
 {
   if (!tasks_states.contains(state)) {
     tasks_states[state] = SourcesReasons();
   }
+
   if (!tasks_states[state].contains(source)) {
     tasks_states[state][source] = Reasons();
   }
+
   if (!tasks_states[state][source].contains(reason)) {
-    process::metrics::Counter counter =
-        process::metrics::Counter(
-            "master/" +
-            strings::lower(TaskState_Name(state)) + "/" +
-            strings::lower(TaskStatus::Source_Name(source)) + "/" +
-            strings::lower(TaskStatus::Reason_Name(reason)));
+    Counter counter = Counter(
+        "master/" +
+        strings::lower(TaskState_Name(state)) + "/" +
+        strings::lower(TaskStatus::Source_Name(source)) + "/" +
+        strings::lower(TaskStatus::Reason_Name(reason)));
+
     tasks_states[state][source].put(reason, counter);
     process::metrics::add(counter);
   }
 
-  process::metrics::Counter counter =
-    tasks_states[state][source].get(reason).get();
+  Counter counter = tasks_states[state][source].get(reason).get();
   counter++;
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/2761bbab/src/master/metrics.hpp
----------------------------------------------------------------------
diff --git a/src/master/metrics.hpp b/src/master/metrics.hpp
index 3d389e6..d37d74a 100644
--- a/src/master/metrics.hpp
+++ b/src/master/metrics.hpp
@@ -173,8 +173,7 @@ struct Metrics
   process::metrics::Counter slave_shutdowns_completed;
   process::metrics::Counter slave_shutdowns_canceled;
 
-  // Resource metrics.
-  // Regular (non-revocable) resources.
+  // Non-revocable resources.
   std::vector<process::metrics::Gauge> resources_total;
   std::vector<process::metrics::Gauge> resources_used;
   std::vector<process::metrics::Gauge> resources_percent;
@@ -185,7 +184,9 @@ struct Metrics
   std::vector<process::metrics::Gauge> resources_revocable_percent;
 
   void incrementTasksStates(
-      TaskState state, TaskStatus::Source source, TaskStatus::Reason reason);
+      const TaskState& state,
+      const TaskStatus::Source& source,
+      const TaskStatus::Reason& reason);
 };
 
 } // namespace master {

http://git-wip-us.apache.org/repos/asf/mesos/blob/2761bbab/src/slave/metrics.cpp
----------------------------------------------------------------------
diff --git a/src/slave/metrics.cpp b/src/slave/metrics.cpp
index 7a31ce7..af6678f 100644
--- a/src/slave/metrics.cpp
+++ b/src/slave/metrics.cpp
@@ -18,6 +18,7 @@
 
 #include <string>
 
+#include <process/metrics/gauge.hpp>
 #include <process/metrics/metrics.hpp>
 
 #include <stout/foreach.hpp>
@@ -25,6 +26,9 @@
 #include "slave/metrics.hpp"
 #include "slave/slave.hpp"
 
+using process::metrics::Gauge;
+
+using std::string;
 
 namespace mesos {
 namespace internal {
@@ -114,26 +118,28 @@ Metrics::Metrics(const Slave& slave)
   // Create resource gauges.
   // TODO(dhamon): Set these up dynamically when creating a slave
   // based on the resources it exposes.
-  const std::string resources[] = {"cpus", "mem", "disk"};
+  const string resources[] = {"cpus", "mem", "disk"};
 
-  foreach (const std::string& resource, resources) {
-    process::metrics::Gauge totalGauge(
+  foreach (const string& resource, resources) {
+    Gauge total(
         "slave/" + resource + "_total",
         defer(slave, &Slave::_resources_total, resource));
-    resources_total.push_back(totalGauge);
-    process::metrics::add(totalGauge);
 
-    process::metrics::Gauge usedGauge(
+    Gauge used(
         "slave/" + resource + "_used",
         defer(slave, &Slave::_resources_used, resource));
-    resources_used.push_back(usedGauge);
-    process::metrics::add(usedGauge);
 
-    process::metrics::Gauge percentGauge(
+    Gauge percent(
         "slave/" + resource + "_percent",
         defer(slave, &Slave::_resources_percent, resource));
-    resources_percent.push_back(percentGauge);
-    process::metrics::add(percentGauge);
+
+    resources_total.push_back(total);
+    resources_used.push_back(used);
+    resources_percent.push_back(percent);
+
+    process::metrics::add(total);
+    process::metrics::add(used);
+    process::metrics::add(percent);
   }
 }
 
@@ -169,17 +175,17 @@ Metrics::~Metrics()
 
   process::metrics::remove(executor_directory_max_allowed_age_secs);
 
-  foreach (const process::metrics::Gauge& gauge, resources_total) {
+  foreach (const Gauge& gauge, resources_total) {
     process::metrics::remove(gauge);
   }
   resources_total.clear();
 
-  foreach (const process::metrics::Gauge& gauge, resources_used) {
+  foreach (const Gauge& gauge, resources_used) {
     process::metrics::remove(gauge);
   }
   resources_used.clear();
 
-  foreach (const process::metrics::Gauge& gauge, resources_percent) {
+  foreach (const Gauge& gauge, resources_percent) {
     process::metrics::remove(gauge);
   }
   resources_percent.clear();

http://git-wip-us.apache.org/repos/asf/mesos/blob/2761bbab/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index bb9dd07..f6b07d1 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -4327,7 +4327,7 @@ void Slave::sendExecutorTerminatedStatusUpdate(
 }
 
 
-double Slave::_resources_total(const std::string& name)
+double Slave::_resources_total(const string& name)
 {
   double total = 0.0;
 
@@ -4341,7 +4341,7 @@ double Slave::_resources_total(const std::string& name)
 }
 
 
-double Slave::_resources_used(const std::string& name)
+double Slave::_resources_used(const string& name)
 {
   double used = 0.0;
 
@@ -4359,15 +4359,15 @@ double Slave::_resources_used(const std::string& name)
 }
 
 
-double Slave::_resources_percent(const std::string& name)
+double Slave::_resources_percent(const string& name)
 {
   double total = _resources_total(name);
 
   if (total == 0.0) {
-    return total;
-  } else {
-    return _resources_used(name) / total;
+    return 0.0;
   }
+
+  return _resources_used(name) / total;
 }