You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2016/01/22 09:28:19 UTC
[01/10] mesos git commit: Fixed race between {stop(),
abort()} and join() in the scheduler driver.
Repository: mesos
Updated Branches:
refs/heads/master c53de123a -> 6195e783d
Fixed race between {stop(), abort()} and join() in the scheduler driver.
Previously, it was possible for join() to return before a schedDriver
was actually fully stopped or aborted (breaking the semantics of the
join() call). The race came from a short circuit in join(), which
simply checked for status != DRIVER_RUNNING before returning. It appears
this short circuit was introduced to handle cases where initialize() or
start() ended up aborting before ever starting the driver to begin with.
However, it unintentionally covers cases where stop() or abort() were
called *after* the driver started running as well.
The problem is that stop() and abort() will change the status
to DRIVER_STOPPED or DRIVER_ABORTED before actually processing
dispatched stop or abort events (which happen asynchronously in a
libprocess thread). Under normal operation, join() would wait for these
events to trigger a latch that allowed the join() call to return.
However, with the short circuit, join() exits immediately even if the
libprocess thread hasn't yet processed the stop() or abort() events.
This commit fixes the semantics of the join() call to avoid this race.
We considered removing the latch completely and replacing it with
process.wait(), but, unlike the latch, this wouldn't ensure that stop()
or abort() was ever called in the first place.
Review: https://reviews.apache.org/r/42519/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f5f76701
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f5f76701
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f5f76701
Branch: refs/heads/master
Commit: f5f76701974482bbeacff4a347390287a788cf9d
Parents: c53de12
Author: Kevin Klues <kl...@gmail.com>
Authored: Thu Jan 21 23:12:00 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Thu Jan 21 23:24:01 2016 -0800
----------------------------------------------------------------------
src/sched/sched.cpp | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f5f76701/src/sched/sched.cpp
----------------------------------------------------------------------
diff --git a/src/sched/sched.cpp b/src/sched/sched.cpp
index 38940b7..67038ef 100644
--- a/src/sched/sched.cpp
+++ b/src/sched/sched.cpp
@@ -1905,19 +1905,20 @@ Status MesosSchedulerDriver::abort()
Status MesosSchedulerDriver::join()
{
- // Exit early if the driver is not running.
+ // We can use the process pointer to detect if the driver was ever
+ // started properly. If it wasn't, we return the current status
+ // (which should either be DRIVER_NOT_STARTED or DRIVER_ABORTED).
synchronized (mutex) {
- if (status != DRIVER_RUNNING) {
+ if (process == NULL) {
+ CHECK(status == DRIVER_NOT_STARTED || status == DRIVER_ABORTED);
+
return status;
}
}
- // If the driver was running, the latch will be triggered regardless
- // of the current `status`. Wait for this to happen to signify
- // termination.
+ // Otherwise, wait for stop() or abort() to trigger the latch.
CHECK_NOTNULL(latch)->await();
- // Now return the current `status` of the driver.
synchronized (mutex) {
CHECK(status == DRIVER_ABORTED || status == DRIVER_STOPPED);
[05/10] mesos git commit: Renamed resource offer timeout for clarity.
Posted by bm...@apache.org.
Renamed resource offer timeout for clarity.
Review: https://reviews.apache.org/r/42631
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/79e149e3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/79e149e3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/79e149e3
Branch: refs/heads/master
Commit: 79e149e3c4f7f0f61502a39fdb120b8fb9aa30be
Parents: afe3a1e
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Fri Jan 22 00:02:53 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Fri Jan 22 00:02:53 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/79e149e3/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 2f40801..e4aef5b 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -899,28 +899,28 @@ void HierarchicalAllocatorProcess::recoverResources(
}
// Create a refused resources filter.
- Try<Duration> seconds = Duration::create(filters.get().refuse_seconds());
+ Try<Duration> timeout = Duration::create(filters.get().refuse_seconds());
- if (seconds.isError()) {
+ if (timeout.isError()) {
LOG(WARNING) << "Using the default value of 'refuse_seconds' to create "
<< "the refused resources filter because the input value "
- << "is invalid: " << seconds.error();
+ << "is invalid: " << timeout.error();
- seconds = Duration::create(Filters().refuse_seconds());
- } else if (seconds.get() < Duration::zero()) {
+ timeout = Duration::create(Filters().refuse_seconds());
+ } else if (timeout.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());
+ timeout = Duration::create(Filters().refuse_seconds());
}
- CHECK_SOME(seconds);
+ CHECK_SOME(timeout);
- if (seconds.get() != Duration::zero()) {
+ if (timeout.get() != Duration::zero()) {
VLOG(1) << "Framework " << frameworkId
<< " filtered slave " << slaveId
- << " for " << seconds.get();
+ << " for " << timeout.get();
// Create a new filter.
OfferFilter* offerFilter = new RefusedOfferFilter(resources);
@@ -940,9 +940,9 @@ void HierarchicalAllocatorProcess::recoverResources(
//
// 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());
+ timeout = std::max(allocationInterval, timeout.get());
- delay(timeout,
+ delay(timeout.get(),
self(),
expireOffer,
frameworkId,
[07/10] mesos git commit: Cleaned up comment formatting in the recent
allocator changes.
Posted by bm...@apache.org.
Cleaned up comment formatting in the recent allocator changes.
Review: https://reviews.apache.org/r/42632/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/25585188
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/25585188
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/25585188
Branch: refs/heads/master
Commit: 255851881233bcce55f7681bd7966896e1fbb2ce
Parents: 6a9263b
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Fri Jan 22 00:14:51 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Fri Jan 22 00:14:51 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 15 +++++++++++++--
src/master/allocator/mesos/hierarchical.hpp | 5 +++++
2 files changed, 18 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/25585188/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 1a9372f..65c7e6b 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -78,6 +78,7 @@ private:
// Used to represent "filters" for inverse offers.
+//
// NOTE: Since this specific allocator implementation only sends inverse offers
// for maintenance primitives, and those are at the whole slave level, we only
// need to filter based on the time-out.
@@ -135,6 +136,7 @@ void HierarchicalAllocatorProcess::initialize(
// non-quota'ed roles, hence a dedicated sorter for quota'ed roles is
// necessary. We create an instance of the same sorter type we use for
// all roles.
+ //
// TODO(alexr): Consider introducing a sorter type for quota'ed roles.
roleSorter = roleSorterFactory();
quotaRoleSorter = roleSorterFactory();
@@ -1137,6 +1139,7 @@ void HierarchicalAllocatorProcess::allocate(
}
// Randomize the order in which slaves' resources are allocated.
+ //
// TODO(vinod): Implement a smarter sorting algorithm.
std::random_shuffle(slaveIds.begin(), slaveIds.end());
@@ -1155,14 +1158,17 @@ void HierarchicalAllocatorProcess::allocate(
// Summing up resources is fine because quota is only for scalar
// resources.
+ //
// NOTE: Reserved and revocable resources are excluded in
// `quotaRoleSorter`.
+ //
// TODO(alexr): Consider including dynamically reserved resources.
Resources roleConsumedResources =
Resources::sum(quotaRoleSorter->allocation(role));
// If quota for the role is satisfied, we do not need to do any further
// allocations for this role, at least at this stage.
+ //
// TODO(alexr): Skipping satisfied roles is pessimistic. Better
// alternatives are:
// * A custom sorter that is aware of quotas and sorts accordingly.
@@ -1184,6 +1190,7 @@ void HierarchicalAllocatorProcess::allocate(
// Quota is satisfied from the available unreserved non-revocable
// resources on the agent.
+ //
// TODO(alexr): Consider adding dynamically reserved resources.
Resources available = slaves[slaveId].total - slaves[slaveId].allocated;
Resources resources = available.unreserved().nonRevocable();
@@ -1213,6 +1220,7 @@ void HierarchicalAllocatorProcess::allocate(
// Resources allocated as part of the quota count towards the
// role's and the framework's fair share.
+ //
// NOTE: Reserved and revocable resources have already been excluded.
frameworkSorters[role]->add(slaveId, resources);
frameworkSorters[role]->allocated(frameworkId_, slaveId, resources);
@@ -1243,6 +1251,7 @@ void HierarchicalAllocatorProcess::allocate(
Resources unallocatedQuotaResources;
foreachpair (const string& name, const Quota& quota, quotas) {
// Compute the amount of quota that the role does not have allocated.
+ //
// NOTE: Reserved and revocable resources are excluded in `quotaRoleSorter`.
Resources allocated = Resources::sum(quotaRoleSorter->allocation(name));
const Resources required = quota.info.guarantee();
@@ -1250,6 +1259,7 @@ void HierarchicalAllocatorProcess::allocate(
}
// Determine how many resources we may allocate during the next stage.
+ //
// NOTE: Resources for quota allocations are already accounted in
// `remainingClusterResources`.
remainingClusterResources -= unallocatedQuotaResources;
@@ -1310,8 +1320,7 @@ void HierarchicalAllocatorProcess::allocate(
// stage to use more than `remainingClusterResources`, move along.
// We do not terminate early, as offers generated further in the
// loop may be small enough to fit within `remainingClusterResources`.
- if (!remainingClusterResources.contains(
- allocatedStage2 + resources)) {
+ if (!remainingClusterResources.contains(allocatedStage2 + resources)) {
continue;
}
@@ -1320,6 +1329,7 @@ void HierarchicalAllocatorProcess::allocate(
// NOTE: We perform "coarse-grained" allocation, meaning that we always
// allocate the entire remaining slave resources to a single framework.
+ //
// NOTE: We may have already allocated some resources on the current
// agent as part of quota.
offerable[frameworkId][slaveId] += resources;
@@ -1406,6 +1416,7 @@ void HierarchicalAllocatorProcess::deallocate(
if (!maintenance.offersOutstanding.contains(frameworkId)) {
// Ignore in case the framework filters inverse offers for this
// slave.
+ //
// NOTE: Since this specific allocator implementation only sends
// inverse offers for maintenance primitives, and those are at the
// whole slave level, we only need to filter based on the
http://git-wip-us.apache.org/repos/asf/mesos/blob/25585188/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 18f77bc..2d01034 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -328,6 +328,7 @@ protected:
// Represents a scheduled unavailability due to maintenance for a specific
// slave, and the responses from frameworks as to whether they will be able
// to gracefully handle this unavailability.
+ //
// NOTE: We currently implement maintenance in the allocator to be able to
// leverage state and features such as the FrameworkSorter and OfferFilter.
struct Maintenance
@@ -340,6 +341,7 @@ protected:
// A mapping of frameworks to the inverse offer status associated with
// this unavailability.
+ //
// NOTE: We currently lose this information during a master fail over
// since it is not persisted or replicated. This is ok as the new master's
// allocator will send out new inverse offers and re-collect the
@@ -403,6 +405,7 @@ protected:
//
// NOTE: The hierarchical allocator considers oversubscribed
// resources as regular resources when doing fairness calculations.
+ //
// TODO(vinod): Consider using a different fairness algorithm for
// oversubscribed resources.
@@ -422,6 +425,7 @@ protected:
// registered frameworks.
//
// NOTE: This sorter counts only unreserved non-revocable resources.
+ //
// TODO(alexr): Consider including dynamically reserved resources.
Sorter* quotaRoleSorter;
@@ -432,6 +436,7 @@ protected:
hashmap<std::string, Sorter*> frameworkSorters;
// Factory functions for sorters.
+ //
// NOTE: `quotaRoleSorter` currently reuses `roleSorterFactory`.
const std::function<Sorter*()> roleSorterFactory;
const std::function<Sorter*()> frameworkSorterFactory;
[06/10] mesos git commit: Minor cleanup to use an initializer list in
the allocator.
Posted by bm...@apache.org.
Minor cleanup to use an initializer list in the allocator.
Review: https://reviews.apache.org/r/41648/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6a9263b2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6a9263b2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6a9263b2
Branch: refs/heads/master
Commit: 6a9263b2ac95bca7f68a924355c5b432d4c33c90
Parents: 79e149e
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Fri Jan 22 00:12:58 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Fri Jan 22 00:12:58 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a9263b2/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index e4aef5b..1a9372f 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1102,9 +1102,7 @@ void HierarchicalAllocatorProcess::allocate(
Stopwatch stopwatch;
stopwatch.start();
- // TODO(bmahler): Add initializer list constructor for hashset.
- hashset<SlaveID> slaves;
- slaves.insert(slaveId);
+ hashset<SlaveID> slaves({slaveId});
allocate(slaves);
VLOG(1) << "Performed allocation for slave " << slaveId << " in "
[03/10] mesos git commit: Added tests for offer filters.
Posted by bm...@apache.org.
Added tests for offer filters.
Review: https://reviews.apache.org/r/42629/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ecfb8d53
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ecfb8d53
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ecfb8d53
Branch: refs/heads/master
Commit: ecfb8d53da58cc694ef885c929873042618dc16e
Parents: 447d814
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Thu Jan 21 23:29:06 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Thu Jan 21 23:59:36 2016 -0800
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 233 ++++++++++++++++++++++++
1 file changed, 233 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ecfb8d53/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 516c5cf..aa364ea 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -446,6 +446,239 @@ TEST_F(HierarchicalAllocatorTest, ReservedDRF)
}
+// This test ensures that an offer filter larger than the
+// allocation interval effectively filters out resources.
+TEST_F(HierarchicalAllocatorTest, OfferFilter)
+{
+ // Pausing the clock is not necessary, but ensures that the test
+ // doesn't rely on the batch allocation in the allocator, which
+ // would slow down the test.
+ Clock::pause();
+
+ // We put both frameworks into the same role, but we could also
+ // have had separate roles; this should not influence the test.
+ const string ROLE{"role"};
+
+ hashmap<FrameworkID, Resources> EMPTY;
+
+ initialize();
+
+ FrameworkInfo framework1 = createFrameworkInfo(ROLE);
+
+ SlaveInfo agent1 = createSlaveInfo("cpus:1;mem:512;disk:0");
+
+ allocator->addFramework(
+ framework1.id(),
+ framework1,
+ hashmap<SlaveID, Resources>());
+
+ allocator->addSlave(
+ agent1.id(),
+ agent1,
+ None(),
+ agent1.resources(),
+ EMPTY);
+
+ // `framework1` will be offered all of `agent1` resources
+ // because it is the only framework in the cluster.
+ Future<Allocation> allocation = allocations.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework1.id(), allocation.get().frameworkId);
+ EXPECT_EQ(agent1.resources(), Resources::sum(allocation.get().resources));
+
+ // Now `framework1` declines the offer and sets a filter
+ // with the duration greater than the allocation interval.
+ Duration filterTimeout = flags.allocation_interval * 2;
+ Filters offerFilter;
+ offerFilter.set_refuse_seconds(filterTimeout.secs());
+
+ allocator->recoverResources(
+ framework1.id(),
+ agent1.id(),
+ allocation.get().resources.get(agent1.id()).get(),
+ offerFilter);
+
+ // Ensure the offer filter timeout is set before advancing the clock.
+ Clock::settle();
+
+ // Trigger a batch allocation.
+ Clock::advance(flags.allocation_interval);
+ Clock::settle();
+
+ // There should be no allocation due to the offer filter.
+ allocation = allocations.get();
+ ASSERT_TRUE(allocation.isPending());
+
+ // Ensure the offer filter times out (2x the allocation interval)
+ // and the next batch allocation occurs.
+ Clock::advance(flags.allocation_interval);
+ Clock::settle();
+
+ // The next batch allocation should offer resources to `framework1`.
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework1.id(), allocation.get().frameworkId);
+ EXPECT_EQ(agent1.resources(), Resources::sum(allocation.get().resources));
+}
+
+
+// This test ensures that an offer filter is not removed earlier than
+// the next batch allocation. See MESOS-4302 for more information.
+//
+// NOTE: If we update the code to allocate upon resource recovery
+// (MESOS-3078), this test should still pass in that the small offer
+// filter timeout should lead to the next allocation for the agent
+// applying the filter.
+TEST_F(HierarchicalAllocatorTest, SmallOfferFilterTimeout)
+{
+ // Pausing the clock is not necessary, but ensures that the test
+ // doesn't rely on the batch allocation in the allocator, which
+ // would slow down the test.
+ Clock::pause();
+
+ // We put both frameworks into the same role, but we could also
+ // have had separate roles; this should not influence the test.
+ const string ROLE{"role"};
+
+ hashmap<FrameworkID, Resources> EMPTY;
+
+ // Explicitly set the allocation interval to make sure
+ // it is greater than the offer filter timeout.
+ master::Flags flags_;
+ flags_.allocation_interval = Minutes(1);
+
+ initialize(flags_);
+
+ // We start with the following cluster setup.
+ // Total cluster resources (1 agent): cpus=1, mem=512.
+ // ROLE1 share = 1 (cpus=1, mem=512)
+ // framework1 share = 1 (cpus=1, mem=512)
+ // framework2 share = 0
+
+ FrameworkInfo framework1 = createFrameworkInfo(ROLE);
+ FrameworkInfo framework2 = createFrameworkInfo(ROLE);
+
+ SlaveInfo agent1 = createSlaveInfo("cpus:1;mem:512;disk:0");
+
+ allocator->addFramework(
+ framework1.id(),
+ framework1,
+ hashmap<SlaveID, Resources>());
+
+ allocator->addFramework(
+ framework2.id(),
+ framework2,
+ hashmap<SlaveID, Resources>());
+
+ allocator->addSlave(
+ agent1.id(),
+ agent1,
+ None(),
+ agent1.resources(),
+ {std::make_pair(framework1.id(), agent1.resources())});
+
+ // Process all triggered allocation events.
+ // NOTE: No allocations happen because there are no resources to allocate.
+ Clock::settle();
+
+ // Add one more agent with some free resources.
+ SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:512;disk:0");
+ allocator->addSlave(
+ agent2.id(),
+ agent2,
+ None(),
+ agent2.resources(),
+ EMPTY);
+
+ // Process the allocation triggered by the agent addition.
+ Clock::settle();
+
+ // `framework2` will be offered all of `agent2` resources
+ // because its share (0) is smaller than `framework1`.
+ Future<Allocation> allocation = allocations.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework2.id(), allocation.get().frameworkId);
+ EXPECT_EQ(agent2.resources(), Resources::sum(allocation.get().resources));
+
+ // Total cluster resources (2 agents): cpus=2, mem=1024.
+ // ROLE1 share = 1 (cpus=2, mem=1024)
+ // framework1 share = 0.5 (cpus=1, mem=512)
+ // framework2 share = 0.5 (cpus=1, mem=512)
+
+ // Now `framework2` declines the offer and sets a filter
+ // for 1 second, which is less than the allocation interval.
+ Duration filterTimeout = Seconds(1);
+ ASSERT_GT(flags.allocation_interval, filterTimeout);
+
+ Filters offerFilter;
+ offerFilter.set_refuse_seconds(filterTimeout.secs());
+
+ allocator->recoverResources(
+ framework2.id(),
+ agent2.id(),
+ allocation.get().resources.get(agent2.id()).get(),
+ offerFilter);
+
+ // Total cluster resources (2 agents): cpus=2, mem=1024.
+ // ROLE1 share = 0.5 (cpus=1, mem=512)
+ // framework1 share = 1 (cpus=1, mem=512)
+ // framework2 share = 0
+
+ // The offer filter times out. Since the allocator ensures that
+ // offer filters are removed after at least one batch allocation
+ // has occurred, we expect that after the timeout elapses, the
+ // filter will remain active for the next allocation and the
+ // resources are allocated to `framework1`.
+ Clock::advance(filterTimeout);
+ Clock::settle();
+
+ // Trigger a batch allocation.
+ Clock::advance(flags.allocation_interval);
+ Clock::settle();
+
+ // Since the filter is applied, resources are offered to `framework1`
+ // even though its share is greater than `framework2`.
+ allocation = allocations.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework1.id(), allocation.get().frameworkId);
+ EXPECT_EQ(agent2.resources(), Resources::sum(allocation.get().resources));
+
+ // Total cluster resources (2 agents): cpus=2, mem=1024.
+ // ROLE1 share = 1 (cpus=2, mem=1024)
+ // framework1 share = 1 (cpus=2, mem=1024)
+ // framework2 share = 0
+
+ // The filter should be removed now than the batch
+ // allocation has occurred!
+
+ // Now `framework1` declines the offer.
+ allocator->recoverResources(
+ framework1.id(),
+ agent2.id(),
+ allocation.get().resources.get(agent2.id()).get(),
+ None());
+
+ // Total cluster resources (2 agents): cpus=2, mem=1024.
+ // ROLE1 share = 0.5 (cpus=1, mem=512)
+ // framework1 share = 1 (cpus=1, mem=512)
+ // framework2 share = 0
+
+ // Trigger a batch allocation.
+ Clock::advance(flags.allocation_interval);
+ Clock::settle();
+
+ // Since the filter is removed, resources are offered to `framework2`.
+ allocation = allocations.get();
+ AWAIT_READY(allocation);
+ EXPECT_EQ(framework2.id(), allocation.get().frameworkId);
+ EXPECT_EQ(agent2.resources(), Resources::sum(allocation.get().resources));
+
+ // Total cluster resources (2 agents): cpus=2, mem=1024.
+ // ROLE1 share = 1 (cpus=2, mem=1024)
+ // framework1 share = 0.5 (cpus=1, mem=512)
+ // framework2 share = 0.5 (cpus=1, mem=512)
+}
+
+
// This test ensures that agents which are scheduled for maintenance are
// properly sent inverse offers after they have accepted or reserved resources.
TEST_F(HierarchicalAllocatorTest, MaintenanceInverseOffers)
[02/10] mesos git commit: Removed the timeout from the offer filter
in the allocator.
Posted by 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);
[10/10] mesos git commit: Cleaned up comment formatting in allocator
tests.
Posted by bm...@apache.org.
Cleaned up comment formatting in allocator tests.
Review: https://reviews.apache.org/r/42637/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6195e783
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6195e783
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6195e783
Branch: refs/heads/master
Commit: 6195e783d21c885c8f74fd0e073d64c99da6e7b1
Parents: 8b5dbd8
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Fri Jan 22 00:20:02 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Fri Jan 22 00:20:02 2016 -0800
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6195e783/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index f7c82a4..b1cb955 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -577,6 +577,7 @@ TEST_F(HierarchicalAllocatorTest, SmallOfferFilterTimeout)
{std::make_pair(framework1.id(), agent1.resources())});
// Process all triggered allocation events.
+ //
// NOTE: No allocations happen because there are no resources to allocate.
Clock::settle();
@@ -795,7 +796,7 @@ TEST_F(HierarchicalAllocatorTest, CoarseGrained)
AWAIT_READY(allocation);
frameworkAllocations[allocation.get().frameworkId] = allocation.get();
- // Note that slave1 and slave2 have the same resources, we don't
+ // NOTE: `slave1` and `slave2` have the same resources, we don't
// care which framework received which slave.. only that they each
// received one.
ASSERT_TRUE(frameworkAllocations.contains(framework1.id()));
@@ -1444,6 +1445,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaProvidesQuarantee)
framework2.id(), framework2, hashmap<SlaveID, Resources>());
// Process all triggered allocation events.
+ //
// NOTE: No allocations happen because there are no resources to allocate.
Clock::settle();
@@ -1510,6 +1512,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaProvidesQuarantee)
// checking methods. Consider adding `process::Queue::empty()` or
// refactor the test harness so that we can reason about whether the
// Hierarchical allocator has assigned expected allocations or not.
+ //
// NOTE: It is hard to capture the absense of an allocation in a
// general case, because an allocator may be complex enough to postpone
// decisions beyond its allocation cycle.
@@ -1601,6 +1604,7 @@ TEST_F(HierarchicalAllocatorTest, RemoveQuota)
allocator->removeQuota(QUOTA_ROLE);
// Process all triggered allocation events.
+ //
// NOTE: No allocations happen because there are no resources to allocate.
Clock::settle();
@@ -1659,6 +1663,7 @@ TEST_F(HierarchicalAllocatorTest, MultipleFrameworksInRoleWithQuota)
framework2.id(), framework2, hashmap<SlaveID, Resources>());
// Process all triggered allocation events.
+ //
// NOTE: No allocations happen because there are no resources to allocate.
Clock::settle();
@@ -1780,6 +1785,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAllocationGranularity)
framework2.id(), framework2, hashmap<SlaveID, Resources>());
// Process all triggered allocation events.
+ //
// NOTE: No allocations happen because there are no resources to allocate.
Clock::settle();
@@ -1846,6 +1852,7 @@ TEST_F(HierarchicalAllocatorTest, DRFWithQuota)
hashmap<SlaveID, Resources>());
// Process all triggered allocation events.
+ //
// NOTE: No allocations happen because there are no resources to allocate.
Clock::settle();
@@ -1947,6 +1954,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAgainstStarvation)
{std::make_pair(framework1.id(), agent1.resources())});
// Process all triggered allocation events.
+ //
// NOTE: No allocations happen because all resources are already allocated.
Clock::settle();
@@ -2060,6 +2068,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAbsentFramework)
framework.id(), framework, hashmap<SlaveID, Resources>());
// Process all triggered allocation events.
+ //
// NOTE: No allocations happen because there are no resources to allocate.
Clock::settle();
@@ -2067,6 +2076,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAbsentFramework)
SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:512;disk:0");
// Each `addSlave()` triggers an event-based allocation.
+ //
// NOTE: The second event-based allocation for `agent2` takes into account
// that `agent1`'s resources are laid away for `QUOTA_ROLE`'s quota and
// hence freely allocates for the non-quota'ed `NO_QUOTA_ROLE` role.
@@ -2192,6 +2202,7 @@ TEST_F(HierarchicalAllocatorTest, MultiQuotaWithFrameworks)
framework2.id(), framework2, hashmap<SlaveID, Resources>());
// Process all triggered allocation events.
+ //
// NOTE: No allocations happen because there are no resources to allocate.
Clock::settle();
[09/10] mesos git commit: Removed misleading note from allocator
tests.
Posted by bm...@apache.org.
Removed misleading note from allocator tests.
Review: https://reviews.apache.org/r/42635/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/8b5dbd8b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/8b5dbd8b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/8b5dbd8b
Branch: refs/heads/master
Commit: 8b5dbd8b0f7c516c82d7b7fb11b54ee3295af290
Parents: cebbdab
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Fri Jan 22 00:17:26 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Fri Jan 22 00:17:26 2016 -0800
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 6 ------
1 file changed, 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/8b5dbd8b/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index d11e1c7..f7c82a4 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1569,8 +1569,6 @@ TEST_F(HierarchicalAllocatorTest, RemoveQuota)
// Notify allocator of agents, frameworks, quota and current allocations.
allocator->setQuota(QUOTA_ROLE, quota1);
- // NOTE: We do not report about allocated resources to `framework1`
- // here to avoid double accounting.
allocator->addFramework(
framework1.id(),
framework1,
@@ -1837,8 +1835,6 @@ TEST_F(HierarchicalAllocatorTest, DRFWithQuota)
// Notify allocator of agents, frameworks, quota and current allocations.
allocator->setQuota(QUOTA_ROLE, quota1);
- // NOTE: We do not report about allocated resources to `framework1`
- // here to avoid double accounting.
allocator->addFramework(
framework1.id(),
framework1,
@@ -1933,8 +1929,6 @@ TEST_F(HierarchicalAllocatorTest, QuotaAgainstStarvation)
SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:512;disk:0");
// Notify allocator of agents, frameworks, and current allocations.
- // NOTE: We do not report about allocated resources to `framework1`
- // here to avoid double accounting.
allocator->addFramework(
framework1.id(),
framework1,
[04/10] mesos git commit: Changed member visibility in offer filter
classes.
Posted by bm...@apache.org.
Changed member visibility in offer filter classes.
Review: https://reviews.apache.org/r/42630/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/afe3a1ed
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/afe3a1ed
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/afe3a1ed
Branch: refs/heads/master
Commit: afe3a1ed1c602b54220a39b699ef9550a3bd6ed4
Parents: ecfb8d5
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Thu Jan 21 23:59:49 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Thu Jan 21 23:59:49 2016 -0800
----------------------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp | 2 ++
1 file changed, 2 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/afe3a1ed/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 6215e2b..2f40801 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -72,6 +72,7 @@ public:
return resources.contains(_resources); // Refused resources are superset.
}
+private:
const Resources resources;
};
@@ -107,6 +108,7 @@ public:
return timeout.remaining() > Seconds(0);
}
+private:
const Timeout timeout;
};
[08/10] mesos git commit: Updated a comment in the allocator tests
for consistency.
Posted by bm...@apache.org.
Updated a comment in the allocator tests for consistency.
Review: https://reviews.apache.org/r/42634/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/cebbdab5
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cebbdab5
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cebbdab5
Branch: refs/heads/master
Commit: cebbdab581abaf2cae3c2f14b58df0654212fbd1
Parents: 2558518
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Fri Jan 22 00:16:56 2016 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Fri Jan 22 00:16:56 2016 -0800
----------------------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/cebbdab5/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index aa364ea..d11e1c7 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1413,7 +1413,7 @@ TEST_F(HierarchicalAllocatorTest, Whitelist)
// resources as part of quota and do not re-offer them afterwards.
// In the presence of quota'ed and non-quota'ed roles, if a framework in
-// the quota'ed role declines offers, some resources are kept aside for
+// the quota'ed role declines offers, some resources are laid away for
// the role, so that a greedy framework from a non-quota'ed role cannot
// eat up all free resources.
TEST_F(HierarchicalAllocatorTest, QuotaProvidesQuarantee)