Repository: mesos
Updated Branches:
  refs/heads/master 704eee5ad -> e91e45540


FrameworkInfo is only updated if the re-registration is valid.

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


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

Branch: refs/heads/master
Commit: e91e45540f201ba6c4313dd7314e9267cab8489f
Parents: 704eee5
Author: Guangya Liu <[email protected]>
Authored: Sat Sep 19 13:03:56 2015 -0400
Committer: Joris Van Remoortere <[email protected]>
Committed: Sat Sep 19 14:05:10 2015 -0400

----------------------------------------------------------------------
 src/master/master.cpp | 61 ++++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e91e4554/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 7fb5159..5eef29b 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -1987,7 +1987,25 @@ void Master::_subscribe(
     Framework* framework =
       CHECK_NOTNULL(frameworks.registered[frameworkInfo.id()]);
 
-    // Update the framework's info fields based on those passed during
+    // Note that if the scheduler is retrying we expect it
+    // to close its old connection. But, the master may not
+    // realize that the connection is closed before the retry
+    // occurs so we may kick off a scheduler unnecessarily.
+    if (framework->connected && !subscribe.force()) {
+      LOG(ERROR) << "Disallowing subscription attempt"
+                 << " of framework " << *framework
+                 << " because it is already connected";
+
+      FrameworkErrorMessage message;
+      message.set_message("Framework is already connected");
+
+      http.send(message);
+      http.close();
+      return;
+    }
+
+    // It is now safe to update the framework fields since the request is now
+    // guaranteed to be successful. We use the fields passed in during
     // re-registration.
     LOG(INFO) << "Updating info for framework " << framework->id();
 
@@ -1999,21 +2017,6 @@ void Master::_subscribe(
     if (subscribe.force()) {
       LOG(INFO) << "Framework " << *framework << " failed over";
       failoverFramework(framework, http);
-    } else if (framework->connected) {
-      // Note that if the scheduler is retrying we expect it
-      // to close its old connection. But, the master may not
-      // realize that the connection is closed before the retry
-      // occurs so we may kick off a scheduler unnecessarily.
-      LOG(ERROR) << "Disallowing subscription attempt"
-                 << " of framework " << *framework
-                 << " because it is already connected";
-
-      FrameworkErrorMessage message;
-      message.set_message("Framework is already connected");
-
-      http.send(message);
-      http.close();
-      return;
     } else {
       LOG(INFO) << "Allowing framework " << *framework
                 << " to subscribe with an already used id";
@@ -2220,6 +2223,7 @@ void Master::_subscribe(
 
     FrameworkErrorMessage message;
     message.set_message(authorizationError.get().message);
+
     send(from, message);
     return;
   }
@@ -2286,8 +2290,21 @@ void Master::_subscribe(
     Framework* framework =
       CHECK_NOTNULL(frameworks.registered[frameworkInfo.id()]);
 
-    // Update the framework's info fields based on those passed during
-    // subscription.
+    // Test for the error case first.
+    if ((framework->pid != from) && !subscribe.force()) {
+      LOG(ERROR) << "Disallowing subscription attempt of"
+                 << " framework " << *framework
+                 << " because it is not expected from " << from;
+
+      FrameworkErrorMessage message;
+      message.set_message("Framework failed over");
+      send(from, message);
+      return;
+    }
+
+    // It is now safe to update the framework fields since the request is now
+    // guaranteed to be successful. We use the fields passed in during
+    // re-registration.
     LOG(INFO) << "Updating info for framework " << framework->id();
 
     framework->updateFrameworkInfo(frameworkInfo);
@@ -2303,14 +2320,6 @@ void Master::_subscribe(
       // if necesssary.
       LOG(INFO) << "Framework " << *framework << " failed over";
       failoverFramework(framework, from);
-    } else if (framework->pid != from) {
-      LOG(ERROR) << "Disallowing subscription attempt of"
-                 << " framework " << *framework
-                 << " because it is not expected from " << from;
-      FrameworkErrorMessage message;
-      message.set_message("Framework failed over");
-      send(from, message);
-      return;
     } else {
       LOG(INFO) << "Allowing framework " << *framework
                 << " to subscribe with an already used id";

Reply via email to