You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gr...@apache.org on 2017/08/14 20:58:07 UTC

[1/5] mesos git commit: Stout: Made `Duration::parse()` handle durations out of range.

Repository: mesos
Updated Branches:
  refs/heads/master 628d6609b -> 2fe2bb26a


Stout: Made `Duration::parse()` handle durations out of range.

Made `Duration:parse()` return an error if the argument is out of the
range that a `Duration` can represent.

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


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

Branch: refs/heads/master
Commit: f4348182c1c5b832743166cfdad9b1a84bc2824e
Parents: 628d660
Author: Gastón Kleiman <ga...@mesosphere.io>
Authored: Mon Aug 14 13:52:49 2017 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Mon Aug 14 13:52:49 2017 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/duration.hpp | 27 ++++++++++++++++++--------
 3rdparty/stout/tests/duration_tests.cpp   |  7 ++++++-
 2 files changed, 25 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/f4348182/3rdparty/stout/include/stout/duration.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/duration.hpp b/3rdparty/stout/include/stout/duration.hpp
index b0cd77b..67ee370 100644
--- a/3rdparty/stout/include/stout/duration.hpp
+++ b/3rdparty/stout/include/stout/duration.hpp
@@ -50,28 +50,39 @@ public:
 
       const std::string unit = s.substr(index);
 
+      int64_t factor;
       if (unit == "ns") {
-        return Duration(value.get(), NANOSECONDS);
+        factor = NANOSECONDS;
       } else if (unit == "us") {
-        return Duration(value.get(), MICROSECONDS);
+        factor = MICROSECONDS;
       } else if (unit == "ms") {
-        return Duration(value.get(), MILLISECONDS);
+        factor = MILLISECONDS;
       } else if (unit == "secs") {
-        return Duration(value.get(), SECONDS);
+        factor = SECONDS;
       } else if (unit == "mins") {
-        return Duration(value.get(), MINUTES);
+        factor = MINUTES;
       } else if (unit == "hrs") {
-        return Duration(value.get(), HOURS);
+        factor = HOURS;
       } else if (unit == "days") {
-        return Duration(value.get(), DAYS);
+        factor = DAYS;
       } else if (unit == "weeks") {
-        return Duration(value.get(), WEEKS);
+        factor = WEEKS;
       } else {
         return Error(
             "Unknown duration unit '" + unit + "'; supported units are"
             " 'ns', 'us', 'ms', 'secs', 'mins', 'hrs', 'days', and 'weeks'");
       }
+
+      double nanos = value.get() * factor;
+      if (nanos > max().nanos || nanos < min().nanos) {
+        return Error(
+            "Argument out of the range that a Duration can represent due"
+            " to int64_t's size limit");
+      }
+
+      return Duration(value.get(), factor);
     }
+
     return Error("Invalid duration '" + s + "'");
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/f4348182/3rdparty/stout/tests/duration_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/duration_tests.cpp b/3rdparty/stout/tests/duration_tests.cpp
index 59b08f1..1587fb6 100644
--- a/3rdparty/stout/tests/duration_tests.cpp
+++ b/3rdparty/stout/tests/duration_tests.cpp
@@ -50,8 +50,13 @@ TEST(DurationTest, ParseAndTry)
   EXPECT_SOME_EQ(Hours(3), Duration::parse("3hrs"));
   EXPECT_SOME_EQ(Hours(3) + Minutes(30), Duration::parse("3.5hrs"));
 
+  EXPECT_ERROR(
+      Duration::parse(
+          stringify(std::numeric_limits<int64_t>::max()) + "seconds"));
+
   EXPECT_SOME_EQ(Nanoseconds(3141592653), Duration::create(3.141592653));
-  // Duration can hold only 9.22337e9 seconds.
+
+  // Duration can hold only +/-9.22337e9 seconds.
   EXPECT_ERROR(Duration::create(10 * 1e9));
   EXPECT_ERROR(Duration::create(-10 * 1e9));
 }


[4/5] mesos git commit: Fixed the default filter used by the allocator.

Posted by gr...@apache.org.
Fixed the default filter used by the allocator.

If a framework accepts/refuses an offer using a very long filter, the
`HierarchicalAllocator` will use the default filter instead, meaning
that it will filter the resources for only 5 seconds. This can happen
when a framework sets `Filter::refuse_seconds` to a number of seconds
larger than what fits in `Duration`.

This patch makes the hierarchical allocator cap the filter duration to
at most 365 days.

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


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

Branch: refs/heads/master
Commit: 183cceef366586f4a55b6ba7144c4a8277eb9962
Parents: 8254fd3
Author: Gastón Kleiman <ga...@mesosphere.io>
Authored: Mon Aug 14 13:52:52 2017 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Mon Aug 14 13:56:34 2017 -0700

----------------------------------------------------------------------
 docs/scheduler-http-api.md                  |  6 ++-
 include/mesos/mesos.proto                   |  5 ++
 include/mesos/v1/mesos.proto                |  5 ++
 src/master/allocator/mesos/hierarchical.cpp | 68 +++++++++++++++---------
 4 files changed, 58 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/183cceef/docs/scheduler-http-api.md
----------------------------------------------------------------------
diff --git a/docs/scheduler-http-api.md b/docs/scheduler-http-api.md
index 4a5d77b..bd0e8c2 100644
--- a/docs/scheduler-http-api.md
+++ b/docs/scheduler-http-api.md
@@ -165,7 +165,11 @@ HTTP/1.1 202 Accepted
 ```
 
 ### ACCEPT
-Sent by the scheduler when it accepts offer(s) sent by the master. The `ACCEPT` request includes the type of operations (e.g., launch task, launch task group, reserve resources, create volumes) that the scheduler wants to perform on the offers. Note that until the scheduler replies (accepts or declines) to an offer, the offer's resources are considered allocated to the offer's role and to the framework. Also, any of the offer's resources not used in the `ACCEPT` call (e.g., to launch a task or task group) are considered declined and might be reoffered to other frameworks. In other words, the same `OfferID` cannot be used in more than one `ACCEPT` call. These semantics might change when we add new features to Mesos (e.g., persistence, reservations, optimistic offers, resizeTask, etc.).
+Sent by the scheduler when it accepts offer(s) sent by the master. The `ACCEPT` request includes the type of operations (e.g., launch task, launch task group, reserve resources, create volumes) that the scheduler wants to perform on the offers. Note that until the scheduler replies (accepts or declines) to an offer, the offer's resources are considered allocated to the offer's role and to the framework. Also, any of the offer's resources not used in the `ACCEPT` call (e.g., to launch a task or task group) are considered declined and might be reoffered to other frameworks, meaning that they will not be reoffered to the scheduler for the amount of time defined by the filter. The same `OfferID` cannot be used in more than one `ACCEPT` call. These semantics might change when we add new features to Mesos (e.g., persistence, reservations, optimistic offers, resizeTask, etc.).
+
+The scheduler API uses `Filters.refuse_seconds` to specify the duration for which resources are considered declined. If `filters` is not set, then the default value defined in [mesos.proto](https://github.com/apache/mesos/blob/master/include/mesos/v1/mesos.proto) will be used.
+
+NOTE: Mesos will cap `Filters.refuse_seconds` at 31536000 seconds (365 days).
 
 ```
 ACCEPT Request (JSON):

http://git-wip-us.apache.org/repos/asf/mesos/blob/183cceef/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 50a5caf..e39926e 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -2254,6 +2254,11 @@ message Filters {
   // SchedulerDriver::launchTasks. You MUST pass Filters with this
   // field set to change this behavior (i.e., get another offer which
   // includes unused resources sooner or later than the default).
+  //
+  // If this field is set to a number of seconds greater than 31536000
+  // (365 days), then the resources will be considered refused for 365
+  // days. If it is set to a negative number, then the default value
+  // will be used.
   optional double refuse_seconds = 1 [default = 5.0];
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/183cceef/include/mesos/v1/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index 9d47a8a..2979f9e 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -2237,6 +2237,11 @@ message Filters {
   // SchedulerDriver::launchTasks. You MUST pass Filters with this
   // field set to change this behavior (i.e., get another offer which
   // includes unused resources sooner or later than the default).
+  //
+  // If this field is set to a number of seconds greater than 31536000
+  // (365 days), then the resources will be considered refused for 365
+  // days. If it is set to a negative number, then the default value
+  // will be used.
   optional double refuse_seconds = 1 [default = 5.0];
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/183cceef/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index e5372ba..2213130 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1015,33 +1015,42 @@ void HierarchicalAllocatorProcess::updateInverseOffer(
     return;
   }
 
-  // Create a refused resource filter.
-  Try<Duration> seconds = Duration::create(filters.get().refuse_seconds());
+  // Create a refused inverse offer filter.
+  Try<Duration> timeout = Duration::create(Filters().refuse_seconds());
 
-  if (seconds.isError()) {
-    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
-                 << " the refused inverse offer filter because the input value"
-                 << " is invalid: " << seconds.error();
+  if (filters->refuse_seconds() > Days(365).secs()) {
+    LOG(WARNING) << "Using 365 days to create the refused inverse offer"
+                 << " filter because the input value is too big";
 
-    seconds = Duration::create(Filters().refuse_seconds());
-  } else if (seconds.get() < Duration::zero()) {
+    timeout = Days(365);
+  } else if (filters->refuse_seconds() < 0) {
     LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
-                 << " the refused inverse offer filter because the input value"
-                 << " is negative";
+                 << " the refused inverse offer filter because the input"
+                 << " value is negative";
+
+    timeout = Duration::create(Filters().refuse_seconds());
+  } else {
+    timeout = Duration::create(filters->refuse_seconds());
 
-    seconds = Duration::create(Filters().refuse_seconds());
+    if (timeout.isError()) {
+      LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
+                   << " the refused inverse offer filter because the input"
+                   << " value is invalid: " + timeout.error();
+
+      timeout = Duration::create(Filters().refuse_seconds());
+    }
   }
 
-  CHECK_SOME(seconds);
+  CHECK_SOME(timeout);
 
-  if (seconds.get() != Duration::zero()) {
+  if (timeout.get() != Duration::zero()) {
     VLOG(1) << "Framework " << frameworkId
             << " filtered inverse offers from agent " << slaveId
-            << " for " << seconds.get();
+            << " for " << timeout.get();
 
     // Create a new inverse offer filter and delay its expiration.
     InverseOfferFilter* inverseOfferFilter =
-      new RefusedInverseOfferFilter(Timeout::in(seconds.get()));
+      new RefusedInverseOfferFilter(Timeout::in(timeout.get()));
 
     framework.inverseOfferFilters[slaveId].insert(inverseOfferFilter);
 
@@ -1053,7 +1062,7 @@ void HierarchicalAllocatorProcess::updateInverseOffer(
              InverseOfferFilter*) = &Self::expire;
 
     delay(
-        seconds.get(),
+        timeout.get(),
         self(),
         expireInverseOffer,
         frameworkId,
@@ -1167,20 +1176,29 @@ void HierarchicalAllocatorProcess::recoverResources(
   }
 
   // Create a refused resources filter.
-  Try<Duration> timeout = Duration::create(filters.get().refuse_seconds());
+  Try<Duration> timeout = Duration::create(Filters().refuse_seconds());
 
-  if (timeout.isError()) {
-    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
-                 << " the refused resources filter because the input value"
-                 << " is invalid: " << timeout.error();
+  if (filters->refuse_seconds() > Days(365).secs()) {
+    LOG(WARNING) << "Using 365 days to create the refused resources offer"
+                 << " filter because the input value is too big";
 
-    timeout = Duration::create(Filters().refuse_seconds());
-  } else if (timeout.get() < Duration::zero()) {
+    timeout = Days(365);
+  } else if (filters->refuse_seconds() < 0) {
     LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
-                 << " the refused resources filter because the input value"
-                 << " is negative";
+                 << " the refused resources offer filter because the input"
+                 << " value is negative";
 
     timeout = Duration::create(Filters().refuse_seconds());
+  } else {
+    timeout = Duration::create(filters->refuse_seconds());
+
+    if (timeout.isError()) {
+      LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
+                   << " the refused resources offer filter because the input"
+                   << " value is invalid: " + timeout.error();
+
+      timeout = Duration::create(Filters().refuse_seconds());
+    }
   }
 
   CHECK_SOME(timeout);


[5/5] mesos git commit: Added MESOS-7660 to the changelog.

Posted by gr...@apache.org.
Added MESOS-7660 to the changelog.

This patch adds MESOS-7660 to the changelog and adds a missing period
to the existing text.

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


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

Branch: refs/heads/master
Commit: 2fe2bb26a425da9aaf1d7cf34019dd347d0cf9a4
Parents: 183ccee
Author: Gastón Kleiman <ga...@mesosphere.io>
Authored: Mon Aug 14 13:52:54 2017 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Mon Aug 14 13:57:15 2017 -0700

----------------------------------------------------------------------
 CHANGELOG | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2fe2bb26/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index abb0e3b..a408c25 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -32,7 +32,7 @@ Deprecations/Removals:
     of LinuxInfo.effective_capabilities.
 
   * [MESOS-7477] - The agent `--allowed_capabilities` flag is
-    deprecated in favor of `--effective_capabilities`
+    deprecated in favor of `--effective_capabilities`.
 
 Additional API Changes:
 
@@ -42,6 +42,8 @@ Additional API Changes:
     resources (both revocable and non-revocable) on the agent. Custom
     allocator implementation should be changed to interpretation of the
     passed value as a total before updating.
+  * [MESOS-7660] `Filter::refuse_seconds` is now capped to 31536000
+    seconds (365 days).
 
 Feature Graduations:
   * [MESOS-2533] - Support HTTP checks in Mesos.


[2/5] mesos git commit: Stout: Made boundary checking in Duration consistent.

Posted by gr...@apache.org.
Stout: Made boundary checking in Duration consistent.

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


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

Branch: refs/heads/master
Commit: 1efe264ebcf998c248cb7eecba57bd65e2060645
Parents: f434818
Author: Gastón Kleiman <ga...@mesosphere.io>
Authored: Mon Aug 14 13:52:50 2017 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Mon Aug 14 13:54:02 2017 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/duration.hpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1efe264e/3rdparty/stout/include/stout/duration.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/duration.hpp b/3rdparty/stout/include/stout/duration.hpp
index 67ee370..d10b6a4 100644
--- a/3rdparty/stout/include/stout/duration.hpp
+++ b/3rdparty/stout/include/stout/duration.hpp
@@ -410,8 +410,7 @@ inline std::ostream& operator<<(std::ostream& stream, const Duration& duration_)
 
 inline Try<Duration> Duration::create(double seconds)
 {
-  if (seconds * SECONDS > std::numeric_limits<int64_t>::max() ||
-      seconds * SECONDS < std::numeric_limits<int64_t>::min()) {
+  if (seconds * SECONDS > max().nanos || seconds * SECONDS < min().nanos) {
     return Error("Argument out of the range that a Duration can represent due "
                  "to int64_t's size limit");
   }


[3/5] mesos git commit: Prefer leading instead of trailing spaces for allocator log messages.

Posted by gr...@apache.org.
Prefer leading instead of trailing spaces for allocator log messages.

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


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

Branch: refs/heads/master
Commit: 8254fd36b668efdbbbd11239e5a16a28e05e65a3
Parents: 1efe264
Author: Gastón Kleiman <ga...@mesosphere.io>
Authored: Mon Aug 14 13:52:51 2017 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Mon Aug 14 13:54:29 2017 -0700

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp | 39 ++++++++++++------------
 1 file changed, 20 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/8254fd36/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index f021c34..e5372ba 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -206,8 +206,8 @@ void HierarchicalAllocatorProcess::recover(
   // exacerbate the issue.
 
   if (quotas.empty()) {
-    VLOG(1) << "Skipping recovery of hierarchical allocator: "
-            << "nothing to recover";
+    VLOG(1) << "Skipping recovery of hierarchical allocator:"
+            << " nothing to recover";
 
     return;
   }
@@ -230,8 +230,8 @@ void HierarchicalAllocatorProcess::recover(
   // to expected behavior by the user: the allocator is not paused until
   // a new agent is added.
   if (expectedAgentCount.get() == 0) {
-    VLOG(1) << "Skipping recovery of hierarchical allocator: "
-            << "no reconnecting agents to wait for";
+    VLOG(1) << "Skipping recovery of hierarchical allocator:"
+            << " no reconnecting agents to wait for";
 
     return;
   }
@@ -1019,15 +1019,15 @@ void HierarchicalAllocatorProcess::updateInverseOffer(
   Try<Duration> seconds = Duration::create(filters.get().refuse_seconds());
 
   if (seconds.isError()) {
-    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create "
-                 << "the refused inverse offer filter because the input value "
-                 << "is invalid: " << seconds.error();
+    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
+                 << " the refused inverse offer filter because the input value"
+                 << " is invalid: " << seconds.error();
 
     seconds = Duration::create(Filters().refuse_seconds());
   } else if (seconds.get() < Duration::zero()) {
-    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create "
-                 << "the refused inverse offer filter because the input value "
-                 << "is negative";
+    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
+                 << " the refused inverse offer filter because the input value"
+                 << " is negative";
 
     seconds = Duration::create(Filters().refuse_seconds());
   }
@@ -1170,15 +1170,15 @@ void HierarchicalAllocatorProcess::recoverResources(
   Try<Duration> timeout = Duration::create(filters.get().refuse_seconds());
 
   if (timeout.isError()) {
-    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create "
-                 << "the refused resources filter because the input value "
-                 << "is invalid: " << timeout.error();
+    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
+                 << " the refused resources filter because the input value"
+                 << " is invalid: " << timeout.error();
 
     timeout = Duration::create(Filters().refuse_seconds());
   } else if (timeout.get() < Duration::zero()) {
-    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create "
-                 << "the refused resources filter because the input value "
-                 << "is negative";
+    LOG(WARNING) << "Using the default value of 'refuse_seconds' to create"
+                 << " the refused resources filter because the input value"
+                 << " is negative";
 
     timeout = Duration::create(Filters().refuse_seconds());
   }
@@ -2149,9 +2149,10 @@ bool HierarchicalAllocatorProcess::isFiltered(
   // Prevent offers from non-HIERARCHICAL_ROLE agents to be allocated
   // to hierarchical roles.
   if (!slave.capabilities.hierarchicalRole && strings::contains(role, "/")) {
-    LOG(WARNING) << "Implicitly filtering agent " << slaveId << " from role "
-                 << role << " because the role is hierarchical but the agent"
-                 << " is not HIERARCHICAL_ROLE capable";
+    LOG(WARNING) << "Implicitly filtering agent " << slaveId
+                 << " from role " << role
+                 << " because the role is hierarchical but the agent is not"
+                 << " HIERARCHICAL_ROLE capable";
 
     return true;
   }