[jira] [Created] (MESOS-765) Remove the implicit ProtobufProcess::from member variable in favor of an explicit UPID passed into install handlers.

["Benjamin Mahler (JIRA)" <ji...@apache.org>]

Benjamin Mahler created MESOS-765:
-------------------------------------

             Summary: 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


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)