Re: [1/3] mesos git commit: Index slaves by UPID in the master.

[James Peach <jo...@gmail.com>]

> On May 19, 2015, at 12:22 PM, bmahler@apache.org wrote:
> 
> Repository: mesos
> Updated Branches:
>  refs/heads/master 26091f461 -> c24268f13
> 
> 
> Index slaves by UPID in the master.
> 
> Review: https://reviews.apache.org/r/34388
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/42cf03af
> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/42cf03af
> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/42cf03af
> 
> Branch: refs/heads/master
> Commit: 42cf03af66f2691d04e5c88ac7e098625d38e0bf
> Parents: b19ffd2
> Author: Benjamin Mahler <be...@gmail.com>
> Authored: Mon May 18 18:37:11 2015 -0700
> Committer: Benjamin Mahler <be...@gmail.com>
> Committed: Tue May 19 11:55:30 2015 -0700
> 
> ----------------------------------------------------------------------
> src/master/master.cpp | 127 +++++++++++++++++++++++----------------------
> src/master/master.hpp |  65 ++++++++++++++++++++++-
> 2 files changed, 129 insertions(+), 63 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/mesos/blob/42cf03af/src/master/master.cpp
> ----------------------------------------------------------------------
> diff --git a/src/master/master.cpp b/src/master/master.cpp
> index eaea79d..d2df99c 100644
> --- a/src/master/master.cpp
> +++ b/src/master/master.cpp
> @@ -973,7 +973,9 @@ void Master::exited(const UPID& pid)
>     }
>   }
> 
> -  // The semantics when a slave gets disconnected are as follows:
> +  // The semantics when a registered slave gets disconnected are as
> +  // follows:
> +  //
>   // 1) If the slave is not checkpointing, the slave is immediately
>   //    removed and all tasks running on it are transitioned to LOST.
>   //    No resources are recovered, because the slave is removed.
> @@ -985,42 +987,42 @@ void Master::exited(const UPID& pid)
>   //    2.2) Framework is not-checkpointing: The slave is not removed
>   //         but the framework is removed from the slave's structs,
>   //         its tasks transitioned to LOST and resources recovered.
> -  foreachvalue (Slave* slave, slaves.registered) {
> -    if (slave->pid == pid) {
> -      LOG(INFO) << "Slave " << *slave << " disconnected";
> -
> -      if (!slave->info.checkpoint()) {
> -        // Remove the slave, if it is not checkpointing.
> -        LOG(INFO) << "Removing disconnected slave " << *slave
> -                  << " because it is not checkpointing!";
> -        removeSlave(slave,
> -                    "slave is non-checkpointing and disconnected");
> -        return;
> -      } else if (slave->connected) {
> -        // Checkpointing slaves can just be disconnected.
> -        disconnect(slave);
> +  if (slaves.registered.contains(pid)) {
> +    Slave* slave = slaves.registered.get(pid);
> +    CHECK_NOTNULL(slave);

Would it be better to do this:
	if (Slave* slave = slaves.registered.get(pid)) {

J