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/05/06 22:57:10 UTC

[1/3] git commit: Fixed garbage collection to allow for disk head room.

Updated Branches:
  refs/heads/master 1647e682d -> 2e38010fb


Fixed garbage collection to allow for disk head room.

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


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

Branch: refs/heads/master
Commit: 2e38010fb58a0399784d1ddc74e1ffba1300997a
Parents: 37c89b6
Author: Vinod Kone <vi...@twitter.com>
Authored: Fri May 3 18:01:51 2013 -0700
Committer: Vinod Kone <vi...@twitter.com>
Committed: Mon May 6 13:55:44 2013 -0700

----------------------------------------------------------------------
 src/slave/constants.cpp |    1 +
 src/slave/constants.hpp |    3 +++
 src/slave/slave.cpp     |    3 ++-
 src/tests/gc_tests.cpp  |    5 ++++-
 4 files changed, 10 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/2e38010f/src/slave/constants.cpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.cpp b/src/slave/constants.cpp
index 91f6f91..5a81f1f 100644
--- a/src/slave/constants.cpp
+++ b/src/slave/constants.cpp
@@ -27,6 +27,7 @@ const Duration EXECUTOR_SHUTDOWN_GRACE_PERIOD = Seconds(5.0);
 const Duration EXECUTOR_REREGISTER_TIMEOUT = Seconds(2.0);
 const Duration STATUS_UPDATE_RETRY_INTERVAL = Seconds(10.0);
 const Duration GC_DELAY = Weeks(1.0);
+const double GC_DISK_HEADROOM = 0.1;
 const Duration DISK_WATCH_INTERVAL = Minutes(1.0);
 const Duration RESOURCE_MONITORING_INTERVAL = Seconds(5.0);
 const uint32_t MAX_COMPLETED_FRAMEWORKS = 50;

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/2e38010f/src/slave/constants.hpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.hpp b/src/slave/constants.hpp
index 24a8dd2..901fdf2 100644
--- a/src/slave/constants.hpp
+++ b/src/slave/constants.hpp
@@ -42,6 +42,9 @@ extern const Duration GC_DELAY;
 extern const Duration DISK_WATCH_INTERVAL;
 extern const Duration RESOURCE_MONITORING_INTERVAL;
 
+// Minimum free disk capacity enforced by the garbage collector.
+extern const double GC_DISK_HEADROOM;
+
 // Maximum number of completed frameworks to store in memory.
 extern const uint32_t MAX_COMPLETED_FRAMEWORKS;
 

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/2e38010f/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 790cf75..d3126cc 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -2456,7 +2456,8 @@ void Slave::registerExecutorTimeout(
 // TODO(vinod): Figure out a way to express this function via cmd line.
 Duration Slave::age(double usage)
 {
- return Weeks(flags.gc_delay.weeks() * (1.0 - usage));
+ return Weeks(
+     flags.gc_delay.weeks() * std::max(0.0, (1.0 - GC_DISK_HEADROOM - usage)));
 }
 
 

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/2e38010f/src/tests/gc_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/gc_tests.cpp b/src/tests/gc_tests.cpp
index 9cd3dfb..97d2685 100644
--- a/src/tests/gc_tests.cpp
+++ b/src/tests/gc_tests.cpp
@@ -620,7 +620,10 @@ TEST_F(GarbageCollectorIntegrationTest, DiskUsage)
     FUTURE_DISPATCH(_, &Slave::_checkDiskUsage);
 
   // Simulate a disk full message to the slave.
-  process::dispatch(slave.get(), &Slave::_checkDiskUsage, Try<double>::some(1));
+  process::dispatch(
+      slave.get(),
+      &Slave::_checkDiskUsage,
+      Try<double>::some(1.0 - slave::GC_DISK_HEADROOM));
 
   AWAIT_READY(_checkDiskUsage);
 


[2/3] git commit: Fixed slave to use Bytes version of memory and disk.

Posted by vi...@apache.org.
Fixed slave to use Bytes version of memory and disk.

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


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

Branch: refs/heads/master
Commit: 37c89b6fefed95b1eff2b8b7bef4cad0ee0b7d2b
Parents: dc4c10a
Author: Vinod Kone <vi...@twitter.com>
Authored: Fri Apr 26 19:49:18 2013 -0700
Committer: Vinod Kone <vi...@twitter.com>
Committed: Mon May 6 13:55:44 2013 -0700

----------------------------------------------------------------------
 src/common/resources.hpp      |   21 ++++++++++---------
 src/slave/constants.cpp       |    4 +-
 src/slave/constants.hpp       |    5 ++-
 src/slave/slave.cpp           |   37 +++++++++++++++++++----------------
 src/slave/slave.hpp           |    3 +-
 src/tests/resources_tests.cpp |   13 ++++++-----
 6 files changed, 45 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/37c89b6f/src/common/resources.hpp
----------------------------------------------------------------------
diff --git a/src/common/resources.hpp b/src/common/resources.hpp
index e15c2d2..42dfb6a 100644
--- a/src/common/resources.hpp
+++ b/src/common/resources.hpp
@@ -26,6 +26,7 @@
 
 #include <mesos/mesos.hpp>
 
+#include <stout/bytes.hpp>
 #include <stout/foreach.hpp>
 #include <stout/none.hpp>
 #include <stout/option.hpp>
@@ -277,8 +278,8 @@ public:
   // Helpers to get known resource types.
   // TODO(vinod): Fix this when we make these types as first class protobufs.
   Option<double> cpus();
-  Option<double> mem();
-  Option<double> disk();
+  Option<Bytes> mem();
+  Option<Bytes> disk();
   Option<Value::Ranges> ports(); // TODO(vinod): Provide a Ranges abstraction.
 
   typedef google::protobuf::RepeatedPtrField<Resource>::iterator
@@ -430,31 +431,31 @@ inline Option<double> Resources::cpus()
       return resource.scalar().value();
     }
   }
-  return Option<double>::none();
+  return None();
 }
 
 
-inline Option<double> Resources::mem()
+inline Option<Bytes> Resources::mem()
 {
   foreach (const Resource& resource, resources) {
     if (resource.name() == "mem" &&
         resource.type() == Value::SCALAR) {
-      return resource.scalar().value();
+      return Megabytes(static_cast<uint64_t>(resource.scalar().value()));
     }
   }
-  return Option<double>::none();
+  return None();
 }
 
 
-inline Option<double> Resources::disk()
+inline Option<Bytes> Resources::disk()
 {
   foreach (const Resource& resource, resources) {
     if (resource.name() == "disk" &&
         resource.type() == Value::SCALAR) {
-      return resource.scalar().value();
+      return Megabytes(static_cast<uint64_t>(resource.scalar().value()));
     }
   }
-  return Option<double>::none();
+  return None();
 }
 
 
@@ -466,7 +467,7 @@ inline Option<Value::Ranges> Resources::ports()
       return resource.ranges();
     }
   }
-  return Option<Value::Ranges>::none();
+  return None();
 }
 
 

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/37c89b6f/src/slave/constants.cpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.cpp b/src/slave/constants.cpp
index 6cd97f9..91f6f91 100644
--- a/src/slave/constants.cpp
+++ b/src/slave/constants.cpp
@@ -33,8 +33,8 @@ const uint32_t MAX_COMPLETED_FRAMEWORKS = 50;
 const uint32_t MAX_COMPLETED_EXECUTORS_PER_FRAMEWORK = 150;
 const uint32_t MAX_COMPLETED_TASKS_PER_EXECUTOR = 200;
 const double DEFAULT_CPUS = 1;
-const double DEFAULT_MEM = 1024; // In MB.
-const double DEFAULT_DISK = 10240; // In MB.
+const Bytes DEFAULT_MEM = Gigabytes(1);
+const Bytes DEFAULT_DISK = Gigabytes(10);
 const std::string DEFAULT_PORTS = "[31000-32000]";
 
 } // namespace slave {

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/37c89b6f/src/slave/constants.hpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.hpp b/src/slave/constants.hpp
index ca1124a..24a8dd2 100644
--- a/src/slave/constants.hpp
+++ b/src/slave/constants.hpp
@@ -21,6 +21,7 @@
 
 #include <stdint.h>
 
+#include <stout/bytes.hpp>
 #include <stout/duration.hpp>
 
 namespace mesos {
@@ -54,10 +55,10 @@ extern const uint32_t MAX_COMPLETED_TASKS_PER_EXECUTOR;
 extern const double DEFAULT_CPUS;
 
 // Default memory offered by the slave.
-extern const double DEFAULT_MEM; // In MB.
+extern const Bytes DEFAULT_MEM;
 
 // Default disk space offered by the slave.
-extern const double DEFAULT_DISK; // In MB.
+extern const Bytes DEFAULT_DISK;
 
 // Default ports range offered by the slave.
 extern const std::string DEFAULT_PORTS;

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/37c89b6f/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 86a15fc..790cf75 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -31,6 +31,7 @@
 #include <process/dispatch.hpp>
 #include <process/id.hpp>
 
+#include <stout/bytes.hpp>
 #include <stout/duration.hpp>
 #include <stout/exit.hpp>
 #include <stout/fs.hpp>
@@ -125,48 +126,46 @@ Slave::Slave(const slave::Flags& _flags,
     }
   }
 
-  double mem; // in MB.
+  Bytes mem;
   if (resources.mem().isSome()) {
     mem = resources.mem().get();
   } else {
-    Try<uint64_t> mem_ = os::memory(); // in bytes.
+    Try<Bytes> mem_ = os::memory();
     if (!mem_.isSome()) {
       LOG(WARNING) << "Failed to auto-detect the size of main memory,"
-                   << " defaulting to " << DEFAULT_MEM << " MB";
+                   << " defaulting to " << DEFAULT_MEM;
       mem = DEFAULT_MEM;
     } else {
-      // Convert to MB.
-      mem = mem_.get() / 1048576;
+      mem = mem_.get();
 
       // Leave 1 GB free if we have more than 1 GB, otherwise, use all!
       // TODO(benh): Have better default scheme (e.g., % of mem not
       // greater than 1 GB?)
-      if (mem > 1024) {
-        mem = mem - 1024;
+      if (mem > Gigabytes(1)) {
+        mem = mem - Gigabytes(1);
       }
     }
   }
 
-  double disk; // in MB.
+  Bytes disk;
   if (resources.disk().isSome()) {
     disk = resources.disk().get();
   } else {
     // NOTE: We calculate disk availability of the file system on
     // which the slave work directory is mounted.
-    Try<uint64_t> disk_ = fs::available(flags.work_dir); // in bytes.
+    Try<Bytes> disk_ = fs::available(flags.work_dir);
     if (!disk_.isSome()) {
       LOG(WARNING) << "Failed to auto-detect the free disk space,"
-                   << " defaulting to " << DEFAULT_DISK  << " MB";
+                   << " defaulting to " << DEFAULT_DISK;
       disk = DEFAULT_DISK;
     } else {
-      // Convert to MB.
-      disk = disk_.get() / 1048576;
+      disk = disk_.get();
 
       // Leave 5 GB free if we have more than 10 GB, otherwise, use all!
       // TODO(benh): Have better default scheme (e.g., % of disk not
       // greater than 10 GB?)
-      if (disk > 1024 * 10) {
-        disk = disk - (1024 * 5);
+      if (disk > Gigabytes(10)) {
+        disk = disk - Gigabytes(5);
       }
     }
   }
@@ -180,7 +179,11 @@ Slave::Slave(const slave::Flags& _flags,
   }
 
   Try<string> defaults = strings::format(
-      "cpus:%f;mem:%f;ports:%s;disk:%f", cpus, mem, ports, disk);
+      "cpus:%f;mem:%lu;ports:%s;disk:%lu",
+      cpus,
+      mem.megabytes(),
+      ports,
+      disk.megabytes());
 
   CHECK_SOME(defaults);
 
@@ -2474,12 +2477,12 @@ void Slave::_checkDiskUsage(const Future<Try<double> >& usage)
     LOG(ERROR) << "Failed to get disk usage: "
                << (usage.isFailed() ? usage.failure() : "future discarded");
   } else {
-    Try<double> result = usage.get();
+    const Try<double>& result = usage.get();
 
     if (result.isSome()) {
       double use = result.get();
 
-      LOG(INFO) << "Current disk usage " << std::setiosflags(std::ios::fixed)
+      LOG(INFO) << "Current usage " << std::setiosflags(std::ios::fixed)
                 << std::setprecision(2) << 100 * use << "%."
                 << " Max allowed age: " << age(use);
 

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/37c89b6f/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index f9d9c2b..82d2804 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -31,6 +31,7 @@
 #include <process/process.hpp>
 #include <process/protobuf.hpp>
 
+#include <stout/bytes.hpp>
 #include <stout/hashmap.hpp>
 #include <stout/hashset.hpp>
 #include <stout/multihashmap.hpp>
@@ -181,7 +182,7 @@ public:
   // TODO(vinod): Instead of making this function public, we need to
   // mock both GarbageCollector (and pass it through slave's constructor)
   // and os calls.
-  void _checkDiskUsage(const Future<Try<double> >& capacity);
+  void _checkDiskUsage(const Future<Try<double> >& usage);
 
   // Shut down an executor. This is a two phase process. First, an
   // executor receives a shut down message (shut down phase), then

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/37c89b6f/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 684379f..67a11b3 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -21,6 +21,7 @@
 
 #include <gtest/gtest.h>
 
+#include <stout/bytes.hpp>
 #include <stout/gtest.hpp>
 
 #include "master/master.hpp"
@@ -65,13 +66,13 @@ TEST(ResourcesTest, Parsing)
 TEST(ResourcesTest, Resources)
 {
   Resources r = Resources::parse("cpus:45.55;"
-                                 "mem:1024.0;"
+                                 "mem:1024;"
                                  "ports:[10000-20000, 30000-50000];"
-                                 "disk:512.4");
+                                 "disk:512");
 
   EXPECT_SOME_EQ(45.55, r.cpus());
-  EXPECT_SOME_EQ(1024.0, r.mem());
-  EXPECT_SOME_EQ(512.4, r.disk());
+  EXPECT_SOME_EQ(Megabytes(1024), r.mem());
+  EXPECT_SOME_EQ(Megabytes(512), r.disk());
 
   EXPECT_SOME(r.ports());
   ostringstream ports;
@@ -79,9 +80,9 @@ TEST(ResourcesTest, Resources)
 
   EXPECT_EQ("[10000-20000, 30000-50000]", ports.str());
 
-  r = Resources::parse("cpus:45.55;disk:512.4");
+  r = Resources::parse("cpus:45.55;disk:512");
   EXPECT_SOME_EQ(45.55, r.cpus());
-  EXPECT_SOME_EQ(512.4, r.disk());
+  EXPECT_SOME_EQ(Megabytes(512), r.disk());
   EXPECT_TRUE(r.mem().isNone());
   EXPECT_TRUE(r.ports().isNone());
 }


[3/3] git commit: Fixed os::memory() and fs::available() to return Bytes.

Posted by vi...@apache.org.
Fixed os::memory() and fs::available() to return Bytes.

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


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

Branch: refs/heads/master
Commit: dc4c10a0b97f552c7b8d43f88c4105087f6e5680
Parents: 1647e68
Author: Vinod Kone <vi...@twitter.com>
Authored: Fri Apr 26 19:46:09 2013 -0700
Committer: Vinod Kone <vi...@twitter.com>
Committed: Mon May 6 13:55:44 2013 -0700

----------------------------------------------------------------------
 .../third_party/stout/include/stout/bytes.hpp      |   28 ++++++++++++++-
 .../third_party/stout/include/stout/fs.hpp         |    5 ++-
 .../third_party/stout/include/stout/os.hpp         |   11 +++---
 3 files changed, 36 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/dc4c10a0/third_party/libprocess/third_party/stout/include/stout/bytes.hpp
----------------------------------------------------------------------
diff --git a/third_party/libprocess/third_party/stout/include/stout/bytes.hpp b/third_party/libprocess/third_party/stout/include/stout/bytes.hpp
index 6a068f0..754fbb2 100644
--- a/third_party/libprocess/third_party/stout/include/stout/bytes.hpp
+++ b/third_party/libprocess/third_party/stout/include/stout/bytes.hpp
@@ -71,7 +71,17 @@ public:
   bool operator == (const Bytes& that) const { return value == that.value; }
   bool operator != (const Bytes& that) const { return value != that.value; }
 
-  // TODO(bmahler): Overload arithmetic operators..
+  Bytes& operator += (const Bytes& that)
+  {
+    value += that.value;
+    return *this;
+  }
+
+  Bytes& operator -= (const Bytes& that)
+  {
+    value -= that.value;
+    return *this;
+  }
 
 protected:
   static const uint64_t BYTES = 1;
@@ -131,4 +141,20 @@ inline std::ostream& operator << (std::ostream& stream, const Bytes& bytes)
   }
 }
 
+
+inline Bytes operator + (const Bytes& lhs, const Bytes& rhs)
+{
+  Bytes sum = lhs;
+  sum += rhs;
+  return sum;
+}
+
+
+inline Bytes operator - (const Bytes& lhs, const Bytes& rhs)
+{
+  Bytes diff = lhs;
+  diff -= rhs;
+  return diff;
+}
+
 #endif // __STOUT_BYTES_HPP__

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/dc4c10a0/third_party/libprocess/third_party/stout/include/stout/fs.hpp
----------------------------------------------------------------------
diff --git a/third_party/libprocess/third_party/stout/include/stout/fs.hpp b/third_party/libprocess/third_party/stout/include/stout/fs.hpp
index ef9fb18..c1a05b5 100644
--- a/third_party/libprocess/third_party/stout/include/stout/fs.hpp
+++ b/third_party/libprocess/third_party/stout/include/stout/fs.hpp
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "bytes.hpp"
 #include "error.hpp"
 #include "nothing.hpp"
 #include "try.hpp"
@@ -15,13 +16,13 @@
 namespace fs {
 
 // Returns the total available disk size in bytes.
-inline Try<uint64_t> available(const std::string& path = "/")
+inline Try<Bytes> available(const std::string& path = "/")
 {
   struct statvfs buf;
   if (::statvfs(path.c_str(), &buf) < 0) {
     return ErrnoError();
   }
-  return buf.f_bavail * buf.f_frsize;
+  return Bytes(buf.f_bavail * buf.f_frsize);
 }
 
 

http://git-wip-us.apache.org/repos/asf/incubator-mesos/blob/dc4c10a0/third_party/libprocess/third_party/stout/include/stout/os.hpp
----------------------------------------------------------------------
diff --git a/third_party/libprocess/third_party/stout/include/stout/os.hpp b/third_party/libprocess/third_party/stout/include/stout/os.hpp
index c71ae9e..29f6fbd 100644
--- a/third_party/libprocess/third_party/stout/include/stout/os.hpp
+++ b/third_party/libprocess/third_party/stout/include/stout/os.hpp
@@ -38,6 +38,7 @@
 #include <sstream>
 #include <string>
 
+#include "bytes.hpp"
 #include "duration.hpp"
 #include "error.hpp"
 #include "foreach.hpp"
@@ -933,8 +934,8 @@ inline Try<long> cpus()
 }
 
 
-// Returns the total size of main memory in bytes.
-inline Try<uint64_t> memory()
+// Returns the total size of main memory.
+inline Try<Bytes> memory()
 {
 #ifdef __linux__
   struct sysinfo info;
@@ -942,9 +943,9 @@ inline Try<uint64_t> memory()
     return ErrnoError();
   }
 # if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 3, 23)
-  return info.totalram * info.mem_unit;
+  return Bytes(info.totalram * info.mem_unit);
 # else
-  return info.totalram;
+  return Bytes(info.totalram);
 # endif
 #elif defined __APPLE__
   const size_t NAME_SIZE = 2;
@@ -958,7 +959,7 @@ inline Try<uint64_t> memory()
   if (sysctl(name, NAME_SIZE, &memory, &length, NULL, 0) < 0) {
     return ErrnoError("Failed to get sysctl of HW_MEMSIZE");
   }
-  return memory;
+  return Bytes(memory);
 #else
   return Error("Cannot determine the size of main memory");
 #endif