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/10 03:37:47 UTC

[mesos] 04/06: Added quota.consumed field to /roles endpoint.

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 f1b484996c856292c8c0a15978e64d3adf14f292
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Mon Jul 1 15:18:35 2019 -0400

    Added quota.consumed field to /roles endpoint.
    
    Per the previous commit that adds the consumed quota computation,
    consumed quota is:
    
      Allocation + Unallocated Reservation ==
      Reservations + Unreserved Allocation
    
    That is, reservations count towards quota regardless of whether
    they're allocated. Allocation counts towards quota. Offered resources
    *do not* count towards quota, this is to (1) provide stability of the
    quota consumption metrics in the face of offers flowing in and out,
    and (2) to ensure that we treat offers as rescindable and therefore
    not yet "counting" towards quota. Also, if in the future schedulers
    are offered more than their quota to improve their choices, counting
    offered resources as consumed quota will be problematic.
    
    Review: https://reviews.apache.org/r/71030
---
 src/master/readonly_handler.cpp |  45 +++++++------
 src/tests/role_tests.cpp        | 144 ++++++++++++++++++++++++++--------------
 2 files changed, 119 insertions(+), 70 deletions(-)

diff --git a/src/master/readonly_handler.cpp b/src/master/readonly_handler.cpp
index 5772b7c..27965e3 100644
--- a/src/master/readonly_handler.cpp
+++ b/src/master/readonly_handler.cpp
@@ -712,28 +712,33 @@ process::http::Response Master::ReadOnlyHandler::roles(
               // Default weight is 1.0.
               writer->field("weight", master->weights.get(name).getOrElse(1.0));
 
-              if (master->quotas.contains(name)) {
-                // Prior to Mesos 1.9, this field is filled based on
-                // `QuotaInfo` which is now deprecated. For backward
-                // compatibility reasons, we do not use any formatter
-                // for the new struct but construct the response by hand.
-                // Specifically:
-                //
-                //  - We keep the `role` field which was present in the
-                //    `QuotaInfo`.
-                //
-                //  - We name the field using singular `guarantee` and `limit`
-                //    which is different from the plural used in `QuotaConfig`.
-                const Quota& quota = master->quotas.at(name);
-                writer->field("quota", [&](JSON::ObjectWriter* writer) {
-                  writer->field("role", name);
-                  writer->field("guarantee", quota.guarantees);
-                  writer->field("limit", quota.limits);
-                });
-              }
-
               Option<Role*> role = master->roles.get(name);
 
+              // Prior to Mesos 1.9, this field is filled based on
+              // `QuotaInfo` which is now deprecated. For backward
+              // compatibility reasons, we do not use any formatter
+              // for the new struct but construct the response by hand.
+              // Specifically:
+              //
+              //  - We keep the `role` field which was present in the
+              //    `QuotaInfo`.
+              //
+              //  - We name the field using singular `guarantee` and `limit`
+              //    which is different from the plural used in `QuotaConfig`.
+              const Quota quota = master->quotas.get(name).getOrElse(Quota());
+
+              writer->field("quota", [&](JSON::ObjectWriter* writer) {
+                writer->field("role", name);
+
+                writer->field("guarantee", quota.guarantees);
+                writer->field("limit", quota.limits);
+
+                ResourceQuantities consumed = role.isSome() ?
+                  (*role)->consumedQuota() : ResourceQuantities();
+
+                writer->field("consumed", consumed);
+              });
+
               if (role.isNone()) {
                 writer->field("resources", Resources());
               } else {
diff --git a/src/tests/role_tests.cpp b/src/tests/role_tests.cpp
index bf5b3cb..384b1d5 100644
--- a/src/tests/role_tests.cpp
+++ b/src/tests/role_tests.cpp
@@ -309,7 +309,10 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(RoleTest, EndpointEmpty)
       "}");
 
   ASSERT_SOME(expected);
-  EXPECT_EQ(expected.get(), parse.get());
+
+  EXPECT_EQ(*expected, *parse)
+    << "expected " << stringify(*expected)
+    << " vs actual " << stringify(*parse);
 }
 
 
@@ -345,10 +348,13 @@ TEST_F(RoleTest, EndpointNoFrameworks)
       "      \"frameworks\": [],"
       "      \"name\": \"*\","
       "      \"resources\": {"
-      "        \"cpus\": 0,"
-      "        \"disk\": 0,"
-      "        \"gpus\": 0,"
-      "        \"mem\":  0"
+      "        \"cpus\": 0, \"disk\": 0, \"gpus\": 0, \"mem\": 0"
+      "      },"
+      "      \"quota\": {"
+      "        \"role\": \"*\","
+      "        \"consumed\": {},"
+      "        \"guarantee\": {},"
+      "        \"limit\": {}"
       "      },"
       "      \"weight\": 1.0"
       "    },"
@@ -356,10 +362,13 @@ TEST_F(RoleTest, EndpointNoFrameworks)
       "      \"frameworks\": [],"
       "      \"name\": \"role1\","
       "      \"resources\": {"
-      "        \"cpus\": 0,"
-      "        \"disk\": 0,"
-      "        \"gpus\": 0,"
-      "        \"mem\":  0"
+      "        \"cpus\": 0, \"disk\": 0, \"gpus\": 0, \"mem\": 0"
+      "      },"
+      "      \"quota\": {"
+      "        \"role\": \"role1\","
+      "        \"consumed\": {},"
+      "        \"guarantee\": {},"
+      "        \"limit\": {}"
       "      },"
       "      \"weight\": 5.0"
       "    },"
@@ -367,10 +376,13 @@ TEST_F(RoleTest, EndpointNoFrameworks)
       "      \"frameworks\": [],"
       "      \"name\": \"role2\","
       "      \"resources\": {"
-      "        \"cpus\": 0,"
-      "        \"disk\": 0,"
-      "        \"gpus\": 0,"
-      "        \"mem\":  0"
+      "        \"cpus\": 0, \"disk\": 0, \"gpus\": 0, \"mem\": 0"
+      "      },"
+      "      \"quota\": {"
+      "        \"role\": \"role2\","
+      "        \"consumed\": {},"
+      "        \"guarantee\": {},"
+      "        \"limit\": {}"
       "      },"
       "      \"weight\": 1.0"
       "    }"
@@ -378,7 +390,10 @@ TEST_F(RoleTest, EndpointNoFrameworks)
       "}");
 
   ASSERT_SOME(expected);
-  EXPECT_EQ(expected.get(), parse.get());
+
+  EXPECT_EQ(*expected, *parse)
+    << "expected " << stringify(*expected)
+    << " vs actual " << stringify(*parse);
 }
 
 
@@ -433,22 +448,25 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(RoleTest, RolesEndpointContainsQuota)
           "\"quota\":"
             "{"
               "\"role\":\"foo\","
+              "\"consumed\": {},"
               "\"guarantee\":"
                 "{"
                   "\"cpus\":1.0,"
                   "\"mem\":512.0"
                 "},"
-                "\"limit\":"
-                  "{"
-                    "\"cpus\":1.0,"
-                    "\"mem\":512.0"
-                  "}"
+              "\"limit\":"
+                "{"
+                  "\"cpus\":1.0,"
+                  "\"mem\":512.0"
+                "}"
             "}"
         "}"
     );
     ASSERT_SOME(expected);
 
-    EXPECT_TRUE(role.contains(expected.get()));
+    EXPECT_TRUE(role.contains(*expected))
+      << "expected " << stringify(*expected)
+      << " vs actual " << stringify(role);
   }
 }
 
@@ -513,10 +531,13 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(RoleTest, EndpointImplicitRolesWeights)
       "      \"frameworks\": [\"" + frameworkId1->value() + "\"],"
       "      \"name\": \"roleX\","
       "      \"resources\": {"
-      "        \"cpus\": 0,"
-      "        \"disk\": 0,"
-      "        \"gpus\": 0,"
-      "        \"mem\":  0"
+      "        \"cpus\": 0, \"disk\": 0, \"gpus\": 0, \"mem\": 0"
+      "      },"
+      "      \"quota\": {"
+      "        \"role\": \"roleX\","
+      "        \"consumed\": {},"
+      "        \"guarantee\": {},"
+      "        \"limit\": {}"
       "      },"
       "      \"weight\": 5.0"
       "    },"
@@ -524,10 +545,13 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(RoleTest, EndpointImplicitRolesWeights)
       "      \"frameworks\": [],"
       "      \"name\": \"roleY\","
       "      \"resources\": {"
-      "        \"cpus\": 0,"
-      "        \"disk\": 0,"
-      "        \"gpus\": 0,"
-      "        \"mem\":  0"
+      "        \"cpus\": 0, \"disk\": 0, \"gpus\": 0, \"mem\": 0"
+      "      },"
+      "      \"quota\": {"
+      "        \"role\": \"roleY\","
+      "        \"consumed\": {},"
+      "        \"guarantee\": {},"
+      "        \"limit\": {}"
       "      },"
       "      \"weight\": 4.0"
       "    },"
@@ -535,10 +559,13 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(RoleTest, EndpointImplicitRolesWeights)
       "      \"frameworks\": [\"" + frameworkId2->value() + "\"],"
       "      \"name\": \"roleZ\","
       "      \"resources\": {"
-      "        \"cpus\": 0,"
-      "        \"disk\": 0,"
-      "        \"gpus\": 0,"
-      "        \"mem\":  0"
+      "        \"cpus\": 0, \"disk\": 0, \"gpus\": 0, \"mem\": 0"
+      "      },"
+      "      \"quota\": {"
+      "        \"role\": \"roleZ\","
+      "        \"consumed\": {},"
+      "        \"guarantee\": {},"
+      "        \"limit\": {}"
       "      },"
       "      \"weight\": 1.0"
       "    }"
@@ -546,7 +573,10 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(RoleTest, EndpointImplicitRolesWeights)
       "}");
 
   ASSERT_SOME(expected);
-  EXPECT_EQ(expected.get(), parse.get());
+
+  EXPECT_EQ(*expected, *parse)
+    << "expected " << stringify(*expected)
+    << " vs actual " << stringify(*parse);
 
   driver1.stop();
   driver1.join();
@@ -607,10 +637,13 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(RoleTest, EndpointImplicitRolesQuotas)
       "      \"frameworks\": [],"
       "      \"name\": \"non-existent-role\","
       "      \"resources\": {"
-      "        \"cpus\": 0,"
-      "        \"disk\": 0,"
-      "        \"gpus\": 0,"
-      "        \"mem\":  0"
+      "        \"cpus\": 0, \"disk\": 0, \"gpus\": 0, \"mem\": 0"
+      "      },"
+      "      \"quota\": {"
+      "        \"role\": \"non-existent-role\","
+      "        \"consumed\": {},"
+      "        \"guarantee\": {},"
+      "        \"limit\": {}"
       "      },"
       "      \"weight\": 1.0"
       "    }"
@@ -651,8 +684,10 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(RoleTest, EndpointImplicitRolesQuotas)
       "}");
 
   ASSERT_SOME(expected);
-  EXPECT_EQ(expected.get(), parse.get());
-}
+
+  EXPECT_EQ(*expected, *parse)
+    << "expected " << stringify(*expected)
+    << " vs actual " << stringify(*parse);}
 
 
 // This test ensures that master adds/removes all roles of
@@ -705,10 +740,13 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(
         "      \"frameworks\": [\"" + frameworkId->value() + "\"],"
         "      \"name\": \"role1\","
         "      \"resources\": {"
-        "        \"cpus\": 0,"
-        "        \"disk\": 0,"
-        "        \"gpus\": 0,"
-        "        \"mem\":  0"
+        "        \"cpus\": 0, \"disk\": 0, \"gpus\": 0, \"mem\": 0"
+        "      },"
+        "      \"quota\": {"
+        "        \"role\": \"role1\","
+        "        \"consumed\": {},"
+        "        \"guarantee\": {},"
+        "        \"limit\": {}"
         "      },"
         "      \"weight\": 1.0"
         "    },"
@@ -716,10 +754,13 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(
         "      \"frameworks\": [\"" + frameworkId->value() + "\"],"
         "      \"name\": \"role2\","
         "      \"resources\": {"
-        "        \"cpus\": 0,"
-        "        \"disk\": 0,"
-        "        \"gpus\": 0,"
-        "        \"mem\":  0"
+        "        \"cpus\": 0, \"disk\": 0, \"gpus\": 0, \"mem\": 0"
+        "      },"
+        "      \"quota\": {"
+        "        \"role\": \"role2\","
+        "        \"consumed\": {},"
+        "        \"guarantee\": {},"
+        "        \"limit\": {}"
         "      },"
         "      \"weight\": 1.0"
         "    }"
@@ -728,7 +769,9 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(
 
     ASSERT_SOME(expected);
 
-    EXPECT_EQ(expected.get(), parse.get());
+    EXPECT_EQ(*expected, *parse)
+      << "expected " << stringify(*expected)
+      << " vs actual " << stringify(*parse);
   }
 
   // Set expectation that Master receives teardown call.
@@ -764,8 +807,9 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(
 
     ASSERT_SOME(expected);
 
-    EXPECT_EQ(expected.get(), parse.get());
-  }
+    EXPECT_EQ(*expected, *parse)
+      << "expected " << stringify(*expected)
+      << " vs actual " << stringify(*parse);  }
 }