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 2019/05/06 22:06:40 UTC

[mesos] branch master updated (bb35433 -> a7d5b93)

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

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


    from bb35433  Added MESOS-9418 to the 1.5.4 CHANGELOG.
     new 52227a0  Fixed an issue where /__processes__ never returns a response.
     new a7d5b93  Adds a regression test for MESOS-9766.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 3rdparty/libprocess/src/process.cpp             | 20 +++++---
 3rdparty/libprocess/src/tests/process_tests.cpp | 66 +++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 6 deletions(-)


[mesos] 01/02: Fixed an issue where /__processes__ never returns a response.

Posted by bm...@apache.org.
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 52227a07bb56195fd2c695080746a583ad4c2c87
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Fri May 3 15:51:31 2019 -0400

    Fixed an issue where /__processes__ never returns a response.
    
    It's possible for /__processes__ to never return a response if
    a process is terminated after the /__processes__ handler dispatches
    to it, thus leading the Future to be abandoned.
    
    This patch ensures we ignore those cases, rather than wait forever.
    
    See MESOS-9766.
    
    Review: https://reviews.apache.org/r/70594
---
 3rdparty/libprocess/src/process.cpp | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 1248364..a47b570 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -3396,15 +3396,23 @@ Future<Response> ProcessManager::__processes__(const Request&)
           // high-priority set of events (i.e., mailbox).
           return dispatch(
               process->self(),
-              [process]() -> JSON::Object {
-                return *process;
-              });
+              [process]() -> Option<JSON::Object> {
+                return Option<JSON::Object>(*process);
+              })
+            // We must recover abandoned futures in case
+            // the process is terminated and the dispatch
+            // is dropped.
+            .recover([](const Future<Option<JSON::Object>>& f) {
+              return Option<JSON::Object>::none();
+            });
         },
         process_manager->processes.values()))
-      .then([](const std::vector<JSON::Object>& objects) -> Response {
+      .then([](const std::vector<Option<JSON::Object>>& objects) -> Response {
         JSON::Array array;
-        foreach (const JSON::Object& object, objects) {
-          array.values.push_back(object);
+        foreach (const Option<JSON::Object>& object, objects) {
+          if (object.isSome()) {
+            array.values.push_back(*object);
+          }
         }
         return OK(array);
       });


[mesos] 02/02: Adds a regression test for MESOS-9766.

Posted by bm...@apache.org.
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 a7d5b9355e03d64268d5b6f760dd2af6fe7e923b
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Fri May 3 15:53:28 2019 -0400

    Adds a regression test for MESOS-9766.
    
    This test fails on master prior to applying the fix for MESOS-9766.
    It attempts to ensure that processes are terminated after the
    /__processes__ handler dispatches to them.
    
    Review: https://reviews.apache.org/r/70595
---
 3rdparty/libprocess/src/tests/process_tests.cpp | 66 +++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/3rdparty/libprocess/src/tests/process_tests.cpp b/3rdparty/libprocess/src/tests/process_tests.cpp
index 60f3dd6..05dc5ec 100644
--- a/3rdparty/libprocess/src/tests/process_tests.cpp
+++ b/3rdparty/libprocess/src/tests/process_tests.cpp
@@ -78,7 +78,10 @@ using process::defer;
 using process::Deferred;
 using process::Event;
 using process::Executor;
+using process::DispatchEvent;
 using process::ExitedEvent;
+using process::TerminateEvent;
+using process::Failure;
 using process::Future;
 using process::Message;
 using process::MessageEncoder;
@@ -87,6 +90,7 @@ using process::Owned;
 using process::PID;
 using process::Process;
 using process::ProcessBase;
+using process::Promise;
 using process::run;
 using process::Subprocess;
 using process::TerminateEvent;
@@ -2081,3 +2085,65 @@ TEST_F(ProcessTest, FirewallUninstall)
   terminate(process);
   wait(process);
 }
+
+
+// This ensures that the `/__processes__` endpoint does not hang
+// if one of the dispatches is abandoned. This can occur if a
+// process is terminated with `inject == true` after the
+// `/__processes__` endpoint handler has dispatched to it.
+TEST_F(ProcessTest, ProcessesEndpointNoHang)
+{
+  // This process will hold itself in a dispatch handler
+  // until both are present in its event queue:
+  //   (1) an injected terminate event, and
+  //   (2) a dispatch event from the `__processes__` endpoint.
+  //
+  // At that point, we know that the future for (2) will get
+  // abandoned.
+  class TestProcess : public Process<TestProcess>
+  {
+  public:
+    Future<Nothing> wait_for_terminate(Promise<Nothing>&& p)
+    {
+      p.set(Nothing()); // Notify that we're inside the function.
+
+      Time start = Clock::now();
+
+      while (Clock::now() - start < process::TEST_AWAIT_TIMEOUT) {
+        if (eventCount<TerminateEvent>() == 1 &&
+            eventCount<DispatchEvent>() == 1) {
+          return Nothing();
+        }
+
+        os::sleep(Milliseconds(1));
+      }
+
+      return Failure("Timed out waiting for terminate and dispatch");
+    }
+  };
+
+  PID<TestProcess> process = spawn(new TestProcess(), true);
+
+  Promise<Nothing> promise;
+  Future<Nothing> inside = promise.future();
+
+  Future<Nothing> waited =
+    dispatch(process, &TestProcess::wait_for_terminate, std::move(promise));
+
+  AWAIT_READY(inside);
+
+  http::URL url = http::URL(
+      "http",
+      process::address().ip,
+      process::address().port,
+      "/__processes__");
+
+  Future<http::Response> response = http::get(url);
+
+  terminate(process, true);
+
+  AWAIT_READY(waited);
+
+  AWAIT_READY(response);
+  EXPECT_EQ(http::Status::OK, response->code);
+}