You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Mahler <be...@gmail.com> on 2013/08/27 21:44:23 UTC

Re: git commit: Added resource checks to slave recovery tests to ensure resources are re-offered after restarts.

+vinod, ian

This appears to have broken the build.


On Tue, Aug 27, 2013 at 10:37 AM, <vi...@apache.org> 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 <ia...@gmail.com>
> 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 <vi...@twitter.com>
> Authored: Tue Aug 27 13:36:42 2013 -0400
> Committer: Vinod Kone <vi...@twitter.com>
> 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();
>
>
>

Re: git commit: Added resource checks to slave recovery tests to ensure resources are re-offered after restarts.

Posted by Ian Downes <id...@twitter.com>.
Very sorry about that, on it now.

Ian
On Aug 27, 2013, at 12:44 PM, Benjamin Mahler <be...@gmail.com> wrote:

> +vinod, ian
> 
> This appears to have broken the build.
> 
> 
> On Tue, Aug 27, 2013 at 10:37 AM, <vi...@apache.org> 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 <ia...@gmail.com>
> 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 <vi...@twitter.com>
> Authored: Tue Aug 27 13:36:42 2013 -0400
> Committer: Vinod Kone <vi...@twitter.com>
> 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();
> 
> 
> 


Re: git commit: Added resource checks to slave recovery tests to ensure resources are re-offered after restarts.

Posted by Vinod Kone <vi...@twitter.com>.
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://vinodkone@git-wip-us.apache.org':
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 <id...@twitter.com> 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 <be...@gmail.com>
> wrote:
>
> +vinod, ian
>
> This appears to have broken the build.
>
>
> On Tue, Aug 27, 2013 at 10:37 AM, <vi...@apache.org> 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 <ia...@gmail.com>
>> 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 <vi...@twitter.com>
>> Authored: Tue Aug 27 13:36:42 2013 -0400
>> Committer: Vinod Kone <vi...@twitter.com>
>> 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();
>>
>>
>>
>
>

Re: git commit: Added resource checks to slave recovery tests to ensure resources are re-offered after restarts.

Posted by Ian Downes <id...@twitter.com>.
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 <be...@gmail.com> wrote:

> +vinod, ian
> 
> This appears to have broken the build.
> 
> 
> On Tue, Aug 27, 2013 at 10:37 AM, <vi...@apache.org> 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 <ia...@gmail.com>
> 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 <vi...@twitter.com>
> Authored: Tue Aug 27 13:36:42 2013 -0400
> Committer: Vinod Kone <vi...@twitter.com>
> 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();
> 
> 
>