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 <[email protected]> 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()); }
