You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2014/01/14 21:05:06 UTC
[15/15] git commit: Refactorings necessary to get Mesos to compile
with clang.
Refactorings necessary to get Mesos to compile with clang.
Review: https://reviews.apache.org/r/16662
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a2792e62
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a2792e62
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a2792e62
Branch: refs/heads/master
Commit: a2792e628be1eac1e834f31e049a0b5d90964e52
Parents: 02bd848
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sun Dec 29 22:57:47 2013 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Tue Jan 14 12:01:11 2014 -0800
----------------------------------------------------------------------
include/mesos/executor.hpp | 2 ++
include/mesos/scheduler.hpp | 2 ++
src/files/files.cpp | 1 +
src/master/allocator.hpp | 3 +++
src/master/contender.cpp | 5 ++++-
src/master/http.cpp | 8 ++++----
src/master/master.cpp | 2 +-
src/master/master.hpp | 7 +++++--
src/master/registrar.cpp | 5 +++--
src/slave/http.cpp | 4 ++--
src/slave/isolator.hpp | 5 ++++-
src/slave/slave.cpp | 5 ++---
src/slave/slave.hpp | 29 ++++++++++++++++++++++-------
src/slave/status_update_manager.cpp | 4 +++-
src/state/state.hpp | 1 +
15 files changed, 59 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/include/mesos/executor.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/executor.hpp b/include/mesos/executor.hpp
index 9b25834..7bc8eca 100644
--- a/include/mesos/executor.hpp
+++ b/include/mesos/executor.hpp
@@ -19,6 +19,8 @@
#ifndef __MESOS_EXECUTOR_HPP__
#define __MESOS_EXECUTOR_HPP__
+#include <pthread.h>
+
#include <string>
#include <mesos/mesos.hpp>
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/include/mesos/scheduler.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/scheduler.hpp b/include/mesos/scheduler.hpp
index 161cc65..8063997 100644
--- a/include/mesos/scheduler.hpp
+++ b/include/mesos/scheduler.hpp
@@ -19,6 +19,8 @@
#ifndef __MESOS_SCHEDULER_HPP__
#define __MESOS_SCHEDULER_HPP__
+#include <pthread.h>
+
#include <string>
#include <vector>
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/files/files.cpp
----------------------------------------------------------------------
diff --git a/src/files/files.cpp b/src/files/files.cpp
index 45ef95c..3dacc66 100644
--- a/src/files/files.cpp
+++ b/src/files/files.cpp
@@ -9,6 +9,7 @@
#include <boost/shared_array.hpp>
+#include <process/deferred.hpp> // TODO(benh): This is required by Clang.
#include <process/dispatch.hpp>
#include <process/future.hpp>
#include <process/http.hpp>
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/master/allocator.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator.hpp b/src/master/allocator.hpp
index 85ed214..2e6a910 100644
--- a/src/master/allocator.hpp
+++ b/src/master/allocator.hpp
@@ -57,6 +57,9 @@ public:
virtual ~AllocatorProcess() {}
+ // Explicitely use 'initialize' since we're overloading below.
+ using process::ProcessBase::initialize;
+
virtual void initialize(
const Flags& flags,
const process::PID<Master>& master,
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/master/contender.cpp
----------------------------------------------------------------------
diff --git a/src/master/contender.cpp b/src/master/contender.cpp
index 84b0552..72a7796 100644
--- a/src/master/contender.cpp
+++ b/src/master/contender.cpp
@@ -49,7 +49,10 @@ class ZooKeeperMasterContenderProcess
public:
ZooKeeperMasterContenderProcess(const zookeeper::URL& url);
ZooKeeperMasterContenderProcess(Owned<zookeeper::Group> group);
- ~ZooKeeperMasterContenderProcess();
+ virtual ~ZooKeeperMasterContenderProcess();
+
+ // Explicitely use 'initialize' since we're overloading below.
+ using process::ProcessBase::initialize;
void initialize(const PID<Master>& master);
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 5eee60f..231041d 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -199,8 +199,8 @@ JSON::Object model(const Framework& framework)
// Model all of the completed tasks of a framework.
{
JSON::Array array;
- foreach (const Task& task, framework.completedTasks) {
- array.values.push_back(model(task));
+ foreach (const memory::shared_ptr<Task>& task, framework.completedTasks) {
+ array.values.push_back(model(*task));
}
object.values["completed_tasks"] = array;
@@ -551,8 +551,8 @@ Future<Response> Master::Http::tasks(const Request& request)
CHECK_NOTNULL(task);
tasks.push_back(task);
}
- foreach (const Task& task, framework->completedTasks) {
- tasks.push_back(&task);
+ foreach (const memory::shared_ptr<Task>& task, framework->completedTasks) {
+ tasks.push_back(task.get());
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index fa1277a..008033e 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -671,7 +671,7 @@ void Master::fileAttached(const Future<Nothing>& result, const string& path)
LOG(INFO) << "Successfully attached file '" << path << "'";
} else {
LOG(ERROR) << "Failed to attach file '" << path << "': "
- << result.isFailed() ? result.failure() : "discarded";
+ << (result.isFailed() ? result.failure() : "discarded");
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 10feb93..6e89149 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -565,7 +565,7 @@ struct Framework
<< "Unknown task " << task->task_id()
<< " of framework " << task->framework_id();
- completedTasks.push_back(*task);
+ completedTasks.push_back(memory::shared_ptr<Task>(new Task(*task)));
tasks.erase(task->task_id());
resources -= task->resources();
}
@@ -633,7 +633,10 @@ struct Framework
hashmap<TaskID, Task*> tasks;
- boost::circular_buffer<Task> completedTasks;
+ // 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<memory::shared_ptr<Task> > completedTasks;
hashset<Offer*> offers; // Active offers for framework.
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/master/registrar.cpp
----------------------------------------------------------------------
diff --git a/src/master/registrar.cpp b/src/master/registrar.cpp
index 61fdea3..915885a 100644
--- a/src/master/registrar.cpp
+++ b/src/master/registrar.cpp
@@ -208,8 +208,9 @@ void RegistrarProcess::_recover(
CHECK(!recovery.isPending());
if (recovery.isFailed() || recovery.isDiscarded()) {
- LOG(WARNING) << "Failed to recover registrar: " << recovery.isFailed()
- ? recovery.failure() : "future discarded";
+ LOG(WARNING)
+ << "Failed to recover registrar: "
+ << (recovery.isFailed() ? recovery.failure() : "future discarded");
recover(); // Retry! TODO(benh): Don't retry forever?
} else {
LOG(INFO) << "Successfully recovered registrar";
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index 1358810..4876364 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -218,8 +218,8 @@ JSON::Object model(const Executor& executor)
object.values["queued_tasks"] = queued;
JSON::Array completed;
- foreach (const Task& task, executor.completedTasks) {
- completed.values.push_back(model(task));
+ foreach (const memory::shared_ptr<Task>& task, executor.completedTasks) {
+ completed.values.push_back(model(*task));
}
// NOTE: We add 'terminatedTasks' to 'completed_tasks' for
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/slave/isolator.hpp
----------------------------------------------------------------------
diff --git a/src/slave/isolator.hpp b/src/slave/isolator.hpp
index fc13089..9634535 100644
--- a/src/slave/isolator.hpp
+++ b/src/slave/isolator.hpp
@@ -41,7 +41,7 @@ namespace internal {
namespace slave {
namespace state {
-class SlaveState; // Forward declaration.
+struct SlaveState; // Forward declaration.
} // namespace state {
@@ -57,6 +57,9 @@ public:
virtual ~Isolator() {}
+ // Explicitely use 'initialize' since we're overloading below.
+ using process::ProcessBase::initialize;
+
// Called during slave initialization.
virtual void initialize(
const Flags& flags,
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 57d58b3..8b83dac 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -465,7 +465,7 @@ void Slave::fileAttached(const Future<Nothing>& result, const string& path)
VLOG(1) << "Successfully attached file '" << path << "'";
} else {
LOG(ERROR) << "Failed to attach file '" << path << "': "
- << result.isFailed() ? result.failure() : "discarded";
+ << (result.isFailed() ? result.failure() : "discarded");
}
}
@@ -3229,9 +3229,8 @@ void Executor::completeTask(const TaskID& taskId)
<< "Failed to find terminated task " << taskId;
Task* task = terminatedTasks[taskId];
- completedTasks.push_back(*task);
+ completedTasks.push_back(memory::shared_ptr<Task>(task));
terminatedTasks.erase(taskId);
- delete task;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 34edef2..38ba7b6 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -211,7 +211,10 @@ public:
TERMINATING, // Slave is shutting down.
} state;
-protected:
+ // TODO(benh): Clang requires members to be public in order to take
+ // their address which we do in tests (for things like
+ // FUTURE_DISPATCH).
+// protected:
virtual void initialize();
virtual void finalize();
virtual void exited(const UPID& pid);
@@ -304,8 +307,8 @@ private:
const Slave& slave;
} http;
- friend class Framework;
- friend class Executor;
+ friend struct Framework;
+ friend struct Executor;
Slave(const Slave&); // No copying.
Slave& operator = (const Slave&); // No assigning.
@@ -410,10 +413,22 @@ struct Executor
Resources resources; // Currently consumed resources.
- LinkedHashMap<TaskID, TaskInfo> queuedTasks; // Not yet launched.
- LinkedHashMap<TaskID, Task*> launchedTasks; // Running.
- LinkedHashMap<TaskID, Task*> terminatedTasks; // Terminated but pending updates.
- boost::circular_buffer<Task> completedTasks; // Terminated and updates acked.
+ // Tasks can be found in one of the following four data structures:
+
+ // Not yet launched.
+ LinkedHashMap<TaskID, TaskInfo> queuedTasks;
+
+ // Running.
+ LinkedHashMap<TaskID, Task*> launchedTasks;
+
+ // Terminated but pending updates.
+ LinkedHashMap<TaskID, Task*> terminatedTasks;
+
+ // Terminated and updates acked.
+ // 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<memory::shared_ptr<Task> > completedTasks;
private:
Executor(const Executor&); // No copying.
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/slave/status_update_manager.cpp
----------------------------------------------------------------------
diff --git a/src/slave/status_update_manager.cpp b/src/slave/status_update_manager.cpp
index f7a0c40..03f5eaf 100644
--- a/src/slave/status_update_manager.cpp
+++ b/src/slave/status_update_manager.cpp
@@ -59,8 +59,10 @@ public:
StatusUpdateManagerProcess() {}
virtual ~StatusUpdateManagerProcess();
- // StatusUpdateManager implementation.
+ // Explicitely use 'initialize' since we're overloading below.
+ using process::ProcessBase::initialize;
+ // StatusUpdateManager implementation.
void initialize(
const Flags& flags,
const PID<Slave>& slave);
http://git-wip-us.apache.org/repos/asf/mesos/blob/a2792e62/src/state/state.hpp
----------------------------------------------------------------------
diff --git a/src/state/state.hpp b/src/state/state.hpp
index 02620b3..133752d 100644
--- a/src/state/state.hpp
+++ b/src/state/state.hpp
@@ -22,6 +22,7 @@
#include <string>
#include <vector>
+#include <process/deferred.hpp> // TODO(benh): This is required by Clang.
#include <process/future.hpp>
#include <stout/lambda.hpp>