Repository: mesos
Updated Branches:
  refs/heads/master d0b375a8a -> 902b8dee3


Fixed a bug in the allocator which could cause filters to be
unnecessarily installed.

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


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

Branch: refs/heads/master
Commit: 902b8dee3aa4ac0cc166af89e7fa1d2630753a6d
Parents: d0b375a
Author: Jie Yu <[email protected]>
Authored: Fri Aug 29 16:27:05 2014 -0700
Committer: Jie Yu <[email protected]>
Committed: Tue Sep 2 10:03:56 2014 -0700

----------------------------------------------------------------------
 src/master/hierarchical_allocator_process.hpp | 72 ++++++++++++----------
 1 file changed, 39 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/902b8dee/src/master/hierarchical_allocator_process.hpp
----------------------------------------------------------------------
diff --git a/src/master/hierarchical_allocator_process.hpp 
b/src/master/hierarchical_allocator_process.hpp
index 34f8cd6..94bd228 100644
--- a/src/master/hierarchical_allocator_process.hpp
+++ b/src/master/hierarchical_allocator_process.hpp
@@ -566,43 +566,49 @@ HierarchicalAllocatorProcess<RoleSorter, 
FrameworkSorter>::resourcesRecovered(
               << " from framework " << frameworkId;
   }
 
-  // Create a filter for this slave/framework pair if both exist.
-  if (frameworks.contains(frameworkId) && slaves.contains(slaveId)) {
-    // Create a refused resources filter.
-    Try<Duration> seconds_ = Duration::create(Filters().refuse_seconds());
-    CHECK_SOME(seconds_);
-    Duration seconds = seconds_.get();
-
-    // Update the value of 'seconds' if the input isSome() and is
-    // valid.
-    if (filters.isSome()) {
-      seconds_ = Duration::create(filters.get().refuse_seconds());
-      if (seconds_.isError()) {
-        LOG(WARNING) << "Using the default value of 'refuse_seconds' to create 
"
-                     << "the refused resources filter because the input value "
-                     << "is invalid: " << seconds_.error();
-      } else if (seconds_.get() < Duration::zero()) {
-        LOG(WARNING) << "Using the default value of 'refuse_seconds' to create 
"
-                     << "the refused resources filter because the input value "
-                     << "is negative";
-      } else {
-        seconds = seconds_.get();
-      }
-    }
+  // No need to install the filter if 'filters' is none.
+  if (filters.isNone()) {
+    return;
+  }
 
-    if (seconds != Duration::zero()) {
-      VLOG(1) << "Framework " << frameworkId
-              << " filtered slave " << slaveId
-              << " for " << seconds;
+  // No need to install the filter if slave/framework does not exist.
+  if (!frameworks.contains(frameworkId) || !slaves.contains(slaveId)) {
+    return;
+  }
 
-      // Create a new filter and delay its expiration.
-      Filter* filter =
-        new RefusedFilter(slaveId, resources, process::Timeout::in(seconds));
+  // Create a refused resources filter.
+  Try<Duration> seconds = Duration::create(filters.get().refuse_seconds());
 
-      frameworks[frameworkId].filters.insert(filter);
+  if (seconds.isError()) {
+    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create "
+                 << "the refused resources filter because the input value "
+                 << "is invalid: " << seconds.error();
 
-      delay(seconds, self(), &Self::expire, frameworkId, filter);
-    }
+    seconds = Duration::create(Filters().refuse_seconds());
+  } else if (seconds.get() < Duration::zero()) {
+    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create "
+                 << "the refused resources filter because the input value "
+                 << "is negative";
+
+    seconds = Duration::create(Filters().refuse_seconds());
+  }
+
+  CHECK_SOME(seconds);
+
+  if (seconds.get() != Duration::zero()) {
+    VLOG(1) << "Framework " << frameworkId
+            << " filtered slave " << slaveId
+            << " for " << seconds.get();
+
+    // Create a new filter and delay its expiration.
+    Filter* filter = new RefusedFilter(
+        slaveId,
+        resources,
+        process::Timeout::in(seconds.get()));
+
+    frameworks[frameworkId].filters.insert(filter);
+
+    delay(seconds.get(), self(), &Self::expire, frameworkId, filter);
   }
 }
 

Reply via email to