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 <[email protected]> Authored: Fri Apr 28 15:07:14 2017 -0700 Committer: Jiang Yan Xu <[email protected]> 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);
