You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2017/04/28 22:10:05 UTC
mesos git commit: Avoid a corruption while rescinding offers.
Repository: mesos
Updated Branches:
refs/heads/master 9fb6a11a5 -> eecd2870c
Avoid a corruption while rescinding offers.
When a `DESTROY` is processed, we rescind all pending offers to which
the destroyed persistent volumes have been offered. As soon as we
rescind an offer, the `Offer` object is freed; and hence, we should
move on to the next offer (even though there could be multiple volumes
being destroyed in the same `ACCEPT` call).
Review: https://reviews.apache.org/r/58485/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/eecd2870
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/eecd2870
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/eecd2870
Branch: refs/heads/master
Commit: eecd2870cdfed1a46bd2e46943fb59e9818a67de
Parents: 9fb6a11
Author: Anindya Sinha <an...@apple.com>
Authored: Fri Apr 28 15:07:14 2017 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Apr 28 15:07:14 2017 -0700
----------------------------------------------------------------------
src/master/master.cpp | 8 ++++++-
src/tests/persistent_volume_tests.cpp | 38 +++++++++++++++++++++---------
2 files changed, 34 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/eecd2870/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 97df727..69f52a4 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -4340,15 +4340,21 @@ void Master::_accept(
// rescind those offers.
foreach (Offer* offer, utils::copy(slave->offers)) {
const Resources& offered = offer->resources();
+
foreach (const Resource& volume, operation.destroy().volumes()) {
if (offered.contains(volume)) {
allocator->recoverResources(
offer->framework_id(),
offer->slave_id(),
- offer->resources(),
+ offered,
None());
removeOffer(offer, true);
+
+ // This offer may contain other volumes that are being destroyed.
+ // However, we have already rescinded it, so we should move on
+ // to the next offer.
+ break;
}
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/eecd2870/src/tests/persistent_volume_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/persistent_volume_tests.cpp b/src/tests/persistent_volume_tests.cpp
index 3eb7afe..d42fd18 100644
--- a/src/tests/persistent_volume_tests.cpp
+++ b/src/tests/persistent_volume_tests.cpp
@@ -1062,16 +1062,28 @@ TEST_P(PersistentVolumeTest, SharedPersistentVolumeRescindOnDestroy)
Offer offer1 = offers1.get()[0];
- // 2. framework1 CREATEs a shared volume, and LAUNCHes a task with a subset
+ // 2. framework1 CREATEs 2 shared volumes, and LAUNCHes a task with a subset
// of resources from the offer.
- Resource volume = createPersistentVolume(
- getDiskResource(Megabytes(2048)),
+ Resource volume1 = createPersistentVolume(
+ getDiskResource(Megabytes(2048), 1),
"id1",
"path1",
None(),
frameworkInfo1.principal(),
true); // Shared volume.
+ Resource volume2 = createPersistentVolume(
+ getDiskResource(Megabytes(2048), 2),
+ "id2",
+ "path2",
+ None(),
+ frameworkInfo1.principal(),
+ true); // Shared volume.
+
+ Resources allVolumes;
+ allVolumes += volume1;
+ allVolumes += volume2;
+
// Create a task which uses a portion of the offered resources, so that
// the remaining resources can be offered to framework2. It's not important
// whether the volume is used (the task is killed soon and its purpose is
@@ -1081,7 +1093,7 @@ TEST_P(PersistentVolumeTest, SharedPersistentVolumeRescindOnDestroy)
Resources::parse("cpus:1;mem:128").get(),
"sleep 1000");
- // Expect an offer containing the persistent volume.
+ // Expect an offer containing the persistent volumes.
EXPECT_CALL(sched1, resourceOffers(&driver1, _))
.WillOnce(FutureArg<1>(&offers1))
.WillRepeatedly(Return()); // Ignore subsequent offers.
@@ -1093,7 +1105,7 @@ TEST_P(PersistentVolumeTest, SharedPersistentVolumeRescindOnDestroy)
driver1.acceptOffers(
{offer1.id()},
- {CREATE(volume),
+ {CREATE(allVolumes),
LAUNCH({task})},
filters);
@@ -1125,10 +1137,12 @@ TEST_P(PersistentVolumeTest, SharedPersistentVolumeRescindOnDestroy)
Offer offer2 = offers2.get()[0];
EXPECT_TRUE(Resources(offer2.resources()).contains(
- allocatedResources(volume, frameworkInfo2.role())));
+ allocatedResources(volume1, frameworkInfo2.role())));
+ EXPECT_TRUE(Resources(offer2.resources()).contains(
+ allocatedResources(volume2, frameworkInfo2.role())));
// 4. framework1 kills the task which results in an offer to framework1
- // with the shared volume. At this point, both frameworks will have
+ // with the shared volumes. At this point, both frameworks will have
// the shared resource in their pending offers.
EXPECT_CALL(sched1, statusUpdate(_, _))
.WillOnce(DoDefault());
@@ -1148,17 +1162,19 @@ TEST_P(PersistentVolumeTest, SharedPersistentVolumeRescindOnDestroy)
offer1 = offers1.get()[0];
EXPECT_TRUE(Resources(offer1.resources()).contains(
- allocatedResources(volume, frameworkInfo1.role())));
+ allocatedResources(volume1, frameworkInfo1.role())));
+ EXPECT_TRUE(Resources(offer1.resources()).contains(
+ allocatedResources(volume2, frameworkInfo1.role())));
- // 5. DESTROY the shared volume via framework2 which would result in
- // framework1 being rescinded the offer.
+ // 5. DESTROY both the shared volumes via framework2 which would result
+ // in framework1 being rescinded the offer.
Future<Nothing> rescinded;
EXPECT_CALL(sched1, offerRescinded(&driver1, _))
.WillOnce(FutureSatisfy(&rescinded));
driver2.acceptOffers(
{offer2.id()},
- {DESTROY(volume)},
+ {DESTROY(allVolumes)},
filters);
AWAIT_READY(rescinded);