You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2016/02/25 11:56:26 UTC

mesos git commit: Added some additional synchronization in ROOT_CGROUPS_Pids_and_Tids.

Repository: mesos
Updated Branches:
  refs/heads/master 014223c63 -> 6642337ba


Added some additional synchronization in ROOT_CGROUPS_Pids_and_Tids.

Adds a set of pipes to mitigate a race between `exec`ing + updating
cgroup tasks and the test reading said cgroup tasks.

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


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

Branch: refs/heads/master
Commit: 6642337baa970ebd265e9497d06d5d8b3fc94e39
Parents: 014223c
Author: Joseph Wu <jo...@mesosphere.io>
Authored: Thu Feb 25 11:22:52 2016 +0100
Committer: Bernd Mathiske <be...@mesosphere.io>
Committed: Thu Feb 25 11:55:48 2016 +0100

----------------------------------------------------------------------
 src/tests/containerizer/isolator_tests.cpp | 28 ++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6642337b/src/tests/containerizer/isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp
index 653b037..7b257de 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -29,6 +29,7 @@
 #include <mesos/slave/isolator.hpp>
 
 #include <process/future.hpp>
+#include <process/io.hpp>
 #include <process/owned.hpp>
 #include <process/reap.hpp>
 
@@ -755,6 +756,14 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
   int pipes[2];
   ASSERT_NE(-1, ::pipe(pipes));
 
+  // Use these to communicate with the test process after it has
+  // exec'd to make sure it is running.
+  int inputPipes[2];
+  ASSERT_NE(-1, ::pipe(inputPipes));
+
+  int outputPipes[2];
+  ASSERT_NE(-1, ::pipe(outputPipes));
+
   vector<string> argv(1);
   argv[0] = "cat";
 
@@ -762,8 +771,8 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
       containerId,
       "cat",
       argv,
-      Subprocess::FD(STDIN_FILENO),
-      Subprocess::FD(STDOUT_FILENO),
+      Subprocess::FD(inputPipes[0], Subprocess::IO::OWNED),
+      Subprocess::FD(outputPipes[1], Subprocess::IO::OWNED),
       Subprocess::FD(STDERR_FILENO),
       None(),
       None(),
@@ -800,7 +809,16 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
 
   ASSERT_SOME(os::close(pipes[1]));
 
-  // Process count should be 1 since 'sleep' is still sleeping.
+  // Write to the test process and wait for an echoed result.
+  // This observation ensures that the "cat" process has
+  // completed its part of the exec() procedure and is now
+  // executing normally.
+  AWAIT_READY(io::write(inputPipes[1], "foo")
+    .then([outputPipes]() -> Future<short> {
+      return io::poll(outputPipes[0], io::READ);
+    }));
+
+  // Process count should be 1 since 'cat' is still idling.
   usage = isolator.get()->usage(containerId);
   AWAIT_READY(usage);
   EXPECT_EQ(1U, usage.get().processes());
@@ -809,6 +827,10 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
   // Ensure all processes are killed.
   AWAIT_READY(launcher.get()->destroy(containerId));
 
+  // Clean up the extra pipes created for synchronization.
+  EXPECT_SOME(os::close(inputPipes[1]));
+  EXPECT_SOME(os::close(outputPipes[0]));
+
   // Wait for the command to complete.
   AWAIT_READY(status);