[
https://issues.apache.org/jira/browse/MESOS-765?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Benjamin Mahler resolved MESOS-765.
-----------------------------------
Resolution: Fixed
{noformat}
commit f6793de10efc596bdf7a09351078cd8577382465
Author: Benjamin Mahler <[email protected]>
Date: Wed Oct 23 20:27:51 2013 -0700
Require ProtobufProcess message handlers to take the sender explicitly.
Review: https://reviews.apache.org/r/14900
{noformat}
{noformat}
commit 1657a84569e8455e7c39b6529a70a248dbab08ac
Author: Benjamin Mahler <[email protected]>
Date: Wed Oct 23 20:29:00 2013 -0700
Updated users of ProtobufProcess::from.
Review: https://reviews.apache.org/r/14901
{noformat}
> Remove the implicit ProtobufProcess::from member variable in favor of an
> explicit UPID passed into install handlers.
> --------------------------------------------------------------------------------------------------------------------
>
> Key: MESOS-765
> URL: https://issues.apache.org/jira/browse/MESOS-765
> Project: Mesos
> Issue Type: Improvement
> Reporter: Benjamin Mahler
> Assignee: Benjamin Mahler
> Labels: technical_debt
>
> Message handlers used by ProtobufProcess must examine the 'from' member
> variable to obtain the sender of the message.
> This has many disadvantages over an explicitly passing 'from' as the first
> argument to message handlers:
> --> Looking at ProtobufProcess::from is incorrect whenever the message
> handler is called outside of the context of receiving a message.
> --> Looking at ProtobufProcess::from is only correct whenever we're in the
> execution context of a message handler.
> --> We've had to add unnecessary continuations that pass an explicit UPID for
> the above reason. In these cases, if we had an explicit UPID in the message
> handler, we could simply re-use the message handler rather than creating a
> continuation that passes an explicit UPID.
> Looking at one example (my notes in XXX blocks):
> void Master::registerFramework(const FrameworkInfo& frameworkInfo)
> {
> if (authenticating.contains(from)) {
> LOG(INFO) << "Queuing up registration request from " << from
> << " because authentication is still in progress";
> // XXX IF THE PID WAS PASSED EXPLICITLY WE COULD DEFER BACK
> // TO registerFramework HERE INSTEAD OF _registerFramework.
> authenticating[from]
> .onReady(defer(self(), &Self::_registerFramework, frameworkInfo, from));
> } else {
> // XXX HERE WE COULD INCLUDE THE LOGIC FOR _registerFramework
> _registerFramework(frameworkInfo, from);
> }
> }
> // XXX WE DO NOT NEED THIS CONTINUATION, IT ONLY EXISTS SO
> // THAT WE CAN PASS from EXPLICITLY.
> void Master::_registerFramework(
> const FrameworkInfo& frameworkInfo,
> const UPID& from)
> {
> if (!elected) {
> LOG(WARNING) << "Ignoring register framework message since not elected
> yet";
> return;
> }
> if (authenticating.contains(from)) {
> // This could happen if another authentication request came
> // through before we are here.
> LOG(WARNING) << "Ignoring registration request from " << from
> << " because authentication is back in progress";
> return;
> }
> if (flags.authenticate && !authenticated.contains(from)) {
> // This could happen if another authentication request came
> // through before we are here or if a framework tried to register
> // without authentication.
> LOG(WARNING) << "Refusing registration of framework at " << from
> << " because it is not authenticated";
> FrameworkErrorMessage message;
> message.set_message("Framework at " + stringify(from) +
> " is not authenticated.");
> reply(message);
> return;
> }
> if (!roles.contains(frameworkInfo.role())) {
> FrameworkErrorMessage message;
> message.set_message("Role '" + frameworkInfo.role() + "' is not valid.");
> reply(message);
> return;
> }
> LOG(INFO) << "Received registration request from " << from;
> // Check if this framework is already registered (because it retries).
> foreachvalue (Framework* framework, frameworks) {
> if (framework->pid == from) {
> LOG(INFO) << "Framework " << framework->id << " (" << framework->pid
> << ") already registered, resending acknowledgement";
> FrameworkRegisteredMessage message;
> message.mutable_framework_id()->MergeFrom(framework->id);
> message.mutable_master_info()->MergeFrom(info);
> reply(message);
> return;
> }
> }
> Framework* framework =
> new Framework(frameworkInfo, newFrameworkId(), from, Clock::now());
> LOG(INFO) << "Registering framework " << framework->id << " at " << from;
> bool rootSubmissions = flags.root_submissions;
> if (framework->info.user() == "root" && rootSubmissions == false) {
> LOG(INFO) << framework << " registering as root, but "
> << "root submissions are disabled on this cluster";
> FrameworkErrorMessage message;
> message.set_message("User 'root' is not allowed to run frameworks");
> reply(message);
> delete framework;
> return;
> }
> addFramework(framework);
> }
--
This message was sent by Atlassian JIRA
(v6.1#6144)
