[5/5] mesos git commit: Improved various comments.

[vi...@apache.org]

Improved various comments.

Review: https://reviews.apache.org/r/54182/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/71eb9c73
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/71eb9c73
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/71eb9c73

Branch: refs/heads/master
Commit: 71eb9c7399fbd18a17c178eb6dd32f81b968d6a0
Parents: ee6f141
Author: Neil Conway <ne...@gmail.com>
Authored: Fri Dec 16 14:23:56 2016 -0500
Committer: Vinod Kone <vi...@gmail.com>
Committed: Fri Dec 16 14:23:56 2016 -0500

----------------------------------------------------------------------
 src/master/master.cpp         | 10 ++++++----
 src/master/master.hpp         | 13 +++++++++++--
 src/tests/partition_tests.cpp |  8 +++++---
 3 files changed, 22 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/71eb9c73/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index efac4f8..b664550 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -5453,10 +5453,12 @@ void Master::_reregisterSlave(
 
   ++metrics->slave_reregistrations;
 
-  // Check whether this master was the one that removed the
-  // reregistering agent from the cluster originally. This is false
-  // if the master has failed over since the agent was removed, for
-  // example.
+  // Check if this master was the one that removed the reregistering
+  // agent from the cluster originally. This is false if the master
+  // has failed over since the agent was removed, for example. Since
+  // `removed` is a cache, we might mistakenly think the master has
+  // failed over and neglect to remove non-partition-aware frameworks
+  // on reregistering agents, but that should be rare in practice.
   bool slaveWasRemoved = slaves.removed.get(slave->id).isSome();
 
   slaves.removed.erase(slave->id);

http://git-wip-us.apache.org/repos/asf/mesos/blob/71eb9c73/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 4f2d2f8..89b3c39 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1731,15 +1731,21 @@ private:
     // This collection includes agents that have gracefully shutdown,
     // as well as those that have been marked unreachable. We keep a
     // cache here to prevent this from growing in an unbounded manner.
+    //
     // TODO(bmahler): Ideally we could use a cache with set semantics.
+    //
+    // TODO(neilc): Consider storing all agent IDs that have been
+    // marked unreachable by this master.
     Cache<SlaveID, Nothing> removed;
 
     // Slaves that have been marked unreachable. We recover this from
     // the registry, so it includes slaves marked as unreachable by
-    // other instances of the master. Note that we use a linkedhashmap
+    // other instances of the master. Note that we use a LinkedHashMap
     // to ensure the order of elements here matches the order in the
     // registry's unreachable list, which matches the order in which
-    // agents are marked unreachable.
+    // agents are marked unreachable. This list is garbage collected;
+    // GC behavior is governed by the `registry_gc_interval`,
+    // `registry_max_agent_age`, and `registry_max_agent_count` flags.
     LinkedHashMap<SlaveID, TimeInfo> unreachable;
 
     // This rate limiter is used to limit the removal of slaves failing
@@ -2594,6 +2600,9 @@ struct Framework
 
   hashmap<TaskID, Task*> tasks;
 
+  // Tasks launched by this framework that have reached a terminal
+  // state and have had all their updates acknowledged. We only keep a
+  // fixed-size cache to avoid consuming too much memory.
   boost::circular_buffer<process::Owned<Task>> completedTasks;
 
   hashset<Offer*> offers; // Active offers for framework.

http://git-wip-us.apache.org/repos/asf/mesos/blob/71eb9c73/src/tests/partition_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/partition_tests.cpp b/src/tests/partition_tests.cpp
index 0b4bc7a..e1e2025 100644
--- a/src/tests/partition_tests.cpp
+++ b/src/tests/partition_tests.cpp
@@ -237,6 +237,8 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, ReregisterSlavePartitionAware)
   EXPECT_CALL(sched, statusUpdate(&driver, _))
     .WillOnce(FutureArg<1>(&unreachableStatus));
 
+  // We expect to get a `slaveLost` callback, even though this
+  // scheduler is partition-aware.
   Future<Nothing> slaveLost;
   EXPECT_CALL(sched, slaveLost(&driver, _))
     .WillOnce(FutureSatisfy(&slaveLost));
@@ -377,8 +379,6 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, ReregisterSlaveNotPartitionAware)
   EXPECT_CALL(sched, statusUpdate(&driver, _))
     .WillOnce(FutureArg<1>(&lostStatus));
 
-  // Note that we expect to get `slaveLost` callbacks in both
-  // schedulers, regardless of PARTITION_AWARE.
   Future<Nothing> slaveLost;
   EXPECT_CALL(sched, slaveLost(&driver, _))
     .WillOnce(FutureSatisfy(&slaveLost));
@@ -971,7 +971,9 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(PartitionTest, DisconnectedFramework)
 
   DROP_PROTOBUFS(PongSlaveMessage(), _, _);
 
-  // Notify the slave about the new master and wait for it to reregister.
+  // Notify the slave about the new master and wait for it to
+  // reregister. The task on the agent should continue running,
+  // because the master has failed over.
   Future<SlaveReregisteredMessage> slaveReregistered = FUTURE_PROTOBUF(
       SlaveReregisteredMessage(), master.get()->pid, slave.get()->pid);