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 2017/11/30 02:52:45 UTC
mesos git commit: Improved master failover performance by avoiding
resource conversions.
Repository: mesos
Updated Branches:
refs/heads/master c4cec59f0 -> 24550e7b8
Improved master failover performance by avoiding resource conversions.
RepeatedPtrField<Resource> can be implicitly converted to Resources,
leading to hidden multiple resources conversions on performance-critical
paths in master. For example, operator += relies on implicit
conversion, when invoked with RepeatedPtrField<Resource> argument.
Using protobuf also implies data validation and sanitization, e.g. when
converting to Resources, as protobuf generally comes from untrusted
sources. By doing conversion only once, and then reusing the result,
we save on these checks as well, as operations on Resources are
generally faster as they can trust data in Resources.
Review: https://reviews.apache.org/r/64028/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/24550e7b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/24550e7b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/24550e7b
Branch: refs/heads/master
Commit: 24550e7b863fee371877ea9d1d9153e0f5054155
Parents: c4cec59
Author: Dmitry Zhuk <dz...@twopensource.com>
Authored: Wed Nov 29 18:47:13 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Wed Nov 29 18:48:48 2017 -0800
----------------------------------------------------------------------
src/master/master.cpp | 37 +++++++++++++++++++++++--------------
src/master/master.hpp | 10 ++++++++--
2 files changed, 31 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/24550e7b/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 1876d7d..fadc78b 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -9561,14 +9561,19 @@ void Master::removeTask(Task* task, bool unreachable)
Slave* slave = slaves.registered.get(task->slave_id());
CHECK_NOTNULL(slave);
+ // Note that we explicitly convert from protobuf to `Resources` here
+ // and then use the result below to avoid performance penalty for multiple
+ // conversions and validations implied by conversion.
+ // Conversion is safe, as resources have already passed validation.
+ const Resources resources = task->resources();
+
if (!isRemovable(task->state())) {
CHECK(!unreachable) << task->task_id();
- // Note that we convert to `Resources` for output as it's faster than
- // logging raw protobuf data. Conversion is safe, as resources have
- // already passed validation.
+ // Note that we use `Resources` for output as it's faster than
+ // logging raw protobuf data.
LOG(WARNING) << "Removing task " << task->task_id()
- << " with resources " << Resources(task->resources())
+ << " with resources " << resources
<< " of framework " << task->framework_id()
<< " on agent " << *slave
<< " in non-removable state " << task->state();
@@ -9578,14 +9583,13 @@ void Master::removeTask(Task* task, bool unreachable)
allocator->recoverResources(
task->framework_id(),
task->slave_id(),
- task->resources(),
+ resources,
None());
} else {
- // Note that we convert to `Resources` for output as it's faster than
- // logging raw protobuf data. Conversion is safe, as resources have
- // already passed validation.
+ // Note that we use `Resources` for output as it's faster than
+ // logging raw protobuf data.
LOG(INFO) << "Removing task " << task->task_id()
- << " with resources " << Resources(task->resources())
+ << " with resources " << resources
<< " of framework " << task->framework_id()
<< " on agent " << *slave;
}
@@ -10774,19 +10778,24 @@ void Slave::addTask(Task* task)
tasks[frameworkId][taskId] = task;
+ // Note that we explicitly convert from protobuf to `Resources` here
+ // and then use the result below to avoid performance penalty for multiple
+ // conversions and validations implied by conversion.
+ // Conversion is safe, as resources have already passed validation.
+ const Resources resources = task->resources();
+
if (!Master::isRemovable(task->state())) {
- usedResources[frameworkId] += task->resources();
+ usedResources[frameworkId] += resources;
}
if (!master->subscribers.subscribed.empty()) {
master->subscribers.send(protobuf::master::event::createTaskAdded(*task));
}
- // Note that we convert to `Resources` for output as it's faster than
- // logging raw protobuf data. Conversion is safe, as resources have
- // already passed validation.
+ // Note that we use `Resources` for output as it's faster than
+ // logging raw protobuf data.
LOG(INFO) << "Adding task " << taskId
- << " with resources " << Resources(task->resources())
+ << " with resources " << resources
<< " on agent " << *this;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/24550e7b/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 3c86463..0f8a2ac 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -2244,8 +2244,14 @@ struct Framework
tasks[task->task_id()] = task;
if (!Master::isRemovable(task->state())) {
- totalUsedResources += task->resources();
- usedResources[task->slave_id()] += task->resources();
+ // Note that we explicitly convert from protobuf to `Resources` once
+ // and then use the result for calculations to avoid performance penalty
+ // for multiple conversions and validations implied by `+=` with protobuf
+ // agrument.
+ // Conversion is safe, as resources have already passed validation.
+ const Resources resources = task->resources();
+ totalUsedResources += resources;
+ usedResources[task->slave_id()] += resources;
// It's possible that we're not tracking the task's role for
// this framework if the role is absent from the framework's