this is how i applied it. not sure what you meant by differently? i just edited the commit message, afaict.
this is the terminal history... ➜ ~/workspace/apache/mesos git:(master) ./support/apply-review.sh 13764 --2013-08-27 13:36:38-- https://reviews.apache.org/r/13764/diff/raw/ Resolving reviews.apache.org... 140.211.11.74 Connecting to reviews.apache.org|140.211.11.74|:443... connected. HTTP request sent, awaiting response... 200 OK Length: 11271 (11K) [text/x-patch] Saving to: `13764.patch' 100%[==================================================================================================================>] 11,271 --.-K/s in 0.005s 2013-08-27 13:36:39 (2.33 MB/s) - `13764.patch' saved [11271/11271] [master ce17048] Add resource checks to slave recovery tests to ensure resources are re-offered after restarts. 1 file changed, 94 insertions(+), 41 deletions(-) [master 2b5463f] Add resource checks to slave recovery tests to ensure resources are re-offered after restarts. 1 file changed, 94 insertions(+), 41 deletions(-) ➜ ~/workspace/apache/mesos git:(master) git commit --amend [master 400a88f] Added resource checks to slave recovery tests to ensure resources are re-offered after restarts. 1 file changed, 94 insertions(+), 41 deletions(-) ➜ ~/workspace/apache/mesos git:(master) git lol ➜ ~/workspace/apache/mesos git:(master) git push apache head Username for 'https://git-wip-us.apache.org': vinodkone Password for 'https://[email protected]': Counting objects: 9, done. Delta compression using up to 4 threads. Compressing objects: 100% (5/5), done. Writing objects: 100% (5/5), 925 bytes, done. Total 5 (delta 4), reused 0 (delta 0) To https://git-wip-us.apache.org/repos/asf/mesos.git eb1cd4a..400a88f head -> master ➜ ~/workspace/apache/mesos git:(master) git push apache head @vinodkone On Tue, Aug 27, 2013 at 4:34 PM, Ian Downes <[email protected]> wrote: > The commit is different from the diff I tested and had for review. I'm not > sure why it was applied differently - Vinod? > > Ian > > > On Aug 27, 2013, at 12:44 PM, Benjamin Mahler <[email protected]> > wrote: > > +vinod, ian > > This appears to have broken the build. > > > On Tue, Aug 27, 2013 at 10:37 AM, <[email protected]> wrote: > >> Updated Branches: >> refs/heads/master eb1cd4a7c -> 400a88f98 >> >> >> Added resource checks to slave recovery tests to ensure resources >> are re-offered after restarts. >> >> From: Ian Downes <[email protected]> >> Review: https://reviews.apache.org/r/13764 >> >> >> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo >> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/400a88f9 >> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/400a88f9 >> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/400a88f9 >> >> Branch: refs/heads/master >> Commit: 400a88f9817bb102522b08b83dca400380ad8a9b >> Parents: eb1cd4a >> Author: Vinod Kone <[email protected]> >> Authored: Tue Aug 27 13:36:42 2013 -0400 >> Committer: Vinod Kone <[email protected]> >> Committed: Tue Aug 27 13:37:05 2013 -0400 >> >> ---------------------------------------------------------------------- >> src/tests/slave_recovery_tests.cpp | 135 ++++++++++++++++++++++---------- >> 1 file changed, 94 insertions(+), 41 deletions(-) >> ---------------------------------------------------------------------- >> >> >> >> http://git-wip-us.apache.org/repos/asf/mesos/blob/400a88f9/src/tests/slave_recovery_tests.cpp >> ---------------------------------------------------------------------- >> diff --git a/src/tests/slave_recovery_tests.cpp >> b/src/tests/slave_recovery_tests.cpp >> index 57636c1..78f42ff 100644 >> --- a/src/tests/slave_recovery_tests.cpp >> +++ b/src/tests/slave_recovery_tests.cpp >> @@ -524,17 +524,16 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverUnregisteredExecutor) >> >> EXPECT_CALL(sched, registered(_, _, _)); >> >> - Future<vector<Offer> > offers; >> + Future<vector<Offer> > offers1; >> EXPECT_CALL(sched, resourceOffers(_, _)) >> - .WillOnce(FutureArg<1>(&offers)) >> - .WillRepeatedly(Return()); // Ignore subsequent offers. >> + .WillOnce(FutureArg<1>(&offers1)); >> >> driver.start(); >> >> - AWAIT_READY(offers); >> - EXPECT_NE(0u, offers.get().size()); >> + AWAIT_READY(offers1); >> + EXPECT_NE(0u, offers1.get().size()); >> >> - TaskInfo task = createTask(offers.get()[0], "sleep 1000"); >> + TaskInfo task = createTask(offers1.get()[0], "sleep 1000"); >> vector<TaskInfo> tasks; >> tasks.push_back(task); // Long-running task. >> >> @@ -542,7 +541,7 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverUnregisteredExecutor) >> Future<Message> registerExecutor = >> DROP_MESSAGE(Eq(RegisterExecutorMessage().GetTypeName()), _, _); >> >> - driver.launchTasks(offers.get()[0].id(), tasks); >> + driver.launchTasks(offers1.get()[0].id(), tasks); >> >> // Stop the slave before the executor is registered. >> AWAIT_READY(registerExecutor); >> @@ -560,6 +559,11 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverUnregisteredExecutor) >> // Restart the slave (use same flags) with a new isolator. >> TypeParam isolator2; >> >> + Future<vector<Offer> > offers2; >> + EXPECT_CALL(sched, resourceOffers(_, _)) >> + .WillOnce(FutureArg<1>(&offers2)) >> + .WillRepeatedly(Return()); // Ignore subsequent offers. >> + >> slave = this->StartSlave(&isolator2, flags); >> ASSERT_SOME(slave); >> >> @@ -583,6 +587,11 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverUnregisteredExecutor) >> >> Clock::resume(); >> >> + // Master should subsequently reoffer the same resources. >> + AWAIT_READY(offers2); >> + ASSERT_EQ(Resources(offers1.get()[0].resources()), >> + Resources(offers2.get()[0].resources())); >> + >> driver.stop(); >> driver.join(); >> >> @@ -617,17 +626,16 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverTerminatedExecutor) >> >> EXPECT_CALL(sched, registered(_, _, _)); >> >> - Future<vector<Offer> > offers; >> + Future<vector<Offer> > offers1; >> EXPECT_CALL(sched, resourceOffers(_, _)) >> - .WillOnce(FutureArg<1>(&offers)) >> - .WillRepeatedly(Return()); // Ignore subsequent offers. >> + .WillOnce(FutureArg<1>(&offers1)); >> >> driver.start(); >> >> - AWAIT_READY(offers); >> - EXPECT_NE(0u, offers.get().size()); >> + AWAIT_READY(offers1); >> + EXPECT_NE(0u, offers1.get().size()); >> >> - TaskInfo task = createTask(offers.get()[0], "sleep 1000"); >> + TaskInfo task = createTask(offers1.get()[0], "sleep 1000"); >> vector<TaskInfo> tasks; >> tasks.push_back(task); // Long-running task. >> >> @@ -639,7 +647,7 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverTerminatedExecutor) >> Future<Nothing> ack = >> FUTURE_DISPATCH(_, &Slave::_statusUpdateAcknowledgement); >> >> - driver.launchTasks(offers.get()[0].id(), tasks); >> + driver.launchTasks(offers1.get()[0].id(), tasks); >> >> // Capture the executor pid. >> AWAIT_READY(registerExecutor); >> @@ -662,6 +670,11 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverTerminatedExecutor) >> // Restart the slave (use same flags) with a new isolator. >> TypeParam isolator2; >> >> + Future<vector<Offer> > offers2; >> + EXPECT_CALL(sched, resourceOffers(_, _)) >> + .WillOnce(FutureArg<1>(&offers2)) >> + .WillRepeatedly(Return()); // Ignore subsequent offers. >> + >> slave = this->StartSlave(&isolator2, flags); >> ASSERT_SOME(slave); >> >> @@ -683,6 +696,13 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverTerminatedExecutor) >> AWAIT_READY(status); >> ASSERT_EQ(TASK_FAILED, status.get().state()); >> >> + Clock::resume(); >> + >> + // Master should subsequently reoffer the same resources. >> + AWAIT_READY(offers2); >> + ASSERT_EQ(Resources(offers1.get()[0].resources()), >> + Resources(offers2.get()[0].resources())); >> + >> driver.stop(); >> driver.join(); >> >> @@ -810,17 +830,17 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverCompletedExecutor) >> >> EXPECT_CALL(sched, registered(_, _, _)); >> >> - Future<vector<Offer> > offers; >> + Future<vector<Offer> > offers1; >> EXPECT_CALL(sched, resourceOffers(_, _)) >> - .WillOnce(FutureArg<1>(&offers)) >> - .WillRepeatedly(Return()); // Ignore subsequent offers. >> + .WillOnce(FutureArg<1>(&offers1)) >> + .WillRepeatedly(Return()); // Ignore subsequent offers. >> >> driver.start(); >> >> - AWAIT_READY(offers); >> - EXPECT_NE(0u, offers.get().size()); >> + AWAIT_READY(offers1); >> + EXPECT_NE(0u, offers1.get().size()); >> >> - TaskInfo task = createTask(offers.get()[0], "exit 0"); >> + TaskInfo task = createTask(offers1.get()[0], "exit 0"); >> vector<TaskInfo> tasks; >> tasks.push_back(task); // Short-lived task. >> >> @@ -833,7 +853,7 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverCompletedExecutor) >> Future<Nothing> schedule = FUTURE_DISPATCH( >> _, &GarbageCollectorProcess::schedule); >> >> - driver.launchTasks(offers.get()[0].id(), tasks); >> + driver.launchTasks(offers1.get()[0].id(), tasks); >> >> // We use 'gc.schedule' as a proxy for the cleanup of the executor. >> AWAIT_READY(schedule); >> @@ -846,12 +866,22 @@ TYPED_TEST(SlaveRecoveryTest, >> RecoverCompletedExecutor) >> // Restart the slave (use same flags) with a new isolator. >> TypeParam isolator2; >> >> + Future<vector<Offer> > offers2; >> + EXPECT_CALL(sched, resourceOffers(_, _)) >> + .WillOnce(FutureArg<1>(&offers2)) >> + .WillRepeatedly(Return()); // Ignore subsequent offers. >> + >> slave = this->StartSlave(&isolator2, flags); >> ASSERT_SOME(slave); >> >> // We use 'gc.schedule' as a proxy for the cleanup of the executor. >> AWAIT_READY(schedule2); >> >> + // Make sure all slave resources are reoffered. >> + AWAIT_READY(offers2); >> + ASSERT_EQ(Resources(offers1.get()[0].resources()), >> + Resources(offers2.get()[0].resources())); >> + >> driver.stop(); >> driver.join(); >> >> @@ -887,7 +917,8 @@ TYPED_TEST(SlaveRecoveryTest, CleanupExecutor) >> >> Future<vector<Offer> > offers; >> EXPECT_CALL(sched, resourceOffers(_, _)) >> - .WillOnce(FutureArg<1>(&offers)); >> + .WillOnce(FutureArg<1>(&offers)) >> + .WillRepeatedly(Return()); // Ignore subsequent offers. >> >> driver.start(); >> >> @@ -1199,7 +1230,8 @@ TYPED_TEST(SlaveRecoveryTest, KillTask) >> >> Future<vector<Offer> > offers1; >> EXPECT_CALL(sched, resourceOffers(_, _)) >> - .WillOnce(FutureArg<1>(&offers1)); >> + .WillOnce(FutureArg<1>(&offers1)) >> + .WillRepeatedly(Return()); // Ignore subsequent offers. >> >> driver.start(); >> >> @@ -1230,6 +1262,11 @@ TYPED_TEST(SlaveRecoveryTest, KillTask) >> // Restart the slave (use same flags) with a new isolator. >> TypeParam isolator2; >> >> + Future<vector<Offer> > offers2; >> + EXPECT_CALL(sched, resourceOffers(_, _)) >> + .WillOnce(FutureArg<1>(&offers2)) >> + .WillRepeatedly(Return()); // Ignore subsequent offers. >> + >> slave = this->StartSlave(&isolator2, flags); >> ASSERT_SOME(slave); >> >> @@ -1275,6 +1312,11 @@ TYPED_TEST(SlaveRecoveryTest, KillTask) >> >> Clock::resume(); >> >> + // Make sure all slave resources are reoffered. >> + AWAIT_READY(offers2); >> + ASSERT_EQ(Resources(offers1.get()[0].resources()), >> + Resources(offers2.get()[0].resources())); >> + >> driver.stop(); >> driver.join(); >> >> @@ -1310,23 +1352,22 @@ TYPED_TEST(SlaveRecoveryTest, GCExecutor) >> >> EXPECT_CALL(sched, registered(_, _, _)); >> >> - Future<vector<Offer> > offers; >> + Future<vector<Offer> > offers1; >> EXPECT_CALL(sched, resourceOffers(_, _)) >> - .WillOnce(FutureArg<1>(&offers)) >> - .WillRepeatedly(Return()); // Ignore subsequent offers. >> + .WillOnce(FutureArg<1>(&offers1)); >> >> driver.start(); >> >> - AWAIT_READY(offers); >> - EXPECT_NE(0u, offers.get().size()); >> + AWAIT_READY(offers1); >> + EXPECT_NE(0u, offers1.get().size()); >> >> - TaskInfo task = createTask(offers.get()[0], "sleep 1000"); >> + TaskInfo task = createTask(offers1.get()[0], "sleep 1000"); >> vector<TaskInfo> tasks; >> tasks.push_back(task); // Long-running task >> >> // Capture the slave and framework ids. >> - SlaveID slaveId = offers.get()[0].slave_id(); >> - FrameworkID frameworkId = offers.get()[0].framework_id(); >> + SlaveID slaveId = offers1.get()[0].slave_id(); >> + FrameworkID frameworkId = offers1.get()[0].framework_id(); >> >> Future<Message> registerExecutorMessage = >> FUTURE_MESSAGE(Eq(RegisterExecutorMessage().GetTypeName()), _, _); >> @@ -1336,7 +1377,7 @@ TYPED_TEST(SlaveRecoveryTest, GCExecutor) >> .WillOnce(FutureSatisfy(&status)) >> .WillRepeatedly(Return()); // Ignore subsequent updates. >> >> - driver.launchTasks(offers.get()[0].id(), tasks); >> + driver.launchTasks(offers1.get()[0].id(), tasks); >> >> // Capture the executor id and pid. >> AWAIT_READY(registerExecutorMessage); >> @@ -1503,6 +1544,9 @@ TYPED_TEST(SlaveRecoveryTest, ShutdownSlave) >> AWAIT_READY(offers2); >> >> EXPECT_NE(0u, offers2.get().size()); >> + // Make sure all slave resources are reoffered. >> + ASSERT_EQ(Resources(offers1.get()[0].resources()), >> + Resources(offers2.get()[0].resources())); >> >> // Ensure the slave id is different. >> ASSERT_NE( >> @@ -1643,30 +1687,29 @@ TYPED_TEST(SlaveRecoveryTest, ReconcileKillTask) >> >> EXPECT_CALL(sched, registered(_, _, _)); >> >> - Future<vector<Offer> > offers; >> + Future<vector<Offer> > offers1; >> EXPECT_CALL(sched, resourceOffers(_, _)) >> - .WillOnce(FutureArg<1>(&offers)) >> - .WillRepeatedly(Return()); // Ignore subsequent offers. >> + .WillOnce(FutureArg<1>(&offers1)); >> >> driver.start(); >> >> - AWAIT_READY(offers); >> - EXPECT_NE(0u, offers.get().size()); >> + AWAIT_READY(offers1); >> + EXPECT_NE(0u, offers1.get().size()); >> >> - TaskInfo task = createTask(offers.get()[0], "sleep 1000"); >> + TaskInfo task = createTask(offers1.get()[0], "sleep 1000"); >> vector<TaskInfo> tasks; >> tasks.push_back(task); // Long-running task >> >> // Capture the slave and framework ids. >> - SlaveID slaveId = offers.get()[0].slave_id(); >> - FrameworkID frameworkId = offers.get()[0].framework_id(); >> + SlaveID slaveId = offers1.get()[0].slave_id(); >> + FrameworkID frameworkId = offers1.get()[0].framework_id(); >> >> EXPECT_CALL(sched, statusUpdate(_, _)); // TASK_RUNNING >> >> Future<Nothing> _statusUpdateAcknowledgement = >> FUTURE_DISPATCH(_, &Slave::_statusUpdateAcknowledgement); >> >> - driver.launchTasks(offers.get()[0].id(), tasks); >> + driver.launchTasks(offers1.get()[0].id(), tasks); >> >> // Wait for TASK_RUNNING update to be acknowledged. >> AWAIT_READY(_statusUpdateAcknowledgement); >> @@ -1684,6 +1727,11 @@ TYPED_TEST(SlaveRecoveryTest, ReconcileKillTask) >> // Now restart the slave (use same flags) with a new isolator. >> TypeParam isolator2; >> >> + Future<vector<Offer> > offers2; >> + EXPECT_CALL(sched, resourceOffers(_, _)) >> + .WillOnce(FutureArg<1>(&offers2)) >> + .WillRepeatedly(Return()); // Ignore subsequent offers. >> + >> slave = this->StartSlave(&isolator2, flags); >> ASSERT_SOME(slave); >> >> @@ -1691,6 +1739,11 @@ TYPED_TEST(SlaveRecoveryTest, ReconcileKillTask) >> AWAIT_READY(status); >> ASSERT_EQ(TASK_KILLED, status.get().state()); >> >> + // Make sure all slave resources are reoffered. >> + AWAIT_READY(offers2); >> + ASSERT_EQ(Resources(offers1.get()[0].resources()), >> + Resources(offers2.get()[0].resources())); >> + >> driver.stop(); >> driver.join(); >> >> >> > >
