[mesos] branch 1.9.x updated: Fixed a bug where sorter may leak clients.

[mz...@apache.org]

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

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


The following commit(s) were added to refs/heads/1.9.x by this push:
     new 27e3e8f  Fixed a bug where sorter may leak clients.
27e3e8f is described below

commit 27e3e8ff1cf0f0b9201a71a6853a1a1da44cb633
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Wed Sep 18 16:45:46 2019 -0700

    Fixed a bug where sorter may leak clients.
    
    It is possible to update allocations to empty in the sorter.
    See MESOS-9015. When this happens, sorter will leak client
    entries, since it does not erase the corresponding entries.
    
    This patch fixes this issue. Also added a regression test.
    
    Review: https://reviews.apache.org/r/71515
---
 src/master/allocator/mesos/sorter/drf/sorter.hpp    |  8 +++++++-
 src/master/allocator/mesos/sorter/random/sorter.hpp |  8 +++++++-
 src/tests/sorter_tests.cpp                          | 16 ++++++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/master/allocator/mesos/sorter/drf/sorter.hpp b/src/master/allocator/mesos/sorter/drf/sorter.hpp
index f157ec6..299985c 100644
--- a/src/master/allocator/mesos/sorter/drf/sorter.hpp
+++ b/src/master/allocator/mesos/sorter/drf/sorter.hpp
@@ -359,7 +359,7 @@ struct DRFSorter::Node
 
       totals -= quantitiesToRemove;
 
-      if (resources[slaveId].empty()) {
+      if (resources.at(slaveId).empty()) {
         resources.erase(slaveId);
       }
     }
@@ -385,6 +385,12 @@ struct DRFSorter::Node
       resources[slaveId] -= oldAllocation;
       resources[slaveId] += newAllocation;
 
+      // It is possible that allocations can be updated to empty.
+      // See MESOS-9015 and MESOS-9975.
+      if (resources.at(slaveId).empty()) {
+        resources.erase(slaveId);
+      }
+
       totals -= oldAllocationQuantities;
       totals += newAllocationQuantities;
     }
diff --git a/src/master/allocator/mesos/sorter/random/sorter.hpp b/src/master/allocator/mesos/sorter/random/sorter.hpp
index 8663ccd..b77c5cc 100644
--- a/src/master/allocator/mesos/sorter/random/sorter.hpp
+++ b/src/master/allocator/mesos/sorter/random/sorter.hpp
@@ -379,7 +379,7 @@ struct RandomSorter::Node
 
       totals -= quantitiesToRemove;
 
-      if (resources[slaveId].empty()) {
+      if (resources.at(slaveId).empty()) {
         resources.erase(slaveId);
       }
     }
@@ -405,6 +405,12 @@ struct RandomSorter::Node
       resources[slaveId] -= oldAllocation;
       resources[slaveId] += newAllocation;
 
+      // It is possible that allocations can be updated to empty.
+      // See MESOS-9015 and MESOS-9975.
+      if (resources.at(slaveId).empty()) {
+        resources.erase(slaveId);
+      }
+
       totals -= oldAllocationQuantities;
       totals += newAllocationQuantities;
     }
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index 97ab910..78d6dde 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -1230,6 +1230,22 @@ TYPED_TEST(CommonSorterTest, UpdateAllocation)
   EXPECT_EQ(
       CHECK_NOTERROR(ResourceQuantities::fromString("cpus:10;mem:10;disk:10")),
       sorter.allocationScalarQuantities());
+
+  // Test update allocation to empty.
+  Resources resourcesC =
+    CHECK_NOTERROR(Resources::parse("cpus:10;mem:10;disk:10"));
+
+  SlaveID slaveId2;
+  slaveId.set_value("agentId2");
+
+  sorter.add("c");
+  sorter.activate("c");
+
+  sorter.add(slaveId, resourcesC);
+
+  sorter.allocated("c", slaveId2, resourcesC);
+  sorter.update("c", slaveId2, resourcesC, Resources());
+  EXPECT_TRUE(sorter.allocation("c").empty());
 }