[1/2] mesos git commit: Added a safety CHECK when accessing activeRoles in the master.

[bm...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master e591e4177 -> e8460826f


Added a safety CHECK when accessing activeRoles in the master.

Currently there is a use of the [] operator that is not preceded
by a CHECK that the key is contained in the map. This leads to a
hard to diagnose issue if there are bugs that violate this
assumption.

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


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

Branch: refs/heads/master
Commit: e8460826fc020a079a36a6993718d7c5bce98515
Parents: d0b519b
Author: Benjamin Mahler <bm...@apache.org>
Authored: Thu Jan 26 14:56:58 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Sun Jan 29 16:44:17 2017 -0800

----------------------------------------------------------------------
 src/master/master.cpp | 2 ++
 1 file changed, 2 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e8460826/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index de100ea..284566c 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -7701,6 +7701,8 @@ void Master::removeFramework(Framework* framework)
       << "Unknown role '" << role << "'"
       << " of framework " << *framework;
 
+    CHECK(activeRoles.contains(role));
+
     activeRoles[role]->removeFramework(framework);
     if (activeRoles[role]->frameworks.empty()) {
       delete activeRoles[role];

[2/2] mesos git commit: Added TODOs to make the master code safer.

[bm...@apache.org]

Added TODOs to make the master code safer.

The executor and tasks fields in the master's structs for agents
and frameworks are publicly visible, but yet for correctness it
is required that callers made modifications through the member
functions which carry additional logic (e.g. tracking allocated
resources). The TODO here is to make these private and to provide
public views that are const.

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


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

Branch: refs/heads/master
Commit: d0b519b23b468bf3ca45697b207880912704a31a
Parents: e591e41
Author: Benjamin Mahler <bm...@apache.org>
Authored: Wed Jan 25 16:18:58 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Sun Jan 29 16:44:17 2017 -0800

----------------------------------------------------------------------
 src/master/master.hpp | 13 +++++++++++++
 1 file changed, 13 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d0b519b2/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 7e38af4..812c740 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -201,6 +201,10 @@ struct Slave
   Option<process::Timer> reregistrationTimer;
 
   // Executors running on this slave.
+  //
+  // TODO(bmahler): Make this private to enforce that `addExecutor()`
+  // and `removeExecutor()` are used, and provide a const view into
+  // the executors.
   hashmap<FrameworkID, hashmap<ExecutorID, ExecutorInfo>> executors;
 
   // Tasks that have not yet been launched because they are currently
@@ -210,6 +214,10 @@ struct Slave
   hashmap<FrameworkID, hashmap<TaskID, TaskInfo>> pendingTasks;
 
   // Tasks present on this slave.
+  //
+  // TODO(bmahler): Make this private to enforce that `addTask()` and
+  // `removeTask()` are used, and provide a const view into the tasks.
+  //
   // TODO(bmahler): The task pointer ownership complexity arises from the fact
   // that we own the pointer here, but it's shared with the Framework struct.
   // We should find a way to eliminate this.
@@ -2570,6 +2578,8 @@ struct Framework
   // being authorized.
   hashmap<TaskID, TaskInfo> pendingTasks;
 
+  // TODO(bmahler): Make this private to enforce that `addTask()` and
+  // `removeTask()` are used, and provide a const view into the tasks.
   hashmap<TaskID, Task*> tasks;
 
   // Tasks launched by this framework that have reached a terminal
@@ -2592,6 +2602,9 @@ struct Framework
 
   hashset<InverseOffer*> inverseOffers; // Active inverse offers for framework.
 
+  // TODO(bmahler): Make this private to enforce that `addExecutor()`
+  // and `removeExecutor()` are used, and provide a const view into
+  // the executors.
   hashmap<SlaveID, hashmap<ExecutorID, ExecutorInfo>> executors;
 
   // NOTE: For the used and offered resources below, we keep the