Do not pass FrameworkID to Framework constructor in Master/Slave. Framework constructor takes FrameworkInfo, which already has a valid FrameworkID.
Review: https://reviews.apache.org/r/32584 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d1724c4d Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d1724c4d Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d1724c4d Branch: refs/heads/master Commit: d1724c4dab65aae8caa3a25ea488ca011f845d36 Parents: 9db1563 Author: Kapil Arya <[email protected]> Authored: Sat Apr 11 01:19:19 2015 -0700 Committer: Adam B <[email protected]> Committed: Sat Apr 11 01:19:19 2015 -0700 ---------------------------------------------------------------------- src/master/master.cpp | 11 ++++++----- src/master/master.hpp | 8 ++++---- src/slave/slave.cpp | 21 +++++++++++++-------- src/slave/slave.hpp | 5 +++-- 4 files changed, 26 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/d1724c4d/src/master/master.cpp ---------------------------------------------------------------------- diff --git a/src/master/master.cpp b/src/master/master.cpp index 618db68..2a2aabe 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -1695,8 +1695,11 @@ void Master::_registerFramework( } } - Framework* framework = - new Framework(frameworkInfo, newFrameworkId(), from, Clock::now()); + // Assign a new FrameworkID. + FrameworkInfo frameworkInfo_ = frameworkInfo; + frameworkInfo_.mutable_id()->CopyFrom(newFrameworkId()); + + Framework* framework = new Framework(frameworkInfo_, from); LOG(INFO) << "Registering framework " << *framework; @@ -1901,9 +1904,7 @@ void Master::_reregisterFramework( // elected Mesos master to which either an existing scheduler or a // failed-over one is connecting. Create a Framework object and add // any tasks it has that have been reported by reconnecting slaves. - Framework* framework = - new Framework(frameworkInfo, frameworkInfo.id(), from, Clock::now()); - framework->reregisteredTime = Clock::now(); + Framework* framework = new Framework(frameworkInfo, from); // TODO(benh): Check for root submissions like above! http://git-wip-us.apache.org/repos/asf/mesos/blob/d1724c4d/src/master/master.hpp ---------------------------------------------------------------------- diff --git a/src/master/master.hpp b/src/master/master.hpp index 05be911..2e08009 100644 --- a/src/master/master.hpp +++ b/src/master/master.hpp @@ -977,10 +977,9 @@ inline std::ostream& operator << (std::ostream& stream, const Slave& slave) struct Framework { Framework(const FrameworkInfo& _info, - const FrameworkID& _id, const process::UPID& _pid, const process::Time& time = process::Clock::now()) - : id(_id), + : id(_info.id()), info(_info), pid(_pid), connected(true), @@ -1097,8 +1096,9 @@ struct Framework } } - const FrameworkID id; // TODO(benh): Store this in 'info'. - + // TODO(karya): Replace 'id' with 'id()' that returns the id from + // 'info'. + const FrameworkID id; // Copied from info.id(). const FrameworkInfo info; process::UPID pid; http://git-wip-us.apache.org/repos/asf/mesos/blob/d1724c4d/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 082f97a..b0a49a9 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -1054,9 +1054,6 @@ void Slave::doReliableRegistration(Duration maxBackoff) completedFramework_->mutable_framework_info(); frameworkInfo->CopyFrom(completedFramework->info); - // TODO(adam-mesos): Needed because FrameworkInfo doesn't have the id. - frameworkInfo->mutable_id()->CopyFrom(completedFramework->id); - completedFramework_->set_pid(completedFramework->pid); foreach (const Owned<Executor>& executor, @@ -1172,7 +1169,7 @@ void Slave::runTask( unschedule = unschedule.then(defer(self(), &Self::unschedule, path)); } - framework = new Framework(this, frameworkId, frameworkInfo, pid); + framework = new Framework(this, frameworkInfo, pid); frameworks[frameworkId] = framework; // Is this same framework in completedFrameworks? If so, move the completed @@ -3895,9 +3892,18 @@ void Slave::recoverFramework(const FrameworkState& state) } CHECK(!frameworks.contains(state.id)); - Framework* framework = new Framework( - this, state.id, state.info.get(), state.pid.get()); + // Merge state.id into state.info. + CHECK_SOME(state.info); + FrameworkInfo frameworkInfo = state.info.get(); + if (!frameworkInfo.has_id()) { + frameworkInfo.mutable_id()->MergeFrom(state.id); + } else { + CHECK_EQ(frameworkInfo.id(), state.id); + } + + CHECK_SOME(state.pid); + Framework* framework = new Framework(this, frameworkInfo, state.pid.get()); frameworks[framework->id] = framework; // Now recover the executors for this framework. @@ -4116,12 +4122,11 @@ double Slave::_resources_percent(const std::string& name) Framework::Framework( Slave* _slave, - const FrameworkID& _id, const FrameworkInfo& _info, const UPID& _pid) : state(RUNNING), slave(_slave), - id(_id), + id(_info.id()), info(_info), pid(_pid), completedExecutors(MAX_COMPLETED_EXECUTORS_PER_FRAMEWORK) http://git-wip-us.apache.org/repos/asf/mesos/blob/d1724c4d/src/slave/slave.hpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp index 19e6b44..bfc7309 100644 --- a/src/slave/slave.hpp +++ b/src/slave/slave.hpp @@ -577,7 +577,6 @@ struct Framework { Framework( Slave* slave, - const FrameworkID& id, const FrameworkInfo& info, const process::UPID& pid); @@ -601,7 +600,9 @@ struct Framework // of the 'Slave' class. Slave* slave; - const FrameworkID id; + // TODO(karya): Replace 'id' with 'id()' that returns the id from + // 'info'. + const FrameworkID id; // Copied from info.id(). const FrameworkInfo info; UPID pid;
