From 446e90a6eddcd41f23657c956fbdd1259111483a Mon Sep 17 00:00:00 2001 From: Carlos Segarra Date: Wed, 2 Feb 2022 17:19:37 +0000 Subject: [PATCH] cleanup --- include/faabric/scheduler/MpiWorld.h | 2 -- include/faabric/scheduler/Scheduler.h | 4 --- src/scheduler/Executor.cpp | 7 +++--- src/scheduler/MpiWorld.cpp | 9 ++----- src/scheduler/Scheduler.cpp | 1 - .../scheduler/test_function_migration.cpp | 25 +------------------ 6 files changed, 7 insertions(+), 41 deletions(-) diff --git a/include/faabric/scheduler/MpiWorld.h b/include/faabric/scheduler/MpiWorld.h index 9ba43f7da..94136b0dd 100644 --- a/include/faabric/scheduler/MpiWorld.h +++ b/include/faabric/scheduler/MpiWorld.h @@ -280,8 +280,6 @@ class MpiWorld int recvRank, int batchSize = 0); - void finishMigration(int thisRank); - /* Helper methods */ void checkRanksRange(int sendRank, int recvRank); diff --git a/include/faabric/scheduler/Scheduler.h b/include/faabric/scheduler/Scheduler.h index 3ab10685d..97b69d538 100644 --- a/include/faabric/scheduler/Scheduler.h +++ b/include/faabric/scheduler/Scheduler.h @@ -90,10 +90,6 @@ class Executor faabric::Message& msg, bool createIfNotExists = false); - // TODO - maybe remove me - bool doMigration( - std::shared_ptr pendingMigrations); - virtual std::span getMemoryView(); protected: diff --git a/src/scheduler/Executor.cpp b/src/scheduler/Executor.cpp index c3a9ab1a4..d8bb9c191 100644 --- a/src/scheduler/Executor.cpp +++ b/src/scheduler/Executor.cpp @@ -545,10 +545,11 @@ void Executor::threadPoolThread(int threadPoolIdx) returnValue = -99; // MPI migration - if(msg.ismpi()) { - // TODO - when should we delete the pending migration? + if (msg.ismpi()) { + // TODO - delete the pending migration auto& mpiWorld = - faabric::scheduler::getMpiWorldRegistry().getWorld(msg.mpiworldid()); + faabric::scheduler::getMpiWorldRegistry().getWorld( + msg.mpiworldid()); mpiWorld.destroy(); } } catch (const std::exception& ex) { diff --git a/src/scheduler/MpiWorld.cpp b/src/scheduler/MpiWorld.cpp index a3627ec43..0cbfa2284 100644 --- a/src/scheduler/MpiWorld.cpp +++ b/src/scheduler/MpiWorld.cpp @@ -1758,8 +1758,6 @@ void MpiWorld::checkRanksRange(int sendRank, int recvRank) } } -// Once per host we modify the per-world accounting of rank-to-hosts mappings -// and the corresponding ports for cross-host messaging. void MpiWorld::prepareMigration( int thisRank, std::shared_ptr pendingMigrations) @@ -1802,16 +1800,13 @@ void MpiWorld::prepareMigration( // TODO - merge with initLocalQueues for (const int sendRank : ranksForHost[thisHost]) { for (const int recvRank : ranksForHost[thisHost]) { - if (localQueues[getIndexForRanks(sendRank, recvRank)] == nullptr) { + if (localQueues[getIndexForRanks(sendRank, recvRank)] == + nullptr) { localQueues[getIndexForRanks(sendRank, recvRank)] = std::make_shared(); } } } - - // Lastly, remove the migrations from the pending migrations map - // TODO - when should we remove the pending migration from the map? - // getScheduler().removePendingMigration(thisRankMsg->appid()); } } } diff --git a/src/scheduler/Scheduler.cpp b/src/scheduler/Scheduler.cpp index 7ecd76817..a9174b57d 100644 --- a/src/scheduler/Scheduler.cpp +++ b/src/scheduler/Scheduler.cpp @@ -1371,7 +1371,6 @@ Scheduler::doCheckForMigrationOpportunities( &(*(req->mutable_messages()->begin() + std::distance(originalDecision.hosts.begin(), right))); auto* migrationMsgPtr = migration->mutable_msg(); - // faabric::util::copyMessage(msgPtr, migrationMsgPtr); *migrationMsgPtr = *msgPtr; // Decrement by one the availability, and check for more // possible sources of migration diff --git a/tests/test/scheduler/test_function_migration.cpp b/tests/test/scheduler/test_function_migration.cpp index c3535a923..4ee305fde 100644 --- a/tests/test/scheduler/test_function_migration.cpp +++ b/tests/test/scheduler/test_function_migration.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include using namespace faabric::scheduler; @@ -86,8 +85,7 @@ class FunctionMigrationTestFixture : public SchedulerTestFixture for (auto pair : migrations) { auto* migration = expected.add_migrations(); auto* migrationMsg = migration->mutable_msg(); - faabric::util::copyMessage(&req->mutable_messages()->at(pair.first), - migrationMsg); + *migrationMsg = req->mutable_messages()->at(pair.first); migration->set_srchost(hosts.at(pair.first)); migration->set_dsthost(hosts.at(pair.second)); } @@ -449,26 +447,6 @@ TEST_CASE_METHOD(FunctionMigrationTestFixture, checkPendingMigrationsExpectation( expectedMigrations, actualMigrations, hosts, true); - // Check that certain MPI calls actually do the migration - SECTION("MPI barrier triggers a migration point") { world.barrier(0); } - - SECTION("MPI all reduce triggers a migration point") - { - std::vector messageData = { 0, 1, 2 }; - world.allReduce(0, - BYTES(messageData.data()), - BYTES(messageData.data()), - MPI_INT, - messageData.size(), - MPI_SUM); - } - - // When performing the migration, MPI will remove it from the pending - // migrations map - REQUIRE(sch.getPendingAppMigrations(appId) == nullptr); - checkPendingMigrationsExpectation( - expectedMigrations, getMpiMockedPendingMigrations().front(), hosts, true); - faabric::Message res = sch.getFunctionResult(firstMsg->id(), 2 * timeToSleep); REQUIRE(res.returnvalue() == 0); @@ -476,5 +454,4 @@ TEST_CASE_METHOD(FunctionMigrationTestFixture, // Clean up world.destroy(); } - }