You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mz...@apache.org on 2019/05/16 15:41:46 UTC

[mesos] branch 1.8.x updated (9424fd8 -> 6c6546d)

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

mzhu pushed a change to branch 1.8.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from 9424fd8  Added MESOS-9782 to the 1.8.1 CHANGELOG.
     new 91c4a8e  Fix a bug where racing quota removal request could crash the master.
     new 6c6546d  Added MESOS-9786 to the 1.8.1 CHANGELOG.

The 2 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:
 CHANGELOG                        |  1 +
 src/master/quota_handler.cpp     |  7 +++++
 src/tests/master_quota_tests.cpp | 60 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+)


[mesos] 01/02: Fix a bug where racing quota removal request could crash the master.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch 1.8.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 91c4a8eb07fd646df479cc142190507aae0c1701
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Thu May 16 12:12:15 2019 +0200

    Fix a bug where racing quota removal request could crash the master.
    
    Also added a test.
    
    Review: https://reviews.apache.org/r/70656
---
 src/master/quota_handler.cpp     |  7 +++++
 src/tests/master_quota_tests.cpp | 60 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp
index 5d449e6..a18d8ba 100644
--- a/src/master/quota_handler.cpp
+++ b/src/master/quota_handler.cpp
@@ -735,6 +735,13 @@ Future<http::Response> Master::QuotaHandler::_remove(
 
 Future<http::Response> Master::QuotaHandler::__remove(const string& role) const
 {
+  // Double check if the quota still exists. It may have been removed
+  // by a previous removal already.
+  if (!master->quotas.contains(role)) {
+    return BadRequest(
+        "Failed to remove quota: Role '" + role + "' has no quota set");
+  }
+
   // Remove quota from the quota-related local state. We do this before
   // updating the registry in order to make sure that we are not already
   // trying to remove quota for this role (since this is a multi-phase event).
diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp
index 66641f2..146b977 100644
--- a/src/tests/master_quota_tests.cpp
+++ b/src/tests/master_quota_tests.cpp
@@ -65,6 +65,7 @@ using process::Clock;
 using process::Future;
 using process::Owned;
 using process::PID;
+using process::Promise;
 
 using process::http::BadRequest;
 using process::http::Conflict;
@@ -508,6 +509,65 @@ TEST_F(MasterQuotaTest, RemoveSingleQuota)
 }
 
 
+// This test ensures that master can handle two racing quota removal requests.
+// This is a regression test for MESOS-9786.
+TEST_F(MasterQuotaTest, RemoveQuotaRace)
+{
+  Clock::pause();
+
+  // Start master with a mock authorizer to block quota removal requests.
+  MockAuthorizer authorizer;
+
+  Try<Owned<cluster::Master>> master = StartMaster(&authorizer);
+  ASSERT_SOME(master);
+
+  // Use the force flag for setting quota that cannot be satisfied in
+  // this empty cluster without any agents.
+  const bool FORCE = true;
+
+  // Wrap the `http::requestDelete` into a lambda for readability of the test.
+  auto removeQuota = [&master](const string& path) {
+    return process::http::requestDelete(
+        master.get()->pid,
+        path,
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+  };
+
+  // Set quota for `ROLE1`.
+  {
+    Resources quotaResources = Resources::parse("cpus:1;mem:512").get();
+
+    Future<Response> response = process::http::post(
+        master.get()->pid,
+        "quota",
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL),
+        createRequestBody(ROLE1, quotaResources, FORCE));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+  }
+
+  // Intercept both quota removal authorizations.
+  Future<Nothing> authorize1, authorize2;
+  Promise<bool> promise1, promise2;
+  EXPECT_CALL(authorizer, authorized(_))
+    .WillOnce(DoAll(FutureSatisfy(&authorize1), Return(promise1.future())))
+    .WillOnce(DoAll(FutureSatisfy(&authorize2), Return(promise2.future())));
+
+  Future<Response> response1 = removeQuota("quota/" + ROLE1);
+  AWAIT_READY(authorize1);
+
+  Future<Response> response2 = removeQuota("quota/" + ROLE1);
+  AWAIT_READY(authorize2);
+
+  // Authorize and process both requests. Only the first request will succeed.
+  promise1.set(true);
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response1);
+
+  promise2.set(true);
+  AWAIT_EXPECT_RESPONSE_STATUS_EQ(BadRequest().status, response2);
+}
+
+
 // Tests whether we can retrieve the current quota status from
 // /master/quota endpoint via a GET request against /quota.
 TEST_F(MasterQuotaTest, Status)


[mesos] 02/02: Added MESOS-9786 to the 1.8.1 CHANGELOG.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch 1.8.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 6c6546da1619e2752033324a473c0dd4b4171cff
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Thu May 16 17:35:39 2019 +0200

    Added MESOS-9786 to the 1.8.1 CHANGELOG.
---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index 3898855..6649f87 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -8,6 +8,7 @@ Release Notes - Mesos - Version 1.8.1 (WIP)
   * [MESOS-9695] - Remove the duplicate pid check in Docker containerizer
   * [MESOS-9766] - /__processes__ endpoint can hang.
   * [MESOS-9782] - Random sorter fails to clear removed clients.
+  * [MESOS-9786] - Race between two REMOVE_QUOTA calls crashes the master.
   * [MESOS-9787] - Log slow SSL (TLS) peer reverse DNS lookup.
 
 ** Improvement