commit c762478ebcbfbcadc513fd6ec57fcdd7a84bbf69
author Benjamin Mahler <bmahler@apache.org> Tue Sep 01 15:08:08 2020 -0400
committer Benjamin Mahler <bmahler@apache.org> Thu Sep 10 13:32:21 2020 -0400
tree b6e6f4c2433ca8506360af57eba023969d126f46
parent 76e330e3f160f59d4909ddeef00ed588f15a218d

Avoided unnecessary [] map operators in the master.

To avoid the potential for accidental insertion into the maps,
we prefer to use the .at operator for const access.

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

src/master/master.cpp

diff --git a/src/master/master.cpp b/src/master/master.cpp
index 97654b5..438ef98 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -10445,7 +10445,7 @@
 
     if (slave != nullptr) {
       foreachkey (const ExecutorID& executorId,
-                  utils::copy(framework->executors[slaveId])) {
+                  utils::copy(framework->executors.at(slaveId))) {
         removeExecutor(slave, framework->id(), executorId);
       }
     }
@@ -10584,7 +10584,7 @@
   // for proper resource accounting.
   if (slave->executors.contains(framework->id())) {
     foreachkey (const ExecutorID& executorId,
-                utils::copy(slave->executors[framework->id()])) {
+                utils::copy(slave->executors.at(framework->id()))) {
       removeExecutor(slave, framework->id(), executorId);
     }
   }
@@ -10805,8 +10805,11 @@
   // Transition the tasks to lost and remove them.
   foreachkey (const FrameworkID& frameworkId, utils::copy(slave->tasks)) {
     Framework* framework = getFramework(frameworkId);
+    CHECK(framework != nullptr)
+      << "Framework " << frameworkId << " not found while removing agent "
+      << *slave << "; agent tasks: " << slave->tasks;
 
-    foreachvalue (Task* task, utils::copy(slave->tasks[frameworkId])) {
+    foreachvalue (Task* task, utils::copy(slave->tasks.at(frameworkId))) {
       // TODO(bmahler): Differentiate between agent removal reasons
       // (e.g. unhealthy vs. unregistered for maintenance).
       const StatusUpdate& update = protobuf::createStatusUpdate(
@@ -10824,7 +10827,7 @@
       updateTask(task, update);
       removeTask(task);
 
-      if (framework == nullptr || !framework->connected()) {
+      if (!framework->connected()) {
         LOG(WARNING) << "Dropping update " << update
                      << " for unknown framework " << frameworkId;
       } else {
@@ -10836,7 +10839,7 @@
   // Remove executors from the slave for proper resource accounting.
   foreachkey (const FrameworkID& frameworkId, utils::copy(slave->executors)) {
     foreachkey (const ExecutorID& executorId,
-                utils::copy(slave->executors[frameworkId])) {
+                utils::copy(slave->executors.at(frameworkId))) {
       removeExecutor(slave, frameworkId, executorId);
     }
   }
@@ -10951,7 +10954,7 @@
       newTaskReason = TaskStatus::REASON_SLAVE_REMOVED_BY_OPERATOR;
     }
 
-    foreachvalue (Task* task, utils::copy(slave->tasks[frameworkId])) {
+    foreachvalue (Task* task, utils::copy(slave->tasks.at(frameworkId))) {
       const StatusUpdate& update = protobuf::createStatusUpdate(
           task->framework_id(),
           task->slave_id(),
@@ -10985,7 +10988,7 @@
   // Remove executors from the slave for proper resource accounting.
   foreachkey (const FrameworkID& frameworkId, utils::copy(slave->executors)) {
     foreachkey (const ExecutorID& executorId,
-                utils::copy(slave->executors[frameworkId])) {
+                utils::copy(slave->executors.at(frameworkId))) {
       removeExecutor(slave, frameworkId, executorId);
     }
   }
@@ -12618,8 +12621,8 @@
     }
   }
 
-  tasks[frameworkId].erase(taskId);
-  if (tasks[frameworkId].empty()) {
+  tasks.at(frameworkId).erase(taskId);
+  if (tasks.at(frameworkId).empty()) {
     tasks.erase(frameworkId);
   }