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

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

commit 37ae5f48d6faebcac81eca4f911af90da482eb27
Author: Andrei Sekretenko <[email protected]>
AuthorDate: Tue May 21 21:12:15 2019 -0400

    Moved the logic of broadcating framework updates into a separate method.
    
    This patch consolidates the scattered logic of sending framework updates
    (namely, the FRAMEWORK_ADDED event to master V1 API subscribers and
    UpdateFrameworkMessage to agents).
    
    NOTE: The UpdateFrameworkMessage is now also being sent in the case of
    non-`forced` subscription request from a framework which has already
    been registered. Previously it was not being sent in this case -
    this likely is a bug.
    
    Review: https://reviews.apache.org/r/70665/
---
 src/master/master.cpp | 47 ++++++++++++++---------------------------------
 src/master/master.hpp |  6 ++++++
 2 files changed, 20 insertions(+), 33 deletions(-)

diff --git a/src/master/master.cpp b/src/master/master.cpp
index 24bd957..72d7453 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -2806,23 +2806,29 @@ void Master::_subscribe(
         framework, frameworkInfo, None(), http, suppressedRoles);
   }
 
+  sendFrameworkUpdates(*framework);
+}
+
+
+void Master::sendFrameworkUpdates(const Framework& framework)
+{
   if (!subscribers.subscribed.empty()) {
     subscribers.send(
-        protobuf::master::event::createFrameworkUpdated(*framework));
+      protobuf::master::event::createFrameworkUpdated(framework));
   }
 
-  // Broadcast the new framework pid to all the slaves. We have to
-  // broadcast because an executor might be running on a slave but
-  // it currently isn't running any tasks.
+  // Broadcast the new framework info and pid to all the slaves. We have to do
+  // this upon frameworkInfo/pid updates because an executor might be running
+  // on a slave but it currently isn't running any tasks.
   foreachvalue (Slave* slave, slaves.registered) {
     UpdateFrameworkMessage message;
-    message.mutable_framework_id()->CopyFrom(frameworkInfo.id());
+    message.mutable_framework_id()->CopyFrom(framework.id());
 
     // TODO(anand): We set 'pid' to UPID() for http frameworks
     // as 'pid' was made optional in 0.24.0. In 0.25.0, we
     // no longer have to set pid here for http frameworks.
-    message.set_pid(UPID());
-    message.mutable_framework_info()->CopyFrom(frameworkInfo);
+    message.set_pid(framework.pid.getOrElse(UPID()));
+    message.mutable_framework_info()->CopyFrom(framework.info);
     send(slave->pid, message);
   }
 }
@@ -3077,11 +3083,6 @@ void Master::_subscribe(
       // if necesssary.
       LOG(INFO) << "Framework " << *framework << " failed over";
       failoverFramework(framework, from);
-
-      if (!subscribers.subscribed.empty()) {
-        subscribers.send(
-            protobuf::master::event::createFrameworkUpdated(*framework));
-      }
     } else {
       LOG(INFO) << "Allowing framework " << *framework
                 << " to subscribe with an already used id";
@@ -3129,34 +3130,14 @@ void Master::_subscribe(
       message.mutable_framework_id()->MergeFrom(frameworkInfo.id());
       message.mutable_master_info()->MergeFrom(info_);
       framework->send(message);
-
-      if (!subscribers.subscribed.empty()) {
-        subscribers.send(
-            protobuf::master::event::createFrameworkUpdated(*framework));
-      }
-      return;
     }
   } else {
     // The framework has not yet reregistered after master failover.
     activateRecoveredFramework(
         framework, frameworkInfo, from, None(), suppressedRoles);
-
-    if (!subscribers.subscribed.empty()) {
-      subscribers.send(
-          protobuf::master::event::createFrameworkUpdated(*framework));
-    }
   }
 
-  // Broadcast the new framework pid to all the slaves. We have to
-  // broadcast because an executor might be running on a slave but
-  // it currently isn't running any tasks.
-  foreachvalue (Slave* slave, slaves.registered) {
-    UpdateFrameworkMessage message;
-    message.mutable_framework_id()->CopyFrom(frameworkInfo.id());
-    message.set_pid(from);
-    message.mutable_framework_info()->CopyFrom(frameworkInfo);
-    send(slave->pid, message);
-  }
+  sendFrameworkUpdates(*framework);
 }
 
 
diff --git a/src/master/master.hpp b/src/master/master.hpp
index bf7a6b4..5ad128d 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -653,11 +653,17 @@ protected:
   // executors and recover the resources.
   void removeFramework(Slave* slave, Framework* framework);
 
+  // TODO(asekretenko): Make sending FrameworkInfo updates to slaves, API
+  // subscribers and anywhere else a responsibility of this method -
+  // currently is is not, see MESOS-9746. After that we can remove the
+  // 'sendFrameworkUpdates()' method.
   void updateFramework(
       Framework* framework,
       const FrameworkInfo& frameworkInfo,
       const std::set<std::string>& suppressedRoles);
 
+  void sendFrameworkUpdates(const Framework& framework);
+
   void disconnect(Framework* framework);
   void deactivate(Framework* framework, bool rescind);
 

Reply via email to