You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2013/03/08 23:44:19 UTC

svn commit: r1454612 - in /incubator/mesos/trunk/src: linux/cgroups.cpp slave/cgroups_isolation_module.cpp tests/cgroups_tests.cpp tests/utils.hpp

Author: vinodkone
Date: Fri Mar  8 22:44:19 2013
New Revision: 1454612

URL: http://svn.apache.org/r1454612
Log:
Fixed cgroups isolation module to close the lock
file on finalize and cleaned up cgroups tests.

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

Modified:
    incubator/mesos/trunk/src/linux/cgroups.cpp
    incubator/mesos/trunk/src/slave/cgroups_isolation_module.cpp
    incubator/mesos/trunk/src/tests/cgroups_tests.cpp
    incubator/mesos/trunk/src/tests/utils.hpp

Modified: incubator/mesos/trunk/src/linux/cgroups.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/linux/cgroups.cpp?rev=1454612&r1=1454611&r2=1454612&view=diff
==============================================================================
--- incubator/mesos/trunk/src/linux/cgroups.cpp (original)
+++ incubator/mesos/trunk/src/linux/cgroups.cpp Fri Mar  8 22:44:19 2013
@@ -1250,7 +1250,8 @@ private:
       // Not done yet, keep watching (and possibly retrying).
       delay(interval, self(), &Freezer::watchFrozen, attempt + 1);
     } else {
-      LOG(FATAL) << "Unexpected state: " << strings::trim(state.get());
+      LOG(FATAL) << "Unexpected state: " << strings::trim(state.get())
+                 << " of cgroup " << path::join(hierarchy, cgroup);
     }
   }
 
@@ -1272,7 +1273,8 @@ private:
       // Not done yet, keep watching.
       delay(interval, self(), &Freezer::watchThawed);
     } else {
-      LOG(FATAL) << "Unexpected state: " << strings::trim(state.get());
+      LOG(FATAL) << "Unexpected state: " << strings::trim(state.get())
+                 << " of cgroup " << path::join(hierarchy, cgroup);
     }
   }
 

Modified: incubator/mesos/trunk/src/slave/cgroups_isolation_module.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/slave/cgroups_isolation_module.cpp?rev=1454612&r1=1454611&r2=1454612&view=diff
==============================================================================
--- incubator/mesos/trunk/src/slave/cgroups_isolation_module.cpp (original)
+++ incubator/mesos/trunk/src/slave/cgroups_isolation_module.cpp Fri Mar  8 22:44:19 2013
@@ -438,6 +438,11 @@ void CgroupsIsolationModule::finalize()
     PLOG(FATAL)
       << "Failed to unlock '" << path::join(hierarchy, "mesos", "tasks");
   }
+
+  Try<Nothing> close = os::close(lockFile.get());
+  if (close.isError()) {
+    LOG(ERROR) << "Failed to close advisory lock file: " << close.error();
+  }
 }
 
 

Modified: incubator/mesos/trunk/src/tests/cgroups_tests.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/tests/cgroups_tests.cpp?rev=1454612&r1=1454611&r2=1454612&view=diff
==============================================================================
--- incubator/mesos/trunk/src/tests/cgroups_tests.cpp (original)
+++ incubator/mesos/trunk/src/tests/cgroups_tests.cpp Fri Mar  8 22:44:19 2013
@@ -46,9 +46,9 @@
 
 #include "tests/utils.hpp"
 
-using namespace process;
+using namespace mesos::internal::tests;
 
-const static std::string HIERARCHY = "/tmp/mesos_cgroups_testing_hierarchy";
+using namespace process;
 
 
 class CgroupsTest : public ::testing::Test
@@ -62,22 +62,22 @@ protected:
   static void TearDownTestCase()
   {
     // Remove the testing hierarchy.
-    Try<bool> mounted = cgroups::mounted(HIERARCHY);
+    Try<bool> mounted = cgroups::mounted(TEST_HIERARCHY);
     ASSERT_SOME(mounted);
     if (mounted.get()) {
       // Remove all cgroups.
-      Try<std::vector<std::string> > cgroups = cgroups::get(HIERARCHY);
+      Try<std::vector<std::string> > cgroups = cgroups::get(TEST_HIERARCHY);
       ASSERT_SOME(cgroups);
       foreach (const std::string& cgroup, cgroups.get()) {
-        ASSERT_SOME(cgroups::remove(HIERARCHY, cgroup));
+        ASSERT_SOME(cgroups::remove(TEST_HIERARCHY, cgroup));
       }
 
       // Remove the hierarchy.
-      ASSERT_SOME(cgroups::unmount(HIERARCHY));
+      ASSERT_SOME(cgroups::unmount(TEST_HIERARCHY));
 
       // Remove the directory if still exists.
-      if (os::exists(HIERARCHY)) {
-        os::rmdir(HIERARCHY);
+      if (os::exists(TEST_HIERARCHY)) {
+        os::rmdir(TEST_HIERARCHY);
       }
     }
   }
@@ -123,16 +123,6 @@ public:
     : subsystems(_subsystems) {}
 
 protected:
-  static void SetUpTestCase()
-  {
-    CgroupsTest::SetUpTestCase();
-  }
-
-  static void TearDownTestCase()
-  {
-    CgroupsTest::TearDownTestCase();
-  }
-
   virtual void SetUp()
   {
     Try<std::set<std::string> > hierarchies = cgroups::hierarchies();
@@ -154,7 +144,7 @@ protected:
 
     if (hierarchy.empty()) {
       // Try to mount a hierarchy for testing.
-      ASSERT_SOME(cgroups::mount(HIERARCHY, subsystems))
+      ASSERT_SOME(cgroups::mount(TEST_HIERARCHY, subsystems))
         << "-------------------------------------------------------------\n"
         << "We cannot run any cgroups tests that require\n"
         << "a hierarchy with subsystems '" << subsystems << "'\n"
@@ -167,7 +157,7 @@ protected:
              ->test_case_name() << ".*).\n"
         << "-------------------------------------------------------------";
 
-      hierarchy = HIERARCHY;
+      hierarchy = TEST_HIERARCHY;
     }
 
     // Create a cgroup (removing first if necessary) for the tests to use.
@@ -196,7 +186,7 @@ protected:
       }
     }
 
-    // And destroy HIERARCHY in the event it needed to be created.
+    // And destroy TEST_HIERARCHY in the event it is needed to be created.
     CgroupsTest::TearDownTestCase();
   }
 
@@ -292,11 +282,11 @@ TEST_F(CgroupsAnyHierarchyWithCpuMemoryT
 TEST_F(CgroupsNoHierarchyTest, ROOT_CGROUPS_MountUnmountHierarchy)
 {
   EXPECT_ERROR(cgroups::mount("/tmp", "cpu"));
-  EXPECT_ERROR(cgroups::mount(HIERARCHY, "invalid"));
-  ASSERT_SOME(cgroups::mount(HIERARCHY, "cpu,memory"));
-  EXPECT_ERROR(cgroups::mount(HIERARCHY, "cpuset"));
+  EXPECT_ERROR(cgroups::mount(TEST_HIERARCHY, "invalid"));
+  ASSERT_SOME(cgroups::mount(TEST_HIERARCHY, "cpu,memory"));
+  EXPECT_ERROR(cgroups::mount(TEST_HIERARCHY, "cpuset"));
   EXPECT_ERROR(cgroups::unmount("/tmp"));
-  ASSERT_SOME(cgroups::unmount(HIERARCHY));
+  ASSERT_SOME(cgroups::unmount(TEST_HIERARCHY));
 }
 
 

Modified: incubator/mesos/trunk/src/tests/utils.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/tests/utils.hpp?rev=1454612&r1=1454611&r2=1454612&view=diff
==============================================================================
--- incubator/mesos/trunk/src/tests/utils.hpp (original)
+++ incubator/mesos/trunk/src/tests/utils.hpp Fri Mar  8 22:44:19 2013
@@ -73,6 +73,12 @@ namespace tests {
 extern flags::Flags<logging::Flags, Flags> flags;
 
 
+#ifdef __linux__
+// Cgroups hierarchy used by the cgroups related tests.
+const static std::string TEST_HIERARCHY = "/tmp/mesos_test_cgroup";
+#endif
+
+
 // Helper to create a temporary directory based on the current test
 // case name and test name (derived from TestInfo via
 // ::testing::UnitTest::GetInstance()->current_test_info()).