[02/10] mesos git commit: Removed the timeout from the offer filter in the allocator.

[bm...@apache.org]

Removed the timeout from the offer filter in the allocator.

Without the timeout, we rely on filter expiration only. This guarantees
that filter removal is scheduled after `allocate()` if the allocator is
backlogged given default parameters are used. Additionally we ensure the
filter timeout is at least as big as the allocation interval.

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


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

Branch: refs/heads/master
Commit: 447d814ac80e67f30a0ffe2ee6047d85dc8fc383
Parents: f5f7670
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Thu Jan 21 23:17:22 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Thu Jan 21 23:38:57 2016 -0800

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp | 38 ++++++++++++------------
 src/tests/hierarchical_allocator_tests.cpp  |  9 ++++--
 2 files changed, 25 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/447d814a/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 53d8784..6215e2b 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -60,10 +60,7 @@ public:
 class RefusedOfferFilter : public OfferFilter
 {
 public:
-  RefusedOfferFilter(
-      const Resources& _resources,
-      const Timeout& _timeout)
-    : resources(_resources), timeout(_timeout) {}
+  RefusedOfferFilter(const Resources& _resources) : resources(_resources) {}
 
   virtual bool filter(const Resources& _resources)
   {
@@ -72,12 +69,10 @@ public:
     // more revocable resources only or non-revocable resources only,
     // but currently the filter only expires if there is more of both
     // revocable and non-revocable resources.
-    return resources.contains(_resources) && // Refused resources are superset.
-           timeout.remaining() > Seconds(0);
+    return resources.contains(_resources); // Refused resources are superset.
   }
 
   const Resources resources;
-  const Timeout timeout;
 };
 
 
@@ -925,11 +920,8 @@ void HierarchicalAllocatorProcess::recoverResources(
             << " filtered slave " << slaveId
             << " for " << seconds.get();
 
-    // Create a new filter and delay its expiration.
-    OfferFilter* offerFilter = new RefusedOfferFilter(
-        resources,
-        Timeout::in(seconds.get()));
-
+    // Create a new filter.
+    OfferFilter* offerFilter = new RefusedOfferFilter(resources);
     frameworks[frameworkId].offerFilters[slaveId].insert(offerFilter);
 
     // We need to disambiguate the function call to pick the correct
@@ -939,13 +931,21 @@ void HierarchicalAllocatorProcess::recoverResources(
               const SlaveID&,
               OfferFilter*) = &Self::expire;
 
-    delay(
-        seconds.get(),
-        self(),
-        expireOffer,
-        frameworkId,
-        slaveId,
-        offerFilter);
+    // Expire the filter after both an `allocationInterval` and the
+    // `timeout` have elapsed. This ensures that the filter does not
+    // expire before we perform the next allocation for this agent,
+    // see MESOS-4302 for more information.
+    //
+    // TODO(alexr): If we allocated upon resource recovery
+    // (MESOS-3078), we would not need to increase the timeout here.
+    Duration timeout = std::max(allocationInterval, seconds.get());
+
+    delay(timeout,
+          self(),
+          expireOffer,
+          frameworkId,
+          slaveId,
+          offerFilter);
   }
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/447d814a/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 9537121..516c5cf 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1281,12 +1281,15 @@ TEST_F(HierarchicalAllocatorTest, QuotaProvidesQuarantee)
   // general case, because an allocator may be complex enough to postpone
   // decisions beyond its allocation cycle.
 
-  // Now advance the clock to make sure the filter is expired. The next
-  // and only allocation should be the declined resources offered to the
-  // quota'ed role.
+  // Now advance the clock to make sure the filter is expired and removed.
   Clock::advance(Duration::create(filter5s.refuse_seconds()).get());
   Clock::settle();
 
+  // Trigger the next periodic allocation. It should offer the previously
+  // declined resources to the quota'ed role.
+  Clock::advance(flags.allocation_interval);
+  Clock::settle();
+
   allocation = allocations.get();
   AWAIT_READY(allocation);
   EXPECT_EQ(framework1.id(), allocation.get().frameworkId);