[mesos] 01/04: 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 master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 82f2dfd27449202afaf3feb08bb6d4ddeba7f515
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 2a861e2..ab20a86 100644
--- a/src/master/allocator/mesos/sorter/drf/sorter.hpp
+++ b/src/master/allocator/mesos/sorter/drf/sorter.hpp
@@ -356,7 +356,7 @@ struct DRFSorter::Node
 
       totals -= quantitiesToRemove;
 
-      if (resources[slaveId].empty()) {
+      if (resources.at(slaveId).empty()) {
         resources.erase(slaveId);
       }
     }
@@ -382,6 +382,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 bc55809..06362ce 100644
--- a/src/master/allocator/mesos/sorter/random/sorter.hpp
+++ b/src/master/allocator/mesos/sorter/random/sorter.hpp
@@ -376,7 +376,7 @@ struct RandomSorter::Node
 
       totals -= quantitiesToRemove;
 
-      if (resources[slaveId].empty()) {
+      if (resources.at(slaveId).empty()) {
         resources.erase(slaveId);
       }
     }
@@ -402,6 +402,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 64627c4..d7fdee8 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -1222,6 +1222,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());
 }