You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2017/05/27 01:19:50 UTC

[1/9] mesos git commit: Added new agent flag 'executor_reregistration_timeout'.

Repository: mesos
Updated Branches:
  refs/heads/1.3.x 904729ed3 -> 7c40f93f5


Added new agent flag 'executor_reregistration_timeout'.

This patch adds a new agent flag, 'executor_reregistration_timeout',
which allows operators to configure the time which an executor has
to reregister with the agent before it will be considered gone and
terminated.

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


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

Branch: refs/heads/1.3.x
Commit: d68d795c51eb111a8243532e2fc057a89d4f9cea
Parents: 904729e
Author: Greg Mann <gr...@mesosphere.io>
Authored: Tue May 23 15:36:36 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri May 26 17:49:38 2017 -0700

----------------------------------------------------------------------
 docs/configuration.md   | 11 +++++++++++
 src/slave/constants.hpp | 11 ++++++++++-
 src/slave/flags.cpp     | 16 ++++++++++++++++
 src/slave/flags.hpp     |  1 +
 src/slave/slave.cpp     |  8 +++++---
 5 files changed, 43 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d68d795c/docs/configuration.md
----------------------------------------------------------------------
diff --git a/docs/configuration.md b/docs/configuration.md
index c5194e7..59e1bbe 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -1451,6 +1451,17 @@ shutting it down (e.g., 60secs, 3mins, etc) (default: 1mins)
 </tr>
 <tr>
   <td>
+    --executor_reregistration_timeout=VALUE
+  </td>
+  <td>
+The timeout within which an executor is expected to re-register after
+the agent has restarted, before the agent considers it gone and shuts
+it down. Note that currently, the agent will not re-register with the
+master until this timeout has elapsed (see MESOS-7539). (default: 2secs)
+  </td>
+</tr>
+<tr>
+  <td>
     --max_completed_executors_per_framework
   </td>
   <td>

http://git-wip-us.apache.org/repos/asf/mesos/blob/d68d795c/src/slave/constants.hpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.hpp b/src/slave/constants.hpp
index ef978d6..398bf62 100644
--- a/src/slave/constants.hpp
+++ b/src/slave/constants.hpp
@@ -33,7 +33,16 @@ namespace slave {
 // details in MESOS-1023.
 
 constexpr Duration EXECUTOR_REGISTRATION_TIMEOUT = Minutes(1);
-constexpr Duration EXECUTOR_REREGISTER_TIMEOUT = Seconds(2);
+constexpr Duration EXECUTOR_REREGISTRATION_TIMEOUT = Seconds(2);
+
+// The maximum timeout within which an executor can re-register.
+// Note that this value has to be << 'MIN_AGENT_REREGISTER_TIMEOUT'
+// declared in 'master/constants.hpp'; since agent recovery will only
+// complete after this timeout has elapsed, this ensures that the
+// agent can re-register with the master before it is marked
+// unreachable and its tasks are transitioned to TASK_UNREACHABLE or
+// TASK_LOST.
+constexpr Duration MAX_EXECUTOR_REREGISTRATION_TIMEOUT = Seconds(15);
 
 // The default amount of time to wait for the executor to
 // shut down before destroying the container.

http://git-wip-us.apache.org/repos/asf/mesos/blob/d68d795c/src/slave/flags.cpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index bc63a6a..ba8d428 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -332,6 +332,22 @@ mesos::internal::slave::Flags::Flags()
       "shutting it down (e.g., 60secs, 3mins, etc)",
       EXECUTOR_REGISTRATION_TIMEOUT);
 
+  add(&Flags::executor_reregistration_timeout,
+      "executor_reregistration_timeout",
+      "The timeout within which an executor is expected to re-register after\n"
+      "the agent has restarted, before the agent considers it gone and shuts\n"
+      "it down. Note that currently, the agent will not re-register with the\n"
+      "master until this timeout has elapsed (see MESOS-7539).",
+      EXECUTOR_REREGISTRATION_TIMEOUT,
+      [](const Duration& value) -> Option<Error> {
+        if (value > MAX_EXECUTOR_REREGISTRATION_TIMEOUT) {
+          return Error("Expected `--executor_reregistration_timeout` "
+                       "to be not more than " +
+                       stringify(MAX_EXECUTOR_REREGISTRATION_TIMEOUT));
+        }
+        return None();
+      });
+
   add(&Flags::executor_shutdown_grace_period,
       "executor_shutdown_grace_period",
       "Default amount of time to wait for an executor to shut down\n"

http://git-wip-us.apache.org/repos/asf/mesos/blob/d68d795c/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index e5784ef..32466e0 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -77,6 +77,7 @@ public:
   Duration authentication_backoff_factor;
   Option<JSON::Object> executor_environment_variables;
   Duration executor_registration_timeout;
+  Duration executor_reregistration_timeout;
   Duration executor_shutdown_grace_period;
 #ifdef USE_SSL_SOCKET
   Option<Path> executor_secret_key;

http://git-wip-us.apache.org/repos/asf/mesos/blob/d68d795c/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 1892326..a850713 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -4182,7 +4182,7 @@ void Slave::reregisterExecutorTimeout()
               TaskStatus::REASON_EXECUTOR_REREGISTRATION_TIMEOUT);
           termination.set_message(
               "Executor did not re-register within " +
-              stringify(EXECUTOR_REREGISTER_TIMEOUT));
+              stringify(flags.executor_reregistration_timeout));
 
           executor->pendingTermination = termination;
           break;
@@ -4196,6 +4196,8 @@ void Slave::reregisterExecutorTimeout()
   }
 
   // Signal the end of recovery.
+  // TODO(greggomann): Allow the agent to complete recovery before the executor
+  // re-registration timeout has elapsed. See MESOS-7539
   recoveryInfo.recovered.set(Nothing());
 }
 
@@ -5941,7 +5943,7 @@ Future<Nothing> Slave::_recover()
 
   if (!frameworks.empty() && flags.recover == "reconnect") {
     // Cleanup unregistered executors after a delay.
-    delay(EXECUTOR_REREGISTER_TIMEOUT,
+    delay(flags.executor_reregistration_timeout,
           self(),
           &Slave::reregisterExecutorTimeout);
 
@@ -7619,7 +7621,7 @@ map<string, string> executorEnvironment(
     // The maximum backoff duration to be used by an executor between two
     // retries when disconnected.
     environment["MESOS_SUBSCRIPTION_BACKOFF_MAX"] =
-      stringify(EXECUTOR_REREGISTER_TIMEOUT);
+      stringify(flags.executor_reregistration_timeout);
   }
 
   if (authenticationToken.isSome()) {


[6/9] mesos git commit: Made the executor driver drop some messages when not connected.

Posted by bm...@apache.org.
Made the executor driver drop some messages when not connected.

This patch updates the executor driver to drop RunTaskMessages,
FrameworkMessages, and StatusUpdateAcknowledgements when it is
not connected to the agent. This facilitates executor reconnect
retry logic which is being added to the agent.

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


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

Branch: refs/heads/1.3.x
Commit: a77d2423c2cc0153c87a8d73ac195e600218453d
Parents: 6709996
Author: Greg Mann <gr...@mesosphere.io>
Authored: Fri May 26 11:14:42 2017 -0400
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri May 26 17:54:10 2017 -0700

----------------------------------------------------------------------
 src/exec/exec.cpp | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a77d2423/src/exec/exec.cpp
----------------------------------------------------------------------
diff --git a/src/exec/exec.cpp b/src/exec/exec.cpp
index f36e370..65c4575 100644
--- a/src/exec/exec.cpp
+++ b/src/exec/exec.cpp
@@ -317,6 +317,12 @@ protected:
       return;
     }
 
+    if (!connected) {
+      VLOG(1) << "Ignoring run task message for task " << task.task_id()
+              << " because the driver is disconnected!";
+      return;
+    }
+
     CHECK(!tasks.contains(task.task_id()))
       << "Unexpected duplicate task " << task.task_id();
 
@@ -371,6 +377,14 @@ protected:
       return;
     }
 
+    if (!connected) {
+      VLOG(1) << "Ignoring status update acknowledgement "
+              << uuid_.get() << " for task " << taskId
+              << " of framework " << frameworkId
+              << " because the driver is disconnected!";
+      return;
+    }
+
     VLOG(1) << "Executor received status update acknowledgement "
             << uuid_.get() << " for task " << taskId
             << " of framework " << frameworkId;
@@ -393,6 +407,12 @@ protected:
       return;
     }
 
+    if (!connected) {
+      VLOG(1) << "Ignoring framework message because "
+              << "the driver is disconnected!";
+      return;
+    }
+
     VLOG(1) << "Executor received framework message";
 
     Stopwatch stopwatch;


[7/9] mesos git commit: Don't crash when re-registering executor from an unknown framework.

Posted by bm...@apache.org.
Don't crash when re-registering executor from an unknown framework.

Rather than crashing the agent, the agent will tell the executor
to shut down.

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


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

Branch: refs/heads/1.3.x
Commit: 27a57a9e3a5a10d1fe03207ada3a409a570a0b48
Parents: a77d242
Author: Benjamin Mahler <bm...@apache.org>
Authored: Thu May 25 15:23:30 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri May 26 17:54:16 2017 -0700

----------------------------------------------------------------------
 src/slave/slave.cpp | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/27a57a9e/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 5ebbb87..ce57620 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -3978,11 +3978,14 @@ void Slave::reregisterExecutor(
     return;
   }
 
-  LOG(INFO) << "Re-registering executor '" << executorId
-            << "' of framework " << frameworkId;
+  if (!frameworks.contains(frameworkId)) {
+    LOG(WARNING) << "Shutting down executor '" << executorId << "'"
+                 << " of framework " << frameworkId
+                 << " because the framework is unknown";
 
-  CHECK(frameworks.contains(frameworkId))
-    << "Unknown framework " << frameworkId;
+    reply(ShutdownExecutorMessage());
+    return;
+  }
 
   Framework* framework = frameworks[frameworkId];
 


[8/9] mesos git commit: Don't crash the agent when an unknown executor re-registers.

Posted by bm...@apache.org.
Don't crash the agent when an unknown executor re-registers.

Rather than crashing the agent, the agent will tell the executor
to shut down.

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


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

Branch: refs/heads/1.3.x
Commit: 107598451bec812a8d845507a938d305b9c7adac
Parents: 27a57a9
Author: Benjamin Mahler <bm...@apache.org>
Authored: Thu May 25 16:44:24 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri May 26 17:54:22 2017 -0700

----------------------------------------------------------------------
 src/slave/slave.cpp | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/10759845/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index ce57620..4b7a336 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -4003,7 +4003,14 @@ void Slave::reregisterExecutor(
   }
 
   Executor* executor = framework->getExecutor(executorId);
-  CHECK_NOTNULL(executor);
+
+  if (executor == nullptr) {
+    LOG(WARNING) << "Shutting down unknown executor '" << executorId << "'"
+                 << " of framework " << frameworkId;
+
+    reply(ShutdownExecutorMessage());
+    return;
+  }
 
   switch (executor->state) {
     case Executor::TERMINATING:


[3/9] mesos git commit: Added MESOS-7540 to the 1.3.1 CHANGELOG.

Posted by bm...@apache.org.
Added MESOS-7540 to the 1.3.1 CHANGELOG.


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

Branch: refs/heads/1.3.x
Commit: ae18261ba450dc9231a46f26e4695cd00a852add
Parents: 9802e96
Author: Benjamin Mahler <bm...@apache.org>
Authored: Fri May 26 17:50:53 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri May 26 17:52:51 2017 -0700

----------------------------------------------------------------------
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/ae18261b/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index 0d8c480..edf5a06 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,6 +5,7 @@ Release Notes - Mesos - Version 1.3.1 (WIP)
 All Issues:
 ** Bug
   * [MESOS-7429] - Allow isolators to inject task-specific environment variables.
+  * [MESOS-7540] - Add an agent flag for executor re-registration timeout.
 
 
 Release Notes - Mesos - Version 1.3.0


[9/9] mesos git commit: Added logging of executor re-registration messages.

Posted by bm...@apache.org.
Added logging of executor re-registration messages.

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


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

Branch: refs/heads/1.3.x
Commit: 7c40f93f5d7a16424b9c9ef947c8aeaf75d75c54
Parents: 1075984
Author: Benjamin Mahler <bm...@apache.org>
Authored: Thu May 25 16:49:22 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri May 26 17:54:28 2017 -0700

----------------------------------------------------------------------
 src/slave/slave.cpp | 4 ++++
 1 file changed, 4 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/7c40f93f/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 4b7a336..ef0daa9 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -3969,6 +3969,10 @@ void Slave::reregisterExecutor(
         state == RUNNING || state == TERMINATING)
     << state;
 
+  LOG(INFO) << "Received re-registration message from"
+            << " executor '" << executorId << "'"
+            << " of framework " << frameworkId;
+
   if (state == TERMINATING) {
     LOG(WARNING) << "Shutting down executor '" << executorId << "'"
                  << " of framework " << frameworkId


[4/9] mesos git commit: Introduced executor reconnect retries on the agent.

Posted by bm...@apache.org.
Introduced executor reconnect retries on the agent.

PID-based v0 executors using Mesos libraries >= 1.1.2 always re-link
with the agent upon receiving the reconnect message. This avoids the
executor replying on a half-open TCP connection to the old agent
(possible if netfilter is dropping packets, see: MESOS-7057).
However, PID-based executors using Mesos libraries < 1.1.2 do not
re-link and are therefore prone to replying on a half-open connection
after the agent restarts. If we only send a single reconnect message,
these "old" executors will reply on their half-open connection,
receive a RST, and think the agent just died. To ensure these "old"
executors can reconnect in the presence of netfilter dropping packets,
we introduced optional retries of the reconnect message. This results
in "old" executors correctly establishing a link when processing the
second reconnect message.

Generally, users should not enable this flag unless they are affected
by this issue.

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


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

Branch: refs/heads/1.3.x
Commit: 6e10737b82de4f936e00273661d509439e13549b
Parents: ae18261
Author: Benjamin Mahler <bm...@apache.org>
Authored: Fri May 26 10:17:58 2017 -0400
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri May 26 17:52:53 2017 -0700

----------------------------------------------------------------------
 src/slave/flags.cpp | 22 +++++++++++
 src/slave/flags.hpp |  1 +
 src/slave/slave.cpp | 99 +++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 117 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6e10737b/src/slave/flags.cpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index ba8d428..36e8ac6 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -348,6 +348,28 @@ mesos::internal::slave::Flags::Flags()
         return None();
       });
 
+  // TODO(bmahler): Remove this once v0 executors are no longer supported.
+  add(&Flags::executor_reregistration_retry_interval,
+      "executor_reregistration_retry_interval",
+      "For PID-based executors, how long the agent waits before retrying\n"
+      "the reconnect message sent to the executor during recovery.\n"
+      "NOTE: Do not use this unless you understand the following\n"
+      "(see MESOS-5332): PID-based executors using Mesos libraries >= 1.1.2\n"
+      "always re-link with the agent upon receiving the reconnect message.\n"
+      "This avoids the executor replying on a half-open TCP connection to\n"
+      "the old agent (possible if netfilter is dropping packets,\n"
+      "see: MESOS-7057). However, PID-based executors using Mesos\n"
+      "libraries < 1.1.2 do not re-link and are therefore prone to\n"
+      "replying on a half-open connection after the agent restarts. If we\n"
+      "only send a single reconnect message, these \"old\" executors will\n"
+      "reply on their half-open connection and receive a RST; without any\n"
+      "retries, they will fail to reconnect and be killed by the agent once\n"
+      "the executor re-registration timeout elapses. To ensure these \"old\"\n"
+      "executors can reconnect in the presence of netfilter dropping\n"
+      "packets, we introduced optional retries of the reconnect message.\n"
+      "This results in \"old\" executors correctly establishing a link\n"
+      "when processing the second reconnect message.");
+
   add(&Flags::executor_shutdown_grace_period,
       "executor_shutdown_grace_period",
       "Default amount of time to wait for an executor to shut down\n"

http://git-wip-us.apache.org/repos/asf/mesos/blob/6e10737b/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 32466e0..e9f0ddc 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -78,6 +78,7 @@ public:
   Option<JSON::Object> executor_environment_variables;
   Duration executor_registration_timeout;
   Duration executor_reregistration_timeout;
+  Option<Duration> executor_reregistration_retry_interval;
   Duration executor_shutdown_grace_period;
 #ifdef USE_SSL_SOCKET
   Option<Path> executor_secret_key;

http://git-wip-us.apache.org/repos/asf/mesos/blob/6e10737b/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index a850713..5ebbb87 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -35,6 +35,7 @@
 
 #include <mesos/module/authenticatee.hpp>
 
+#include <process/after.hpp>
 #include <process/async.hpp>
 #include <process/check.hpp>
 #include <process/collect.hpp>
@@ -43,6 +44,7 @@
 #include <process/dispatch.hpp>
 #include <process/http.hpp>
 #include <process/id.hpp>
+#include <process/loop.hpp>
 #include <process/reap.hpp>
 #include <process/time.hpp>
 
@@ -130,9 +132,13 @@ using std::set;
 using std::string;
 using std::vector;
 
+using process::after;
 using process::async;
 using process::wait; // Necessary on some OS's to disambiguate.
+using process::Break;
 using process::Clock;
+using process::Continue;
+using process::ControlFlow;
 using process::Failure;
 using process::Future;
 using process::Owned;
@@ -3963,10 +3969,11 @@ void Slave::reregisterExecutor(
         state == RUNNING || state == TERMINATING)
     << state;
 
-  if (state != RECOVERING) {
-    LOG(WARNING) << "Shutting down executor '" << executorId
-                 << "' of framework " << frameworkId
-                 << " because the agent is not in recovery mode";
+  if (state == TERMINATING) {
+    LOG(WARNING) << "Shutting down executor '" << executorId << "'"
+                 << " of framework " << frameworkId
+                 << " because the agent is terminating";
+
     reply(ShutdownExecutorMessage());
     return;
   }
@@ -4000,11 +4007,37 @@ void Slave::reregisterExecutor(
     case Executor::TERMINATED:
       // TERMINATED is possible if the executor forks, the parent process
       // terminates and the child process (driver) tries to register!
-    case Executor::RUNNING:
       LOG(WARNING) << "Shutting down executor " << *executor
                    << " because it is in unexpected state " << executor->state;
       reply(ShutdownExecutorMessage());
       break;
+
+    case Executor::RUNNING:
+      if (flags.executor_reregistration_retry_interval.isNone()) {
+        // Previously, when an executor sends a re-registration while
+        // in the RUNNING state, we would shut the executor down. We
+        // preserve that behavior when the optional reconnect retry
+        // is not enabled.
+        LOG(WARNING) << "Shutting down executor " << *executor
+                     << " because it is in unexpected state "
+                     << executor->state;
+        reply(ShutdownExecutorMessage());
+      } else {
+        // When the agent is configured to retry the reconnect requests
+        // to executors, we ignore any further re-registrations. This
+        // is because we can't easily handle re-registering libprocess
+        // based executors in the steady state, and we plan to move to
+        // only allowing v1 HTTP executors (where re-subscription in
+        // the steady state is supported). Also, ignoring this message
+        // ensures that any executors mimicking the libprocess protocol
+        // do not have any illusion of being able to re-register without
+        // an agent restart (hopefully they will commit suicide if they
+        // fail to re-register).
+        LOG(WARNING) << "Ignoring executor re-registration message from "
+                     << *executor << " because it is already registered";
+      }
+      break;
+
     case Executor::REGISTERING: {
       executor->state = Executor::RUNNING;
 
@@ -5915,6 +5948,62 @@ Future<Nothing> Slave::_recover()
           ReconnectExecutorMessage message;
           message.mutable_slave_id()->MergeFrom(info.id());
           send(executor->pid.get(), message);
+
+          // PID-based executors using Mesos libraries >= 1.1.2 always
+          // re-link with the agent upon receiving the reconnect message.
+          // This avoids the executor replying on a half-open TCP
+          // connection to the old agent (possible if netfilter is
+          // dropping packets, see: MESOS-7057). However, PID-based
+          // executors using Mesos libraries < 1.1.2 do not re-link
+          // and are therefore prone to replying on a half-open connection
+          // after the agent restarts. If we only send a single reconnect
+          // message, these "old" executors will reply on their half-open
+          // connection and receive a RST; without any retries, they will
+          // fail to reconnect and be killed by the agent once the executor
+          // re-registration timeout elapses. To ensure these "old"
+          // executors can reconnect in the presence of netfilter dropping
+          // packets, we introduced optional retries of the reconnect
+          // message. This results in "old" executors correctly establishing
+          // a link when processing the second reconnect message.
+          if (flags.executor_reregistration_retry_interval.isSome()) {
+            const Duration& retryInterval =
+              flags.executor_reregistration_retry_interval.get();
+
+            const FrameworkID& frameworkId = framework->id();
+            const ExecutorID& executorId = executor->id;
+
+            process::loop(
+                self(),
+                [retryInterval]() {
+                  return after(retryInterval);
+                },
+                [this, frameworkId, executorId, message](Nothing)
+                    -> ControlFlow<Nothing> {
+                  if (state != RECOVERING) {
+                    return Break();
+                  }
+
+                  Framework* framework = getFramework(frameworkId);
+                  if (framework == nullptr) {
+                    return Break();
+                  }
+
+                  Executor* executor = framework->getExecutor(executorId);
+                  if (executor == nullptr) {
+                    return Break();
+                  }
+
+                  if (executor->state != Executor::REGISTERING) {
+                    return Break();
+                  }
+
+                  LOG(INFO) << "Re-sending reconnect request to executor "
+                            << *executor;
+
+                  send(executor->pid.get(), message);
+                  return Continue();
+                });
+          }
         } else if (executor->pid.isNone()) {
           LOG(INFO) << "Waiting for executor " << *executor
                     << " to subscribe";


[5/9] mesos git commit: Added MESOS-7569 to the CHANGELOG.

Posted by bm...@apache.org.
Added MESOS-7569 to the CHANGELOG.


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

Branch: refs/heads/1.3.x
Commit: 6709996909f04481e1d5271cfc0e6e52fed60214
Parents: 6e10737
Author: Benjamin Mahler <bm...@apache.org>
Authored: Fri May 26 17:53:48 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri May 26 17:53:48 2017 -0700

----------------------------------------------------------------------
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/67099969/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index edf5a06..6b770ee 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,6 +6,7 @@ All Issues:
 ** Bug
   * [MESOS-7429] - Allow isolators to inject task-specific environment variables.
   * [MESOS-7540] - Add an agent flag for executor re-registration timeout.
+  * [MESOS-7569] - Allow "old" executors with half-open connections to be preserved during agent upgrade / restart.
 
 
 Release Notes - Mesos - Version 1.3.0


[2/9] mesos git commit: Added 'executor_reregister_timeout' agent flag to the tests.

Posted by bm...@apache.org.
Added 'executor_reregister_timeout' agent flag to the tests.

This patch updates the tests to make use of the new
'executor_reregister_timeout' agent flag.

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


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

Branch: refs/heads/1.3.x
Commit: 9802e96c5d2d102162cbe4f68ca23db6427066aa
Parents: d68d795
Author: Greg Mann <gr...@mesosphere.io>
Authored: Tue May 23 15:37:34 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri May 26 17:49:45 2017 -0700

----------------------------------------------------------------------
 src/tests/containerizer/port_mapping_tests.cpp |  4 ++--
 src/tests/disk_quota_tests.cpp                 |  2 +-
 src/tests/master_tests.cpp                     |  4 ++--
 src/tests/persistent_volume_tests.cpp          |  2 +-
 src/tests/slave_recovery_tests.cpp             | 26 ++++++++++-----------
 src/tests/slave_tests.cpp                      |  2 +-
 6 files changed, 20 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9802e96c/src/tests/containerizer/port_mapping_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp
index a528382..d062f2f 100644
--- a/src/tests/containerizer/port_mapping_tests.cpp
+++ b/src/tests/containerizer/port_mapping_tests.cpp
@@ -1962,7 +1962,7 @@ TEST_F(PortMappingMesosTest, CGROUPS_ROOT_RecoverMixedContainers)
   AWAIT_READY(_recover1);
 
   Clock::settle(); // Wait for slave to schedule reregister timeout.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(slaveFlags.executor_reregistration_timeout);
 
   AWAIT_READY(slaveReregisteredMessage1);
 
@@ -2011,7 +2011,7 @@ TEST_F(PortMappingMesosTest, CGROUPS_ROOT_RecoverMixedContainers)
   AWAIT_READY(_recover2);
 
   Clock::settle(); // Wait for slave to schedule reregister timeout.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(slaveFlags.executor_reregistration_timeout);
 
   AWAIT_READY(slaveReregisteredMessage2);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/9802e96c/src/tests/disk_quota_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/disk_quota_tests.cpp b/src/tests/disk_quota_tests.cpp
index f758bee..9fe2c86 100644
--- a/src/tests/disk_quota_tests.cpp
+++ b/src/tests/disk_quota_tests.cpp
@@ -701,7 +701,7 @@ TEST_F(DiskQuotaTest, SlaveRecovery)
   AWAIT_READY(reregisterExecutorMessage);
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(slave::EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
 
   // NOTE: We resume the clock because we need the reaper to reap the
   // 'du' subprocess.

http://git-wip-us.apache.org/repos/asf/mesos/blob/9802e96c/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index ceee2f4..b0678ad 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -6442,7 +6442,7 @@ TEST_F(MasterTest, AgentRestartNoReregister)
 
   // The agent waits for the executor reregister timeout to expire,
   // even if all executors have re-reregistered.
-  Clock::advance(slave::EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(agentFlags.executor_reregistration_timeout);
   Clock::settle();
 
   // Agent will try to re-register after completing recovery; prevent
@@ -6473,7 +6473,7 @@ TEST_F(MasterTest, AgentRestartNoReregister)
     .WillOnce(FutureArg<1>(&unreachableStatus));
 
   Duration elapsedTime =
-    masterFlags.agent_ping_timeout + slave::EXECUTOR_REREGISTER_TIMEOUT;
+    masterFlags.agent_ping_timeout + agentFlags.executor_reregistration_timeout;
 
   Duration remainingReregisterTime =
     masterFlags.agent_reregister_timeout - elapsedTime;

http://git-wip-us.apache.org/repos/asf/mesos/blob/9802e96c/src/tests/persistent_volume_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/persistent_volume_tests.cpp b/src/tests/persistent_volume_tests.cpp
index d42fd18..8f8bb26 100644
--- a/src/tests/persistent_volume_tests.cpp
+++ b/src/tests/persistent_volume_tests.cpp
@@ -1775,7 +1775,7 @@ TEST_P(PersistentVolumeTest, SlaveRecovery)
   Clock::settle();
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(slave::EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(slaveFlags.executor_reregistration_timeout);
 
   Clock::resume();
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/9802e96c/src/tests/slave_recovery_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_recovery_tests.cpp b/src/tests/slave_recovery_tests.cpp
index 52e78b6..0aa87f5 100644
--- a/src/tests/slave_recovery_tests.cpp
+++ b/src/tests/slave_recovery_tests.cpp
@@ -889,7 +889,7 @@ TYPED_TEST(SlaveRecoveryTest, DISABLED_RecoverUnregisteredHTTPExecutor)
   Clock::settle(); // Wait for slave to schedule reregister timeout.
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
 
   // Now advance time until the reaper reaps the executor.
   while (status.isPending()) {
@@ -1001,7 +1001,7 @@ TYPED_TEST(SlaveRecoveryTest, RecoverUnregisteredExecutor)
   Clock::settle(); // Wait for slave to schedule reregister timeout.
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
 
   // Now advance time until the reaper reaps the executor.
   while (status.isPending()) {
@@ -1119,7 +1119,7 @@ TYPED_TEST(SlaveRecoveryTest, KillTaskUnregisteredExecutor)
   Clock::settle(); // Wait for the agent to schedule reregister timeout.
 
   // Ensure the agent considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
 
   while(executorTerminated.isPending()) {
     Clock::advance(process::MAX_REAP_INTERVAL());
@@ -1256,7 +1256,7 @@ TYPED_TEST(SlaveRecoveryTest, RecoverTerminatedHTTPExecutor)
   Clock::settle(); // Wait for slave to schedule reregister timeout.
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
 
   // Now advance time until the reaper reaps the executor.
   while (status.isPending()) {
@@ -1376,7 +1376,7 @@ TYPED_TEST(SlaveRecoveryTest, RecoverTerminatedExecutor)
   Clock::settle(); // Wait for slave to schedule reregister timeout.
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
 
   // Now advance time until the reaper reaps the executor.
   while (status.isPending()) {
@@ -1493,7 +1493,7 @@ TYPED_TEST(SlaveRecoveryTest, DISABLED_RecoveryTimeout)
   AWAIT_READY(_recover);
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
   Clock::resume();
 
   // Scheduler should receive the TASK_FAILED update.
@@ -2206,7 +2206,7 @@ TYPED_TEST(SlaveRecoveryTest, KillTask)
   AWAIT_READY(reregisterExecutorMessage);
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
   Clock::resume();
 
   // Wait for the slave to re-register.
@@ -2497,7 +2497,7 @@ TYPED_TEST(SlaveRecoveryTest, GCExecutor)
   Clock::settle(); // Wait for slave to schedule reregister timeout.
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
 
   AWAIT_READY(slaveReregisteredMessage);
 
@@ -3388,7 +3388,7 @@ TYPED_TEST(SlaveRecoveryTest, SchedulerFailover)
   AWAIT_READY(reregisterExecutorMessage);
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
   Clock::resume();
 
   // Wait for the slave to re-register.
@@ -3534,7 +3534,7 @@ TYPED_TEST(SlaveRecoveryTest, MasterFailover)
   AWAIT_READY(reregisterExecutorMessage);
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
   Clock::resume();
 
   // Wait for the slave to re-register.
@@ -3693,7 +3693,7 @@ TYPED_TEST(SlaveRecoveryTest, MultipleFrameworks)
   AWAIT_READY(reregisterExecutorMessage2);
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
   Clock::resume();
 
   // Wait for the slave to re-register.
@@ -3869,7 +3869,7 @@ TYPED_TEST(SlaveRecoveryTest, MultipleSlaves)
   AWAIT_READY(reregisterExecutorMessage2);
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags1.executor_reregistration_timeout);
   Clock::resume();
 
   // Wait for the slaves to re-register.
@@ -3990,7 +3990,7 @@ TYPED_TEST(SlaveRecoveryTest, RestartBeforeContainerizerLaunch)
   Clock::settle(); // Wait for slave to schedule reregister timeout.
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
   Clock::resume();
 
   // Scheduler should receive the TASK_FAILED update.

http://git-wip-us.apache.org/repos/asf/mesos/blob/9802e96c/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 8c97dc6..a4b4eb7 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -5005,7 +5005,7 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(SlaveTest, HTTPSchedulerSlaveRestart)
   Clock::settle();
 
   // Ensure the slave considers itself recovered.
-  Clock::advance(slave::EXECUTOR_REREGISTER_TIMEOUT);
+  Clock::advance(flags.executor_reregistration_timeout);
 
   Clock::resume();