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 2011/06/05 07:37:56 UTC

svn commit: r1131826 - in /incubator/mesos/trunk/src: lxc_isolation_module.cpp lxc_isolation_module.hpp slave.cpp slave.hpp

Author: benh
Date: Sun Jun  5 05:37:55 2011
New Revision: 1131826

URL: http://svn.apache.org/viewvc?rev=1131826&view=rev
Log:
A few fixes to LXC isolation module:
- Kill an executor if we can't set one of its resource limits. (This is
  rather harsh, but we can replace it with something smarter later. At
  least it maintains the "correctness" of the isolation.)
- Redirect IO to files properly (wasn't happening before due to a typo)
- A bit of code refactoring.

Modified:
    incubator/mesos/trunk/src/lxc_isolation_module.cpp
    incubator/mesos/trunk/src/lxc_isolation_module.hpp
    incubator/mesos/trunk/src/slave.cpp
    incubator/mesos/trunk/src/slave.hpp

Modified: incubator/mesos/trunk/src/lxc_isolation_module.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/lxc_isolation_module.cpp?rev=1131826&r1=1131825&r2=1131826&view=diff
==============================================================================
--- incubator/mesos/trunk/src/lxc_isolation_module.cpp (original)
+++ incubator/mesos/trunk/src/lxc_isolation_module.cpp Sun Jun  5 05:37:55 2011
@@ -27,6 +27,14 @@ using namespace nexus;
 using namespace nexus::internal;
 using namespace nexus::internal::slave;
 
+namespace {
+
+const int32_t CPU_SHARES_PER_CPU = 1024;
+const int32_t MIN_CPU_SHARES = 10;
+const int64_t MIN_RSS = 128 * Megabyte;
+
+}
+
 
 LxcIsolationModule::LxcIsolationModule(Slave* slave)
 {
@@ -120,7 +128,7 @@ void LxcIsolationModule::startExecutor(F
     setenv("NEXUS_EXECUTOR_URI", fw->executorInfo.uri.c_str(), 1);
     setenv("NEXUS_USER", fw->user.c_str(), 1);
     setenv("NEXUS_SLAVE_PID", lexical_cast<string>(slave->self()).c_str(), 1);
-    setenv("NEXUS_REDIRECT_IO", slave->local ? "1" : "0", 1);
+    setenv("NEXUS_REDIRECT_IO", slave->local ? "0" : "1", 1);
     setenv("NEXUS_WORK_DIRECTORY", slave->getWorkDirectory(fw->id).c_str(), 1);
 
     // Run lxc-execute.
@@ -149,27 +157,44 @@ void LxcIsolationModule::killExecutor(Fr
 void LxcIsolationModule::resourcesChanged(Framework* fw)
 {
   if (container[fw->id] != "") {
-    // For now, just try setting the CPUs and mem right away.
-    // A slightly smarter thing might be to only update them periodically.
-    int ret;
-    
-    int32_t cpuShares = max(1024 * fw->resources.cpus, 10);
-    LOG(INFO) << "Setting CPU shares for " << fw->id << " to " << cpuShares;
-    ret = shell("lxc-cgroup -n %s cpu.shares %d",
-                container[fw->id].c_str(), cpuShares);
-    if (ret != 0)
-      LOG(ERROR) << "lxc-cgroup returned " << ret;
+    // For now, just try setting the CPUs and memory right away, and kill the
+    // framework if this fails.
+    // A smarter thing to do might be to only update them periodically in a
+    // separate thread, and to give frameworks some time to scale down their
+    // memory usage.
+
+    int32_t cpuShares = max(CPU_SHARES_PER_CPU * fw->resources.cpus,
+                            MIN_CPU_SHARES);
+    if (!setResourceLimit(fw, "cpu.shares", cpuShares)) {
+      slave->removeExecutor(fw->id, true);
+      return;
+    }
 
-    int64_t rssLimit = max(fw->resources.mem, 128 * Megabyte);
-    LOG(INFO) << "Setting RSS limit for " << fw->id << " to " << rssLimit;
-    ret = shell("lxc-cgroup -n %s memory.limit_in_bytes %lld",
-                container[fw->id].c_str(), rssLimit);
-    if (ret != 0)
-      LOG(ERROR) << "lxc-cgroup returned " << ret;
-    
-    // TODO: Decreasing the RSS limit will fail if the current RSS is too
-    // large and memory can't be swapped out. In that case, we should
-    // either freeze the container before changing RSS, or just kill it.
+    int64_t rssLimit = max(fw->resources.mem, MIN_RSS);
+    if (!setResourceLimit(fw, "memory.limit_in_bytes", rssLimit)) {
+      slave->removeExecutor(fw->id, true);
+      return;
+    }
+  }
+}
+
+
+bool LxcIsolationModule::setResourceLimit(Framework* fw,
+                                          const string& property,
+                                          int64_t value)
+{
+  LOG(INFO) << "Setting " << property << " for framework " << fw->id
+            << " to " << value;
+  int ret = shell("lxc-cgroup -n %s %s %lld",
+                  container[fw->id].c_str(),
+                  property.c_str(),
+                  value);
+  if (ret != 0) {
+    LOG(ERROR) << "Failed to set " << property << " for framework " << fw->id
+               << ": lxc-cgroup returned " << ret;
+    return false;
+  } else {
+    return true;
   }
 }
 

Modified: incubator/mesos/trunk/src/lxc_isolation_module.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/lxc_isolation_module.hpp?rev=1131826&r1=1131825&r2=1131826&view=diff
==============================================================================
--- incubator/mesos/trunk/src/lxc_isolation_module.hpp (original)
+++ incubator/mesos/trunk/src/lxc_isolation_module.hpp Sun Jun  5 05:37:55 2011
@@ -50,7 +50,12 @@ public:
   virtual void resourcesChanged(Framework* framework);
 
 protected:
+  // Run a shell command formatted with varargs and return its exit code.
   int shell(const char* format, ...);
+
+  // Attempt to set a resource limit of a framework's container for a given
+  // cgroup property (e.g. cpu.shares). Returns true on success.
+  bool setResourceLimit(Framework* fw, const string& property, int64_t value);
 };
 
 }}}

Modified: incubator/mesos/trunk/src/slave.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/slave.cpp?rev=1131826&r1=1131825&r2=1131826&view=diff
==============================================================================
--- incubator/mesos/trunk/src/slave.cpp (original)
+++ incubator/mesos/trunk/src/slave.cpp Sun Jun  5 05:37:55 2011
@@ -229,13 +229,13 @@ void Slave::operator () ()
       case M2S_KILL_TASK: {
         unpack<M2S_KILL_TASK>(fid, tid);
         LOG(INFO) << "Killing task " << fid << ":" << tid;
+        if (Executor *ex = getExecutor(fid)) {
+          send(ex->pid, pack<S2E_KILL_TASK>(tid));
+        }
         if (Framework *fw = getFramework(fid)) {
           fw->removeTask(tid);
           isolationModule->resourcesChanged(fw);
         }
-        if (Executor *ex = getExecutor(fid)) {
-          send(ex->pid, pack<S2E_KILL_TASK>(tid));
-        }
         // Report to master that the task is killed
         send(master, pack<S2M_STATUS_UPDATE>(id, fid, tid, TASK_KILLED, ""));
         break;

Modified: incubator/mesos/trunk/src/slave.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/slave.hpp?rev=1131826&r1=1131825&r2=1131826&view=diff
==============================================================================
--- incubator/mesos/trunk/src/slave.hpp (original)
+++ incubator/mesos/trunk/src/slave.hpp Sun Jun  5 05:37:55 2011
@@ -187,6 +187,9 @@ public:
 
   string getWorkDirectory(FrameworkID fid);
 
+  // Remove a framework's Executor, possibly killing its process
+  void removeExecutor(FrameworkID frameworkId, bool killProcess);
+
   // TODO(benh): Can this be cleaner?
   // Make self() public so that isolation modules and tests can access it
   using Tuple<ReliableProcess>::self;
@@ -202,9 +205,6 @@ protected:
   // (needed if we received tasks while the executor was starting up)
   void sendQueuedTasks(Framework *framework);
 
-  // Remove a framework's Executor, possibly killing its process
-  void removeExecutor(FrameworkID frameworkId, bool killProcess);
-
   // Kill a framework (including its executor)
   void killFramework(Framework *fw);