This is an automated email from the ASF dual-hosted git repository. mzhu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit c20469edb25348e70bf1647a57694e6dff8041c4 Author: Andrei Sekretenko <[email protected]> AuthorDate: Fri Sep 6 14:15:48 2019 -0700 Moved setting expectation for recoverResources() to a proper place. Currently, when accepting offers for a slave already removed from the master, Master::_accept() dispatches `recoverResources()` unconditionally (with empty resources if, due to framework/slave removal the offers were already rescinded and their resources recovered). The depending patch removes this redundant recovery of empty resources, thus exposing a race in some of `MasterAuthorizationTest.*` tests between `FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources)` and the dispatch which performs the actual resource recovery. This patch is fixing these tests to wait for the first dispatch of `recoverResources()`. Review: https://reviews.apache.org/r/71435/ --- src/tests/master_authorization_tests.cpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/tests/master_authorization_tests.cpp b/src/tests/master_authorization_tests.cpp index 2932564..06471aa 100644 --- a/src/tests/master_authorization_tests.cpp +++ b/src/tests/master_authorization_tests.cpp @@ -433,6 +433,9 @@ TEST_F(MasterAuthorizationTest, KillTask) EXPECT_CALL(sched, statusUpdate(&driver, _)) .WillOnce(FutureArg<1>(&status)); + Future<Nothing> recoverResources = + FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); + // Now kill the task. driver.killTask(task.task_id()); @@ -441,9 +444,6 @@ TEST_F(MasterAuthorizationTest, KillTask) EXPECT_EQ(TASK_KILLED, status->state()); EXPECT_EQ(TaskStatus::REASON_TASK_KILLED_DURING_LAUNCH, status->reason()); - Future<Nothing> recoverResources = - FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); - // Now complete authorization. promise.set(true); @@ -556,6 +556,9 @@ TEST_F(MasterAuthorizationTest, KillPendingTaskInTaskGroup) AWAIT_READY(authorize1); AWAIT_READY(authorize2); + Future<Nothing> recoverResources = + FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); + // Now kill task1. driver.killTask(task1.task_id()); @@ -566,9 +569,6 @@ TEST_F(MasterAuthorizationTest, KillPendingTaskInTaskGroup) EXPECT_EQ(TaskStatus::REASON_TASK_KILLED_DURING_LAUNCH, task1Status->reason()); - Future<Nothing> recoverResources = - FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); - // Now complete authorizations for task1 and task2. promise1.set(true); promise2.set(true); @@ -651,6 +651,9 @@ TEST_F(MasterAuthorizationTest, SlaveRemovedLost) EXPECT_CALL(sched, slaveLost(&driver, _)) .WillOnce(FutureSatisfy(&slaveLost)); + Future<Nothing> recoverResources = + FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); + // Stop the slave with explicit shutdown as otherwise with // checkpointing the master will wait for the slave to reconnect. slave.get()->shutdown(); @@ -662,9 +665,6 @@ TEST_F(MasterAuthorizationTest, SlaveRemovedLost) EXPECT_CALL(sched, statusUpdate(&driver, _)) .WillOnce(FutureArg<1>(&status)); - Future<Nothing> recoverResources = - FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); - // Now complete authorization. promise.set(true); @@ -756,6 +756,9 @@ TEST_F(MasterAuthorizationTest, SlaveRemovedDropped) EXPECT_CALL(sched, slaveLost(&driver, _)) .WillOnce(FutureSatisfy(&slaveLost)); + Future<Nothing> recoverResources = + FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); + // Stop the slave with explicit shutdown as otherwise with // checkpointing the master will wait for the slave to reconnect. slave.get()->shutdown(); @@ -767,9 +770,6 @@ TEST_F(MasterAuthorizationTest, SlaveRemovedDropped) EXPECT_CALL(sched, statusUpdate(&driver, _)) .WillOnce(FutureArg<1>(&status)); - Future<Nothing> recoverResources = - FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); - // Now complete authorization. promise.set(true); @@ -856,15 +856,15 @@ TEST_F(MasterAuthorizationTest, FrameworkRemoved) Future<Nothing> removeFramework = FUTURE_DISPATCH(_, &MesosAllocatorProcess::removeFramework); + Future<Nothing> recoverResources = + FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); + // Now stop the framework. driver.stop(); driver.join(); AWAIT_READY(removeFramework); - Future<Nothing> recoverResources = - FUTURE_DISPATCH(_, &MesosAllocatorProcess::recoverResources); - // Now complete authorization. promise.set(true);
