You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2016/04/06 20:16:54 UTC

mesos git commit: Fixed leaked `roleSorter` and `quotaRoleSorter` in allocator.

Repository: mesos
Updated Branches:
  refs/heads/master 4e86a8c1c -> 03bb330b2


Fixed leaked `roleSorter` and `quotaRoleSorter` in allocator.

Review: https://reviews.apache.org/r/45529/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/03bb330b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/03bb330b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/03bb330b

Branch: refs/heads/master
Commit: 03bb330b27c335f50311d7d7b6471e5baac0e339
Parents: 4e86a8c
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Wed Apr 6 13:58:33 2016 -0400
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Wed Apr 6 14:16:46 2016 -0400

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp | 13 +++++++------
 src/master/allocator/mesos/hierarchical.hpp |  7 ++++---
 2 files changed, 11 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/03bb330b/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index cd6b91e..a8d80b4 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -39,6 +39,7 @@ using mesos::master::InverseOfferStatus;
 
 using process::Failure;
 using process::Future;
+using process::Owned;
 using process::Timeout;
 
 namespace mesos {
@@ -138,8 +139,8 @@ void HierarchicalAllocatorProcess::initialize(
   // all roles.
   //
   // TODO(alexr): Consider introducing a sorter type for quota'ed roles.
-  roleSorter = roleSorterFactory();
-  quotaRoleSorter = roleSorterFactory();
+  roleSorter.reset(roleSorterFactory());
+  quotaRoleSorter.reset(roleSorterFactory());
 
   VLOG(1) << "Initialized hierarchical allocator process";
 
@@ -225,7 +226,7 @@ void HierarchicalAllocatorProcess::addFramework(
   if (!activeRoles.contains(role)) {
     activeRoles[role] = 1;
     roleSorter->add(role, roleWeight(role));
-    frameworkSorters[role] = frameworkSorterFactory();
+    frameworkSorters[role].reset(frameworkSorterFactory());
     metrics.addRole(role);
   } else {
     activeRoles[role]++;
@@ -314,7 +315,6 @@ void HierarchicalAllocatorProcess::removeFramework(
     roleSorter->remove(role);
 
     CHECK(frameworkSorters.contains(role));
-    delete frameworkSorters[role];
     frameworkSorters.erase(role);
 
     metrics.removeRole(role);
@@ -609,7 +609,8 @@ void HierarchicalAllocatorProcess::updateAllocation(
   // remain unchanged.
 
   // Update the allocated resources.
-  Sorter* frameworkSorter = frameworkSorters[role];
+  CHECK(frameworkSorters.contains(role));
+  const Owned<Sorter>& frameworkSorter = frameworkSorters[role];
 
   Resources frameworkAllocation =
     frameworkSorter->allocation(frameworkId.value(), slaveId);
@@ -1526,7 +1527,7 @@ void HierarchicalAllocatorProcess::deallocate(
   // keep generating new inverse offers even though the framework had not
   // responded yet.
 
-  foreachvalue (Sorter* frameworkSorter, frameworkSorters) {
+  foreachvalue (const Owned<Sorter>& frameworkSorter, frameworkSorters) {
     foreach (const SlaveID& slaveId, slaveIds_) {
       CHECK(slaves.contains(slaveId));
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/03bb330b/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index e979fdf..be4ccff 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -23,6 +23,7 @@
 
 #include <process/future.hpp>
 #include <process/id.hpp>
+#include <process/owned.hpp>
 
 #include <stout/duration.hpp>
 #include <stout/hashmap.hpp>
@@ -420,7 +421,7 @@ protected:
 
   // A sorter for active roles. This sorter determines the order in which
   // roles are allocated resources during Level 1 of the second stage.
-  Sorter* roleSorter;
+  process::Owned<Sorter> roleSorter;
 
   // A dedicated sorter for roles for which quota is set. This sorter
   // determines the order in which quota'ed roles are allocated resources
@@ -438,13 +439,13 @@ protected:
   // the quota roles as it pertains to their level of quota satisfaction.
   // Since revocable resources do not increase a role's level of satisfaction
   // toward its quota, we choose to exclude them from the quota role sorter.
-  Sorter* quotaRoleSorter;
+  process::Owned<Sorter> quotaRoleSorter;
 
   // A collection of sorters, one per active role. Each sorter determines
   // the order in which frameworks that belong to the same role are allocated
   // resources inside the role's share. These sorters are used during Level 2
   // for both the first and the second stages.
-  hashmap<std::string, Sorter*> frameworkSorters;
+  hashmap<std::string, process::Owned<Sorter>> frameworkSorters;
 
   // Factory functions for sorters.
   //