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 2016/12/16 19:24:25 UTC
[2/5] mesos git commit: Updated master to use BoundedHashMap.
Updated master to use BoundedHashMap.
This replaces a few instances where boost::circular_buffer was
previously used.
Review: https://reviews.apache.org/r/54179/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b6ce8c94
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b6ce8c94
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b6ce8c94
Branch: refs/heads/master
Commit: b6ce8c94f74f42a0ad39ef1ceecad90c6e9de54e
Parents: 12baba3
Author: Neil Conway <ne...@gmail.com>
Authored: Fri Dec 16 14:23:32 2016 -0500
Committer: Vinod Kone <vi...@gmail.com>
Committed: Fri Dec 16 14:23:32 2016 -0500
----------------------------------------------------------------------
src/master/http.cpp | 45 +++++++++++++++++++++------------------------
src/master/master.cpp | 15 +++++----------
src/master/master.hpp | 16 +++++++++-------
3 files changed, 35 insertions(+), 41 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b6ce8c94/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index fd93616..ee55980 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -25,8 +25,6 @@
#include <utility>
#include <vector>
-#include <boost/array.hpp>
-
#include <mesos/attributes.hpp>
#include <mesos/type_utils.hpp>
@@ -284,13 +282,13 @@ struct FullFrameworkWriter {
});
writer->field("completed_tasks", [this](JSON::ArrayWriter* writer) {
- foreach (const std::shared_ptr<Task>& task, framework_->completedTasks) {
+ foreach (const Owned<Task>& task, framework_->completedTasks) {
// Skip unauthorized tasks.
if (!approveViewTask(taskApprover_, *task.get(), framework_->info)) {
continue;
}
- writer->element(*task);
+ writer->element(*task.get());
}
});
@@ -1343,8 +1341,8 @@ Future<Response> Master::Http::frameworks(
"completed_frameworks",
[this, &frameworksApprover, &executorsApprover, &tasksApprover](
JSON::ArrayWriter* writer) {
- foreach (const std::shared_ptr<Framework>& framework,
- master->frameworks.completed) {
+ foreachvalue (const Owned<Framework>& framework,
+ master->frameworks.completed) {
// Skip unauthorized frameworks.
if (!approveViewFrameworkInfo(
frameworksApprover, framework->info)) {
@@ -1487,14 +1485,14 @@ mesos::master::Response::GetFrameworks Master::Http::_getFrameworks(
getFrameworks.add_frameworks()->CopyFrom(model(*framework));
}
- foreach (const std::shared_ptr<Framework>& framework,
- master->frameworks.completed) {
+ foreachvalue (const Owned<Framework>& framework,
+ master->frameworks.completed) {
// Skip unauthorized frameworks.
if (!approveViewFrameworkInfo(frameworksApprover, framework->info)) {
continue;
}
- getFrameworks.add_completed_frameworks()->CopyFrom(model(*framework));
+ getFrameworks.add_completed_frameworks()->CopyFrom(model(*framework.get()));
}
return getFrameworks;
@@ -1564,8 +1562,8 @@ mesos::master::Response::GetExecutors Master::Http::_getExecutors(
frameworks.push_back(framework);
}
- foreach (const std::shared_ptr<Framework>& framework,
- master->frameworks.completed) {
+ foreachvalue (const Owned<Framework>& framework,
+ master->frameworks.completed) {
// Skip unauthorized frameworks.
if (!approveViewFrameworkInfo(frameworksApprover, framework->info)) {
continue;
@@ -2681,9 +2679,8 @@ Future<Response> Master::Http::state(
"completed_frameworks",
[this, &frameworksApprover, &executorsApprover, &tasksApprover](
JSON::ArrayWriter* writer) {
- foreach (
- const std::shared_ptr<Framework>& framework,
- master->frameworks.completed) {
+ foreachvalue (const Owned<Framework>& framework,
+ master->frameworks.completed) {
// Skip unauthorized frameworks.
if (!approveViewFrameworkInfo(
frameworksApprover, framework->info)) {
@@ -2826,7 +2823,7 @@ public:
slavesToFrameworks[task->slave_id()].insert(frameworkId);
}
- foreach (const std::shared_ptr<Task>& task, framework->completedTasks) {
+ foreach (const Owned<Task>& task, framework->completedTasks) {
frameworksToSlaves[frameworkId].insert(task->slave_id());
slavesToFrameworks[task->slave_id()].insert(frameworkId);
}
@@ -2942,9 +2939,9 @@ public:
slaveTaskSummaries[task->slave_id()].count(*task);
}
- foreach (const std::shared_ptr<Task>& task, framework->completedTasks) {
- frameworkTaskSummaries[frameworkId].count(*task);
- slaveTaskSummaries[task->slave_id()].count(*task);
+ foreach (const Owned<Task>& task, framework->completedTasks) {
+ frameworkTaskSummaries[frameworkId].count(*task.get());
+ slaveTaskSummaries[task->slave_id()].count(*task.get());
}
}
}
@@ -3645,8 +3642,8 @@ Future<Response> Master::Http::tasks(
frameworks.push_back(framework);
}
- foreach (const std::shared_ptr<Framework>& framework,
- master->frameworks.completed) {
+ foreachvalue (const Owned<Framework>& framework,
+ master->frameworks.completed) {
// Skip unauthorized frameworks.
if (!approveViewFrameworkInfo(frameworksApprover, framework->info)) {
continue;
@@ -3667,7 +3664,7 @@ Future<Response> Master::Http::tasks(
tasks.push_back(task);
}
- foreach (const std::shared_ptr<Task>& task, framework->completedTasks) {
+ foreach (const Owned<Task>& task, framework->completedTasks) {
// Skip unauthorized tasks.
if (!approveViewTask(tasksApprover, *task.get(), framework->info)) {
continue;
@@ -3767,8 +3764,8 @@ mesos::master::Response::GetTasks Master::Http::_getTasks(
frameworks.push_back(framework);
}
- foreach (const std::shared_ptr<Framework>& framework,
- master->frameworks.completed) {
+ foreachvalue (const Owned<Framework>& framework,
+ master->frameworks.completed) {
// Skip unauthorized frameworks.
if (!approveViewFrameworkInfo(frameworksApprover, framework->info)) {
continue;
@@ -3806,7 +3803,7 @@ mesos::master::Response::GetTasks Master::Http::_getTasks(
}
// Completed tasks.
- foreach (const std::shared_ptr<Task>& task, framework->completedTasks) {
+ foreach (const Owned<Task>& task, framework->completedTasks) {
// Skip unauthorized tasks.
if (!approveViewTask(tasksApprover, *task.get(), framework->info)) {
continue;
http://git-wip-us.apache.org/repos/asf/mesos/blob/b6ce8c94/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 9023a5f..efac4f8 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -6951,7 +6951,8 @@ void Master::reconcileKnownSlave(
//
// TODO(vinod): Revisit this when registrar is in place. It would
// likely involve storing this information in the registrar.
- foreach (const shared_ptr<Framework>& framework, frameworks.completed) {
+ foreachvalue (const Owned<Framework>& framework,
+ frameworks.completed) {
if (slaveTasks.contains(framework->id())) {
LOG(WARNING) << "Agent " << *slave
<< " re-registered with completed framework " << *framework
@@ -7448,8 +7449,8 @@ void Master::removeFramework(Framework* framework)
frameworks.registered.erase(framework->id());
allocator->removeFramework(framework->id());
- // The completedFramework buffer now owns the framework pointer.
- frameworks.completed.push_back(shared_ptr<Framework>(framework));
+ // The framework pointer is now owned by `frameworks.completed`.
+ frameworks.completed.set(framework->id(), Owned<Framework>(framework));
}
@@ -8139,13 +8140,7 @@ void Master::removeInverseOffer(InverseOffer* inverseOffer, bool rescind)
bool Master::isCompletedFramework(const FrameworkID& frameworkId)
{
- foreach (const shared_ptr<Framework>& framework, frameworks.completed) {
- if (framework->id() == frameworkId) {
- return true;
- }
- }
-
- return false;
+ return frameworks.completed.contains(frameworkId);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/b6ce8c94/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index c304e69..4f2d2f8 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -52,6 +52,7 @@
#include <process/metrics/counter.hpp>
+#include <stout/boundedhashmap.hpp>
#include <stout/cache.hpp>
#include <stout/foreach.hpp>
#include <stout/hashmap.hpp>
@@ -1764,7 +1765,7 @@ private:
hashmap<FrameworkID, Framework*> registered;
- boost::circular_buffer<std::shared_ptr<Framework>> completed;
+ BoundedHashMap<FrameworkID, process::Owned<Framework>> completed;
// Principals of frameworks keyed by PID.
// NOTE: Multiple PIDs can map to the same principal. The
@@ -2320,8 +2321,12 @@ struct Framework
void addCompletedTask(const Task& task)
{
- // TODO(adam-mesos): Check if completed task already exists.
- completedTasks.push_back(std::shared_ptr<Task>(new Task(task)));
+ // TODO(neilc): We currently allow frameworks to reuse the task
+ // IDs of completed tasks (although this is discouraged). This
+ // means that there might be multiple completed tasks with the
+ // same task ID. We should consider rejecting attempts to reuse
+ // task IDs (MESOS-6779).
+ completedTasks.push_back(process::Owned<Task>(new Task(task)));
}
void removeTask(Task* task)
@@ -2589,10 +2594,7 @@ struct Framework
hashmap<TaskID, Task*> tasks;
- // NOTE: We use a shared pointer for Task because clang doesn't like
- // Boost's implementation of circular_buffer with Task (Boost
- // attempts to do some memset's which are unsafe).
- boost::circular_buffer<std::shared_ptr<Task>> completedTasks;
+ boost::circular_buffer<process::Owned<Task>> completedTasks;
hashset<Offer*> offers; // Active offers for framework.