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/08/14 20:07:34 UTC
[11/18] git commit: Improved task validation error messages in master.
Improved task validation error messages in master.
Review: https://reviews.apache.org/r/13454
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/930aca13
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/930aca13
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/930aca13
Branch: refs/heads/master
Commit: 930aca1367afa0ad139300b1f8f8ee2e7d871f00
Parents: 376cd66
Author: Vinod Kone <vi...@twitter.com>
Authored: Fri Aug 9 16:18:05 2013 -0700
Committer: Vinod Kone <vi...@twitter.com>
Committed: Tue Aug 13 14:42:32 2013 -0700
----------------------------------------------------------------------
src/common/type_utils.hpp | 8 +++++++
src/master/master.cpp | 36 ++++++++++++++++++--------------
src/slave/slave.cpp | 3 +++
src/tests/resource_offers_tests.cpp | 10 +++++----
4 files changed, 37 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/930aca13/src/common/type_utils.hpp
----------------------------------------------------------------------
diff --git a/src/common/type_utils.hpp b/src/common/type_utils.hpp
index 9320ced..674a882 100644
--- a/src/common/type_utils.hpp
+++ b/src/common/type_utils.hpp
@@ -89,6 +89,14 @@ inline std::ostream& operator << (std::ostream& stream, const SlaveInfo& slave)
}
+inline std::ostream& operator << (
+ std::ostream& stream,
+ const ExecutorInfo& executor)
+{
+ return stream << executor.DebugString();
+}
+
+
inline bool operator == (const FrameworkID& left, const FrameworkID& right)
{
return left.value() == right.value();
http://git-wip-us.apache.org/repos/asf/mesos/blob/930aca13/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 6530008..2b60e32 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -1515,12 +1515,11 @@ struct ResourceUsageChecker : TaskInfoVisitor
Resources taskResources = task.resources();
if (!((usedResources + taskResources) <= offer->resources())) {
- LOG(WARNING) << "Task " << task.task_id() << " attempted to use "
- << taskResources << " combined with already used "
- << usedResources << " is greater than offered "
- << offer->resources();
-
- return TaskInfoError::some("Task uses more resources than offered");
+ return TaskInfoError::some(
+ "Task " + stringify(task.task_id()) + " attempted to use " +
+ stringify(taskResources) + " combined with already used " +
+ stringify(usedResources) + " is greater than offered " +
+ stringify(offer->resources()));
}
// Check this task's executor's resources.
@@ -1530,9 +1529,9 @@ struct ResourceUsageChecker : TaskInfoVisitor
foreach (const Resource& resource, task.executor().resources()) {
if (!Resources::isAllocatable(resource)) {
// TODO(benh): Send back the invalid resources?
- LOG(WARNING) << "Executor for task " << task.task_id()
- << " uses invalid resources " << resource;
- return TaskInfoError::some("Task's executor uses invalid resources");
+ return TaskInfoError::some(
+ "Executor for task " + stringify(task.task_id()) +
+ " uses invalid resources " + stringify(resource));
}
}
@@ -1542,13 +1541,11 @@ struct ResourceUsageChecker : TaskInfoVisitor
if (!slave->hasExecutor(framework->id, task.executor().executor_id())) {
taskResources += task.executor().resources();
if (!((usedResources + taskResources) <= offer->resources())) {
- LOG(WARNING) << "Task " << task.task_id() << " + executor attempted"
- << " to use " << taskResources << " combined with"
- << " already used " << usedResources << " is greater"
- << " than offered " << offer->resources();
-
return TaskInfoError::some(
- "Task + executor uses more resources than offered");
+ "Task " + stringify(task.task_id()) + " + executor attempted" +
+ " to use " + stringify(taskResources) + " combined with" +
+ " already used " + stringify(usedResources) + " is greater" +
+ " than offered " + stringify(offer->resources()));
}
}
executors.insert(task.executor().executor_id());
@@ -1588,7 +1585,14 @@ struct ExecutorInfoChecker : TaskInfoVisitor
if (!(task.executor() == executorInfo)) {
return TaskInfoError::some(
"Task has invalid ExecutorInfo (existing ExecutorInfo"
- " with same ExecutorID is not compatible)");
+ " with same ExecutorID is not compatible).\n"
+ "------------------------------------------------------------\n"
+ "Existing ExecutorInfo:\n" +
+ stringify(executorInfo) + "\n"
+ "------------------------------------------------------------\n"
+ "Task's ExecutorInfo:\n" +
+ stringify(task.executor()) + "\n"
+ "------------------------------------------------------------\n");
}
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/930aca13/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index e8176d2..83c250a 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -2593,8 +2593,11 @@ Future<Nothing> Slave::recover(bool reconnect, bool strict)
if (reconnect && !(info == state.get().info.get())) {
EXIT(1)
<< "Incompatible slave info detected.\n"
+ << "------------------------------------------------------------\n"
<< "Old slave info:\n" << state.get().info.get() << "\n"
+ << "------------------------------------------------------------\n"
<< "New slave info:\n" << info << "\n"
+ << "------------------------------------------------------------\n"
<< "To properly upgrade the slave do as follows:\n"
<< "Step 1: Start the slave with --recover=cleanup.\n"
<< "Step 2: Wait till the slave kills all executors and shuts down.\n"
http://git-wip-us.apache.org/repos/asf/mesos/blob/930aca13/src/tests/resource_offers_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resource_offers_tests.cpp b/src/tests/resource_offers_tests.cpp
index a96e775..3888e46 100644
--- a/src/tests/resource_offers_tests.cpp
+++ b/src/tests/resource_offers_tests.cpp
@@ -23,6 +23,8 @@
#include <mesos/executor.hpp>
#include <mesos/scheduler.hpp>
+#include <stout/strings.hpp>
+
#include "master/hierarchical_allocator_process.hpp"
#include "master/master.hpp"
@@ -252,7 +254,8 @@ TEST_F(ResourceOffersTest, TaskUsesMoreResourcesThanOffered)
EXPECT_EQ(task.task_id(), status.get().task_id());
EXPECT_EQ(TASK_LOST, status.get().state());
EXPECT_TRUE(status.get().has_message());
- EXPECT_EQ("Task uses more resources than offered", status.get().message());
+ EXPECT_TRUE(strings::contains(
+ status.get().message(), "greater than offered"));
driver.stop();
driver.join();
@@ -570,9 +573,8 @@ TEST_F(MultipleExecutorsTest, TasksExecutorInfoDiffers)
EXPECT_EQ(task2.task_id(), status.get().task_id());
EXPECT_EQ(TASK_LOST, status.get().state());
EXPECT_TRUE(status.get().has_message());
- EXPECT_EQ("Task has invalid ExecutorInfo (existing ExecutorInfo"
- " with same ExecutorID is not compatible)",
- status.get().message());
+ EXPECT_TRUE(strings::contains(
+ status.get().message(), "Task has invalid ExecutorInfo"));
EXPECT_CALL(exec, shutdown(_))
.Times(AtMost(1));