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 2019/07/30 01:45:18 UTC

[mesos] branch master updated (19e5259 -> 74f0131)

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

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


    from 19e5259  Added unsuppressing via `updateFramework()` to Java example framework.
     new db864ff  Updated PushGauge to work with double.
     new f3e8dd3  Added metrics for quota limits.
     new 74f0131  Added a test for quota guarantee/limit metrics.

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../include/process/metrics/push_gauge.hpp         | 30 +++++--
 src/master/allocator/mesos/metrics.cpp             | 98 +++++++++++++++-------
 src/master/allocator/mesos/metrics.hpp             |  8 +-
 src/tests/hierarchical_allocator_tests.cpp         | 78 +++++++++++++++++
 src/tests/master_quota_tests.cpp                   | 11 ++-
 5 files changed, 181 insertions(+), 44 deletions(-)


[mesos] 03/03: Added a test for quota guarantee/limit metrics.

Posted by bm...@apache.org.
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

commit 74f01313d563ab127fa34cc14383061a7812ddb0
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Mon Jul 29 17:45:26 2019 -0400

    Added a test for quota guarantee/limit metrics.
    
    Review: https://reviews.apache.org/r/71189
---
 src/tests/hierarchical_allocator_tests.cpp | 78 ++++++++++++++++++++++++++++++
 src/tests/master_quota_tests.cpp           | 11 +++--
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index e549951..2c1d0fe 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -4251,10 +4251,24 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(HierarchicalAllocatorTest, DRFWithQuota)
   metric =
     "allocator/mesos/quota"
     "/roles/" + QUOTA_ROLE +
+    "/resources/cpus"
+    "/limit";
+  EXPECT_EQ(0.25, metrics.values[metric]);
+
+  metric =
+    "allocator/mesos/quota"
+    "/roles/" + QUOTA_ROLE +
     "/resources/mem"
     "/guarantee";
   EXPECT_EQ(128, metrics.values[metric]);
 
+  metric =
+    "allocator/mesos/quota"
+    "/roles/" + QUOTA_ROLE +
+    "/resources/mem"
+    "/limit";
+  EXPECT_EQ(128, metrics.values[metric]);
+
   // Add an agent with some allocated resources.
   SlaveInfo agent1 = createSlaveInfo("cpus:1;mem:512;disk:0");
   allocator->addSlave(
@@ -5097,6 +5111,70 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(HierarchicalAllocatorTest, ResourceMetrics)
 }
 
 
+// Ensures that guarantee and limit metrics are exposed
+// and updated correctly.
+TEST_F(HierarchicalAllocatorTest, QuotaMetrics)
+{
+  // Pausing the clock is not necessary, but ensures that the test
+  // doesn't rely on the batch allocation in the allocator, which
+  // would slow down the test.
+  Clock::pause();
+
+  initialize();
+
+  const string cpuGuaranteeKey =
+    "allocator/mesos/quota/roles/role/resources/cpus/guarantee";
+  const string cpuLimitKey =
+    "allocator/mesos/quota/roles/role/resources/cpus/limit";
+  const string memGuaranteeKey =
+    "allocator/mesos/quota/roles/role/resources/mem/guarantee";
+  const string memLimitKey =
+    "allocator/mesos/quota/roles/role/resources/mem/limit";
+
+  JSON::Object metrics = Metrics();
+
+  EXPECT_TRUE(metrics.values.count(cpuGuaranteeKey) == 0);
+  EXPECT_TRUE(metrics.values.count(cpuLimitKey) == 0);
+  EXPECT_TRUE(metrics.values.count(memGuaranteeKey) == 0);
+  EXPECT_TRUE(metrics.values.count(memLimitKey) == 0);
+
+  // Set quota to have:
+  //   * 1 cpu guarantee / no cpu limit and
+  //   * no mem guarantee / 1024 mem limit
+  Quota quota = createQuota("cpus:1", "mem:1024");
+  allocator->updateQuota("role", quota);
+  Clock::settle();
+
+  metrics = Metrics();
+
+  EXPECT_EQ(1, metrics.values[cpuGuaranteeKey]);
+  EXPECT_TRUE(metrics.values.count(cpuLimitKey) == 0);
+  EXPECT_TRUE(metrics.values.count(memGuaranteeKey) == 0);
+  EXPECT_EQ(1024, metrics.values[memLimitKey]);
+
+  // Increase the cpu guarantee:
+  quota = createQuota("cpus:2", "mem:1024");
+  allocator->updateQuota("role", quota);
+  Clock::settle();
+
+  metrics = Metrics();
+
+  EXPECT_EQ(2, metrics.values[cpuGuaranteeKey]);
+
+  // Set back to default quota.
+  quota = createQuota("", "");
+  allocator->updateQuota("role", quota);
+  Clock::settle();
+
+  metrics = Metrics();
+
+  EXPECT_TRUE(metrics.values.count(cpuGuaranteeKey) == 0);
+  EXPECT_TRUE(metrics.values.count(cpuLimitKey) == 0);
+  EXPECT_TRUE(metrics.values.count(memGuaranteeKey) == 0);
+  EXPECT_TRUE(metrics.values.count(memLimitKey) == 0);
+}
+
+
 // The allocator is not fully initialized until `allocator->initialize(...)`
 // is called (e.g., from `Master::initialize()` or
 // `HierarchicalAllocatorTestBase::initialize(...)`). This test
diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp
index 7745fb0..02b1e8d 100644
--- a/src/tests/master_quota_tests.cpp
+++ b/src/tests/master_quota_tests.cpp
@@ -723,13 +723,15 @@ TEST_F(MasterQuotaTest, RemoveSingleQuota)
     // Ensure metrics update is finished.
     Clock::settle();
 
-    const string metricKey =
+    const string guaranteeKey =
       "allocator/mesos/quota/roles/" + ROLE1 + "/resources/cpus/guarantee";
-
+    const string limitKey =
+      "allocator/mesos/quota/roles/" + ROLE1 + "/resources/cpus/limit";
 
     JSON::Object metrics = Metrics();
 
-    EXPECT_EQ(1, metrics.values[metricKey]);
+    EXPECT_EQ(1, metrics.values[guaranteeKey]);
+    EXPECT_EQ(1, metrics.values[limitKey]);
 
     // Remove the previously requested quota.
     Future<Nothing> receivedQuotaRequest;
@@ -747,7 +749,8 @@ TEST_F(MasterQuotaTest, RemoveSingleQuota)
 
     metrics = Metrics();
 
-    ASSERT_NONE(metrics.at<JSON::Number>(metricKey));
+    ASSERT_NONE(metrics.at<JSON::Number>(guaranteeKey));
+    ASSERT_NONE(metrics.at<JSON::Number>(limitKey));
   }
 }
 


[mesos] 01/03: Updated PushGauge to work with double.

Posted by bm...@apache.org.
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

commit db864ff4579371a7b7086f3789093ba27bb62279
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Mon Jul 29 17:38:11 2019 -0400

    Updated PushGauge to work with double.
    
    This enables adoption in Mesos code, since many PullGauges are
    set with double values.
    
    Review: https://reviews.apache.org/r/71187
---
 .../include/process/metrics/push_gauge.hpp         | 30 +++++++++++++++++-----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/3rdparty/libprocess/include/process/metrics/push_gauge.hpp b/3rdparty/libprocess/include/process/metrics/push_gauge.hpp
index 0dd2c20..bce1917 100644
--- a/3rdparty/libprocess/include/process/metrics/push_gauge.hpp
+++ b/3rdparty/libprocess/include/process/metrics/push_gauge.hpp
@@ -58,7 +58,7 @@ public:
     return static_cast<double>(data->value.load());
   }
 
-  PushGauge& operator=(int64_t v)
+  PushGauge& operator=(double v)
   {
     data->value.store(v);
     push(v);
@@ -67,18 +67,36 @@ public:
 
   PushGauge& operator++() { return *this += 1; }
 
-  PushGauge& operator+=(int64_t v)
+  PushGauge& operator+=(double v)
   {
-    int64_t prev = data->value.fetch_add(v);
+    double prev;
+
+    while (true) {
+      prev = data->value.load();
+
+      if (data->value.compare_exchange_weak(prev, prev + v)) {
+        break;
+      }
+    }
+
     push(static_cast<double>(prev + v));
     return *this;
   }
 
   PushGauge& operator--() { return *this -= 1; }
 
-  PushGauge& operator-=(int64_t v)
+  PushGauge& operator-=(double v)
   {
-    int64_t prev = data->value.fetch_sub(v);
+    double prev;
+
+    while (true) {
+      prev = data->value.load();
+
+      if (data->value.compare_exchange_weak(prev, prev - v)) {
+        break;
+      }
+    }
+
     push(static_cast<double>(prev - v));
     return *this;
   }
@@ -88,7 +106,7 @@ private:
   {
     explicit Data() : value(0) {}
 
-    std::atomic<int64_t> value;
+    std::atomic<double> value;
   };
 
   std::shared_ptr<Data> data;


[mesos] 02/03: Added metrics for quota limits.

Posted by bm...@apache.org.
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

commit f3e8dd3186777bd10d3788c3d1e7705482d8779a
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Mon Jul 29 17:41:10 2019 -0400

    Added metrics for quota limits.
    
    This is done similarly to to the existing quota guarantee metrics.
    
    However, the logic is adjusted given the presence of bugs in the
    existing implementation. In the case that a non-default quota gets
    updated to a new value, the existing logic would fail to add the
    new metric and the metric exposed to the user would be stuck with
    the original value. To resolve this, we now remove the original
    metrics and subsequently add the new ones.
    
    Review: https://reviews.apache.org/r/71188
---
 src/master/allocator/mesos/metrics.cpp | 98 +++++++++++++++++++++++-----------
 src/master/allocator/mesos/metrics.hpp |  8 ++-
 2 files changed, 72 insertions(+), 34 deletions(-)

diff --git a/src/master/allocator/mesos/metrics.cpp b/src/master/allocator/mesos/metrics.cpp
index 2d72757..f8465be 100644
--- a/src/master/allocator/mesos/metrics.cpp
+++ b/src/master/allocator/mesos/metrics.cpp
@@ -111,13 +111,19 @@ Metrics::~Metrics()
   }
 
   foreachkey (const string& role, quota_allocated) {
-    foreachvalue (const PullGauge& gauge, quota_allocated[role]) {
+    foreachvalue (const PullGauge& gauge, quota_allocated.at(role)) {
       process::metrics::remove(gauge);
     }
   }
 
   foreachkey (const string& role, quota_guarantee) {
-    foreachvalue (const PullGauge& gauge, quota_guarantee[role]) {
+    foreachvalue (const PushGauge& gauge, quota_guarantee.at(role)) {
+      process::metrics::remove(gauge);
+    }
+  }
+
+  foreachkey (const string& role, quota_limit) {
+    foreachvalue (const PushGauge& gauge, quota_limit.at(role)) {
       process::metrics::remove(gauge);
     }
   }
@@ -128,61 +134,89 @@ Metrics::~Metrics()
 }
 
 
-// TODO(mzhu): This currently only updates quota guarantees.
-// Add metrics for quota limits as well.
 void Metrics::updateQuota(const string& role, const Quota& quota)
 {
-  // This is the "remove" case where the role's quota
-  // is set to the default.
-  if (quota.guarantees == DEFAULT_QUOTA.guarantees) {
-    // TODO(mzhu): always expose the allocated gauge event if
-    // a role only has default quota.
-    foreachvalue (const PullGauge& gauge, quota_allocated[role]) {
-      process::metrics::remove(gauge);
-    }
+  // First remove the existing metrics.
+  foreachvalue (const PullGauge& gauge, quota_allocated[role]) {
+    process::metrics::remove(gauge);
+  }
+  quota_allocated.erase(role);
 
-    foreachvalue (const PullGauge& gauge, quota_guarantee[role]) {
-      process::metrics::remove(gauge);
-    }
+  foreachvalue (const PushGauge& gauge, quota_guarantee[role]) {
+    process::metrics::remove(gauge);
+  }
+  quota_guarantee.erase(role);
 
-    quota_allocated.erase(role);
-    quota_guarantee.erase(role);
+  foreachvalue (const PushGauge& gauge, quota_limit[role]) {
+    process::metrics::remove(gauge);
+  }
+  quota_limit.erase(role);
 
+  // This is the original quota "remove" case where the
+  // role's quota is set to the default. We used to not
+  // show the quota allocated_or_offered metric in this
+  // case, so we keep this behavior for now.
+  //
+  // TODO(mzhu): always expose the allocated gauge even
+  // if a role only has default quota.
+  if (quota == DEFAULT_QUOTA) {
     return;
   }
 
-  hashmap<string, PullGauge> allocated;
-  hashmap<string, PullGauge> guarantees;
+  // Now add the new metrics.
 
   foreach (auto& quantity, quota.guarantees) {
-    double value = quantity.second.value();
+    PushGauge guarantee(
+        "allocator/mesos/quota"
+        "/roles/" + role +
+        "/resources/" + quantity.first +
+        "/guarantee");
+
+    guarantee = quantity.second.value();
 
-    PullGauge guarantee = PullGauge(
+    process::metrics::add(guarantee);
+
+    quota_guarantee[role].put(quantity.first, guarantee);
+  }
+
+  foreach (auto& quantity, quota.limits) {
+    PushGauge limit(
         "allocator/mesos/quota"
         "/roles/" + role +
         "/resources/" + quantity.first +
-        "/guarantee",
-        process::defer([value]() { return value; }));
+        "/limit");
+
+    limit = quantity.second.value();
+
+    process::metrics::add(limit);
+
+    quota_limit[role].put(quantity.first, limit);
+  }
+
+  hashset<string> names;
 
+  foreach (auto& quantity, quota.guarantees) {
+    names.insert(quantity.first);
+  }
+  foreach (auto& quantity, quota.limits) {
+    names.insert(quantity.first);
+  }
+
+  foreach (const string& name, names) {
     PullGauge offered_or_allocated(
         "allocator/mesos/quota"
         "/roles/" + role +
-        "/resources/" + quantity.first +
+        "/resources/" + name +
         "/offered_or_allocated",
         defer(allocator,
               &HierarchicalAllocatorProcess::_quota_allocated,
               role,
-              quantity.first));
+              name));
 
-    guarantees.put(quantity.first, guarantee);
-    allocated.put(quantity.first, offered_or_allocated);
-
-    process::metrics::add(guarantee);
     process::metrics::add(offered_or_allocated);
-  }
 
-  quota_allocated[role] = allocated;
-  quota_guarantee[role] = guarantees;
+    quota_allocated[role].put(name, offered_or_allocated);
+  }
 }
 
 
diff --git a/src/master/allocator/mesos/metrics.hpp b/src/master/allocator/mesos/metrics.hpp
index fa2a5dc..adb9515 100644
--- a/src/master/allocator/mesos/metrics.hpp
+++ b/src/master/allocator/mesos/metrics.hpp
@@ -82,10 +82,14 @@ struct Metrics
   hashmap<std::string, hashmap<std::string, process::metrics::PullGauge>>
     quota_allocated;
 
-  // PullGauges for the per-role quota guarantee for each resource.
-  hashmap<std::string, hashmap<std::string, process::metrics::PullGauge>>
+  // PushGauges for the per-role quota guarantee for each resource.
+  hashmap<std::string, hashmap<std::string, process::metrics::PushGauge>>
     quota_guarantee;
 
+  // PushGauges for the per-role quota limit for each resource.
+  hashmap<std::string, hashmap<std::string, process::metrics::PushGauge>>
+    quota_limit;
+
   // PullGauges for the per-role count of active offer filters.
   hashmap<std::string, process::metrics::PullGauge> offer_filters_active;
 };