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 2014/06/04 00:43:29 UTC

[1/2] git commit: Updated dropped message logging to use VLOG.

Repository: mesos
Updated Branches:
  refs/heads/master 331076901 -> b4ec4eb98


Updated dropped message logging to use VLOG.

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


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

Branch: refs/heads/master
Commit: b4ec4eb981ab8fbc1c8da373cce2909d0bd16ee3
Parents: 2d6b20e
Author: Ben Mahler <be...@gmail.com>
Authored: Tue Jun 3 14:59:42 2014 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Tue Jun 3 15:17:47 2014 -0700

----------------------------------------------------------------------
 src/master/master.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b4ec4eb9/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 766a0e3..91dc1fd 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -748,8 +748,8 @@ void Master::visit(const MessageEvent& event)
 {
   // All messages are filtered when non-leading.
   if (!elected()) {
-    LOG(WARNING) << "Dropping '" << event.message->name << "' message since "
-                 << "not elected yet";
+    VLOG(1) << "Dropping '" << event.message->name << "' message since "
+            << "not elected yet";
     ++metrics.dropped_messages;
     return;
   }
@@ -762,8 +762,8 @@ void Master::visit(const MessageEvent& event)
   // the additional queueing delay and the accumulated backlog
   // of messages post-recovery?
   if (!recovered.get().isReady()) {
-    LOG(WARNING) << "Dropping '" << event.message->name << "' message since "
-                 << "not recovered yet";
+    VLOG(1) << "Dropping '" << event.message->name << "' message since "
+            << "not recovered yet";
     ++metrics.dropped_messages;
     return;
   }


[2/2] git commit: Fixed memory leaks and cleaned up the mesos containerizer logic.

Posted by bm...@apache.org.
Fixed memory leaks and cleaned up the mesos containerizer logic.

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


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

Branch: refs/heads/master
Commit: 2d6b20e2f839f1f278791162f620a383f4f9026e
Parents: 3310769
Author: Ben Mahler <be...@gmail.com>
Authored: Tue Jun 3 14:58:51 2014 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Tue Jun 3 15:17:47 2014 -0700

----------------------------------------------------------------------
 .../isolators/cgroups/cpushare.cpp              | 42 ++++++++------------
 .../isolators/cgroups/cpushare.hpp              |  1 +
 .../containerizer/isolators/cgroups/mem.cpp     | 32 +++++++--------
 .../containerizer/isolators/cgroups/mem.hpp     |  1 +
 4 files changed, 33 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2d6b20e2/src/slave/containerizer/isolators/cgroups/cpushare.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/cgroups/cpushare.cpp b/src/slave/containerizer/isolators/cgroups/cpushare.cpp
index 47a61af..3d253af 100644
--- a/src/slave/containerizer/isolators/cgroups/cpushare.cpp
+++ b/src/slave/containerizer/isolators/cgroups/cpushare.cpp
@@ -126,14 +126,10 @@ Future<Nothing> CgroupsCpushareIsolatorProcess::recover(
     }
 
     const ContainerID& containerId = state.id.get();
+    const string cgroup = path::join(flags.cgroups_root, containerId.value());
 
-    Info* info = new Info(
-        containerId, path::join(flags.cgroups_root, containerId.value()));
-    CHECK_NOTNULL(info);
-
-    Try<bool> exists = cgroups::exists(hierarchies["cpu"], info->cgroup);
+    Try<bool> exists = cgroups::exists(hierarchies["cpu"], cgroup);
     if (exists.isError()) {
-      delete info;
       foreachvalue (Info* info, infos) {
         delete info;
       }
@@ -151,8 +147,8 @@ Future<Nothing> CgroupsCpushareIsolatorProcess::recover(
       continue;
     }
 
-    infos[containerId] = info;
-    cgroups.insert(info->cgroup);
+    infos[containerId] = new Info(containerId, cgroup);;
+    cgroups.insert(cgroup);
   }
 
   // Remove orphans in the cpu hierarchy.
@@ -218,27 +214,27 @@ Future<Option<CommandInfo> > CgroupsCpushareIsolatorProcess::prepare(
     return Failure("Container has already been prepared");
   }
 
+  // TODO(bmahler): Don't insert into 'infos' unless we create the
+  // cgroup successfully. It's safe for now because 'cleanup' gets
+  // called if we return a Failure, but cleanup will fail because
+  // the cgroup does not exist when cgroups::destroy is called.
   Info* info = new Info(
       containerId, path::join(flags.cgroups_root, containerId.value()));
 
-  infos[containerId] = CHECK_NOTNULL(info);
+  infos[containerId] = info;
 
   // Create a 'cpu' cgroup for this container.
   Try<bool> exists = cgroups::exists(hierarchies["cpu"], info->cgroup);
 
   if (exists.isError()) {
     return Failure("Failed to prepare isolator: " + exists.error());
-  }
-
-  if (exists.get()) {
+  } else if (exists.get()) {
     return Failure("Failed to prepare isolator: cgroup already exists");
   }
 
-  if (!exists.get()) {
-    Try<Nothing> create = cgroups::create(hierarchies["cpu"], info->cgroup);
-    if (create.isError()) {
-      return Failure("Failed to prepare isolator: " + create.error());
-    }
+  Try<Nothing> create = cgroups::create(hierarchies["cpu"], info->cgroup);
+  if (create.isError()) {
+    return Failure("Failed to prepare isolator: " + create.error());
   }
 
   // Create a 'cpuacct' cgroup for this container.
@@ -246,17 +242,13 @@ Future<Option<CommandInfo> > CgroupsCpushareIsolatorProcess::prepare(
 
   if (exists.isError()) {
     return Failure("Failed to prepare isolator: " + exists.error());
-  }
-
-  if (exists.get()) {
+  } else if (exists.get()) {
     return Failure("Failed to prepare isolator: cgroup already exists");
   }
 
-  if (!exists.get()) {
-    Try<Nothing> create = cgroups::create(hierarchies["cpuacct"], info->cgroup);
-    if (create.isError()) {
-      return Failure("Failed to prepare isolator: " + create.error());
-    }
+  create = cgroups::create(hierarchies["cpuacct"], info->cgroup);
+  if (create.isError()) {
+    return Failure("Failed to prepare isolator: " + create.error());
   }
 
   return update(containerId, executorInfo.resources())

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d6b20e2/src/slave/containerizer/isolators/cgroups/cpushare.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/cgroups/cpushare.hpp b/src/slave/containerizer/isolators/cgroups/cpushare.hpp
index 36fa997..909ea88 100644
--- a/src/slave/containerizer/isolators/cgroups/cpushare.hpp
+++ b/src/slave/containerizer/isolators/cgroups/cpushare.hpp
@@ -94,6 +94,7 @@ private:
   // Map from subsystem to hierarchy.
   hashmap<std::string, std::string> hierarchies;
 
+  // TODO(bmahler): Use Owned<Info>.
   hashmap<ContainerID, Info*> infos;
 };
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d6b20e2/src/slave/containerizer/isolators/cgroups/mem.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/cgroups/mem.cpp b/src/slave/containerizer/isolators/cgroups/mem.cpp
index f2509ad..60013d4 100644
--- a/src/slave/containerizer/isolators/cgroups/mem.cpp
+++ b/src/slave/containerizer/isolators/cgroups/mem.cpp
@@ -119,14 +119,10 @@ Future<Nothing> CgroupsMemIsolatorProcess::recover(
     }
 
     const ContainerID& containerId = state.id.get();
+    const string cgroup = path::join(flags.cgroups_root, containerId.value());
 
-    Info* info = new Info(
-        containerId, path::join(flags.cgroups_root, containerId.value()));
-    CHECK_NOTNULL(info);
-
-    Try<bool> exists = cgroups::exists(hierarchy, info->cgroup);
+    Try<bool> exists = cgroups::exists(hierarchy, cgroup);
     if (exists.isError()) {
-      delete info;
       foreachvalue (Info* info, infos) {
         delete info;
       }
@@ -137,15 +133,15 @@ Future<Nothing> CgroupsMemIsolatorProcess::recover(
 
     if (!exists.get()) {
       VLOG(1) << "Couldn't find cgroup for container " << containerId;
-      // This may occur if the executor has exiting and the isolator has
+      // This may occur if the executor has exited and the isolator has
       // destroyed the cgroup but the slave dies before noticing this. This
       // will be detected when the containerizer tries to monitor the
       // executor's pid.
       continue;
     }
 
-    infos[containerId] = info;
-    cgroups.insert(info->cgroup);
+    infos[containerId] = new Info(containerId, cgroup);
+    cgroups.insert(cgroup);
 
     oomListen(containerId);
   }
@@ -186,27 +182,27 @@ Future<Option<CommandInfo> > CgroupsMemIsolatorProcess::prepare(
     return Failure("Container has already been prepared");
   }
 
+  // TODO(bmahler): Don't insert into 'infos' unless we create the
+  // cgroup successfully. It's safe for now because 'cleanup' gets
+  // called if we return a Failure, but cleanup will fail because
+  // the cgroup does not exist when cgroups::destroy is called.
   Info* info = new Info(
       containerId, path::join(flags.cgroups_root, containerId.value()));
 
-  infos[containerId] = CHECK_NOTNULL(info);
+  infos[containerId] = info;
 
   // Create a cgroup for this container.
   Try<bool> exists = cgroups::exists(hierarchy, info->cgroup);
 
   if (exists.isError()) {
     return Failure("Failed to prepare isolator: " + exists.error());
-  }
-
-  if (exists.get()) {
+  } else if (exists.get()) {
     return Failure("Failed to prepare isolator: cgroup already exists");
   }
 
-  if (!exists.get()) {
-    Try<Nothing> create = cgroups::create(hierarchy, info->cgroup);
-    if (create.isError()) {
-      return Failure("Failed to prepare isolator: " + create.error());
-    }
+  Try<Nothing> create = cgroups::create(hierarchy, info->cgroup);
+  if (create.isError()) {
+    return Failure("Failed to prepare isolator: " + create.error());
   }
 
   oomListen(containerId);

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d6b20e2/src/slave/containerizer/isolators/cgroups/mem.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/cgroups/mem.hpp b/src/slave/containerizer/isolators/cgroups/mem.hpp
index 6e79256..362ebcf 100644
--- a/src/slave/containerizer/isolators/cgroups/mem.hpp
+++ b/src/slave/containerizer/isolators/cgroups/mem.hpp
@@ -108,6 +108,7 @@ private:
   // The path to the cgroups subsystem hierarchy root.
   const std::string hierarchy;
 
+  // TODO(bmahler): Use Owned<Info>.
   hashmap<ContainerID, Info*> infos;
 };