This is an automated email from the ASF dual-hosted git repository.

vinodkone pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit f3db5ad2ca4b01f000e9d204ea970434d585f38b
Author: Vinod Kone <[email protected]>
AuthorDate: Thu Nov 15 15:01:18 2018 -0600

    Improved log messages in master when adding/removing tasks/executors.
    
    Made the log messages and the calling sites consistent and also added
    one for adding an executor.
    
    Review: https://reviews.apache.org/r/61128/
---
 src/master/master.cpp | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/src/master/master.cpp b/src/master/master.cpp
index 1e326ec..9458ff1 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -4053,6 +4053,17 @@ void Master::addExecutor(
   CHECK(slave->connected) << "Adding executor " << executorInfo.executor_id()
                           << " to disconnected agent " << *slave;
 
+  // Note that we explicitly convert from protobuf to `Resources` here
+  // and then use the result below to avoid performance penalty for multiple
+  // conversions and validations implied by conversion.
+  // Conversion is safe, as resources have already passed validation.
+  const Resources resources = executorInfo.resources();
+
+  LOG(INFO) << "Adding executor '" << executorInfo.executor_id()
+            << "' with resources " << resources
+            << " of framework " << *framework
+            << " on agent " << *slave;
+
   slave->addExecutor(framework->id(), executorInfo);
   framework->addExecutor(slave->id, executorInfo);
 }
@@ -4068,6 +4079,17 @@ void Master::addTask(
   CHECK(slave->connected) << "Adding task " << task.task_id()
                           << " to disconnected agent " << *slave;
 
+  // Note that we explicitly convert from protobuf to `Resources` here
+  // and then use the result below to avoid performance penalty for multiple
+  // conversions and validations implied by conversion.
+  // Conversion is safe, as resources have already passed validation.
+  const Resources resources = task.resources();
+
+  LOG(INFO) << "Adding task " << task.task_id()
+            << " with resources " << resources
+            << " of framework " << *framework
+            << " on agent " << *slave;
+
   // Add the task to the framework and slave.
   Task* t = new Task(protobuf::createTask(task, TASK_STAGING, 
framework->id()));
 
@@ -11082,12 +11104,17 @@ void Master::removeExecutor(
 
   ExecutorInfo executor = slave->executors[frameworkId][executorId];
 
+  // Note that we explicitly convert from protobuf to `Resources` here
+  // and then use the result below to avoid performance penalty for multiple
+  // conversions and validations implied by conversion.
+  // Conversion is safe, as resources have already passed validation.
+  const Resources resources = executor.resources();
+
   LOG(INFO) << "Removing executor '" << executorId
-            << "' with resources " << executor.resources()
+            << "' with resources " << resources
             << " of framework " << frameworkId << " on agent " << *slave;
 
-  allocator->recoverResources(
-      frameworkId, slave->id, executor.resources(), None());
+  allocator->recoverResources(frameworkId, slave->id, resources, None());
 
   Framework* framework = getFramework(frameworkId);
   if (framework != nullptr) { // The framework might not be reregistered yet.
@@ -12227,12 +12254,6 @@ void Slave::addTask(Task* task)
   if (!protobuf::isTerminalState(task->state())) {
     usedResources[frameworkId] += resources;
   }
-
-  // Note that we use `Resources` for output as it's faster than
-  // logging raw protobuf data.
-  LOG(INFO) << "Adding task " << taskId
-            << " with resources " << resources
-            << " on agent " << *this;
 }
 
 

Reply via email to