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 2015/06/14 11:46:38 UTC
[20/21] mesos git commit: Update HookManager to use synchronized.
Update HookManager to use synchronized.
Review: https://reviews.apache.org/r/35100
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5a69aa74
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5a69aa74
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5a69aa74
Branch: refs/heads/master
Commit: 5a69aa74d5d464f53629d81b7200e67f818789b4
Parents: 51fca3f
Author: Joris Van Remoortere <jo...@gmail.com>
Authored: Sat Jun 13 07:19:31 2015 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jun 14 02:43:01 2015 -0700
----------------------------------------------------------------------
src/hook/manager.cpp | 172 ++++++++++++++++++++++++----------------------
1 file changed, 91 insertions(+), 81 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/5a69aa74/src/hook/manager.cpp
----------------------------------------------------------------------
diff --git a/src/hook/manager.cpp b/src/hook/manager.cpp
index 54b0d34..5236035 100644
--- a/src/hook/manager.cpp
+++ b/src/hook/manager.cpp
@@ -16,8 +16,7 @@
* limitations under the License.
*/
-#include <pthread.h>
-
+#include <mutex>
#include <string>
#include <vector>
@@ -31,7 +30,6 @@
#include <stout/strings.hpp>
#include <stout/try.hpp>
-#include "common/lock.hpp"
#include "hook/manager.hpp"
#include "module/manager.hpp"
@@ -43,49 +41,52 @@ using mesos::modules::ModuleManager;
namespace mesos {
namespace internal {
-static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+static std::mutex mutex;
static hashmap<string, Hook*> availableHooks;
Try<Nothing> HookManager::initialize(const string& hookList)
{
- Lock lock(&mutex);
-
- const vector<string> hooks = strings::split(hookList, ",");
- foreach (const string& hook, hooks) {
- if (availableHooks.contains(hook)) {
- return Error("Hook module '" + hook + "' already loaded");
- }
-
- if (!ModuleManager::contains<Hook>(hook)) {
- return Error("No hook module named '" + hook + "' available");
- }
-
- // Let's create an instance of the hook module.
- Try<Hook*> module = ModuleManager::create<Hook>(hook);
- if (module.isError()) {
- return Error(
- "Failed to instantiate hook module '" + hook + "': " +
- module.error());
+ synchronized (mutex) {
+ const vector<string> hooks = strings::split(hookList, ",");
+ foreach (const string& hook, hooks) {
+ if (availableHooks.contains(hook)) {
+ return Error("Hook module '" + hook + "' already loaded");
+ }
+
+ if (!ModuleManager::contains<Hook>(hook)) {
+ return Error("No hook module named '" + hook + "' available");
+ }
+
+ // Let's create an instance of the hook module.
+ Try<Hook*> module = ModuleManager::create<Hook>(hook);
+ if (module.isError()) {
+ return Error(
+ "Failed to instantiate hook module '" + hook + "': " +
+ module.error());
+ }
+
+ // Add the hook module to the list of available hooks.
+ availableHooks[hook] = module.get();
}
-
- // Add the hook module to the list of available hooks.
- availableHooks[hook] = module.get();
}
+
return Nothing();
}
Try<Nothing> HookManager::unload(const std::string& hookName)
{
- Lock lock(&mutex);
- if (!availableHooks.contains(hookName)) {
- return Error(
- "Error unloading hook module '" + hookName + "': module not loaded");
+ synchronized (mutex) {
+ if (!availableHooks.contains(hookName)) {
+ return Error(
+ "Error unloading hook module '" + hookName + "': module not loaded");
+ }
+
+ // Now remove the hook from the list of available hooks.
+ availableHooks.erase(hookName);
}
- // Now remove the hook from the list of available hooks.
- availableHooks.erase(hookName);
return Nothing();
}
@@ -95,28 +96,33 @@ Labels HookManager::masterLaunchTaskLabelDecorator(
const FrameworkInfo& frameworkInfo,
const SlaveInfo& slaveInfo)
{
- Lock lock(&mutex);
-
- // We need a mutable copy of the task info and set the new
- // labels after each hook invocation. Otherwise, the last hook
- // will be the only effective hook setting the labels.
- TaskInfo taskInfo_ = taskInfo;
-
- foreachpair (const string& name, Hook* hook, availableHooks) {
- const Result<Labels>& result =
- hook->masterLaunchTaskLabelDecorator(taskInfo_, frameworkInfo, slaveInfo);
-
- // NOTE: If the hook returns None(), the task labels won't be
- // changed.
- if (result.isSome()) {
- taskInfo_.mutable_labels()->CopyFrom(result.get());
- } else if (result.isError()) {
- LOG(WARNING) << "Master label decorator hook failed for module '"
- << name << "': " << result.error();
+ synchronized (mutex) {
+ // We need a mutable copy of the task info and set the new
+ // labels after each hook invocation. Otherwise, the last hook
+ // will be the only effective hook setting the labels.
+ TaskInfo taskInfo_ = taskInfo;
+
+ foreachpair (const string& name, Hook* hook, availableHooks) {
+ const Result<Labels>& result =
+ hook->masterLaunchTaskLabelDecorator(
+ taskInfo_,
+ frameworkInfo,
+ slaveInfo);
+
+ // NOTE: If the hook returns None(), the task labels won't be
+ // changed.
+ if (result.isSome()) {
+ taskInfo_.mutable_labels()->CopyFrom(result.get());
+ } else if (result.isError()) {
+ LOG(WARNING) << "Master label decorator hook failed for module '"
+ << name << "': " << result.error();
+ }
}
+
+ return taskInfo_.labels();
}
- return taskInfo_.labels();
+ UNREACHABLE();
}
@@ -125,49 +131,53 @@ Labels HookManager::slaveRunTaskLabelDecorator(
const FrameworkInfo& frameworkInfo,
const SlaveInfo& slaveInfo)
{
- Lock lock(&mutex);
-
- TaskInfo taskInfo_ = taskInfo;
-
- foreachpair (const string& name, Hook* hook, availableHooks) {
- const Result<Labels>& result =
- hook->slaveRunTaskLabelDecorator(taskInfo_, frameworkInfo, slaveInfo);
-
- // NOTE: If the hook returns None(), the task labels won't be
- // changed.
- if (result.isSome()) {
- taskInfo_.mutable_labels()->CopyFrom(result.get());
- } else if (result.isError()) {
- LOG(WARNING) << "Slave label decorator hook failed for module '"
- << name << "': " << result.error();
+ synchronized (mutex) {
+ TaskInfo taskInfo_ = taskInfo;
+
+ foreachpair (const string& name, Hook* hook, availableHooks) {
+ const Result<Labels>& result =
+ hook->slaveRunTaskLabelDecorator(taskInfo_, frameworkInfo, slaveInfo);
+
+ // NOTE: If the hook returns None(), the task labels won't be
+ // changed.
+ if (result.isSome()) {
+ taskInfo_.mutable_labels()->CopyFrom(result.get());
+ } else if (result.isError()) {
+ LOG(WARNING) << "Slave label decorator hook failed for module '"
+ << name << "': " << result.error();
+ }
}
+
+ return taskInfo_.labels();
}
- return taskInfo_.labels();
+ UNREACHABLE();
}
Environment HookManager::slaveExecutorEnvironmentDecorator(
ExecutorInfo executorInfo)
{
- Lock lock(&mutex);
-
- foreachpair (const string& name, Hook* hook, availableHooks) {
- const Result<Environment>& result =
- hook->slaveExecutorEnvironmentDecorator(executorInfo);
-
- // NOTE: If the hook returns None(), the environment won't be
- // changed.
- if (result.isSome()) {
- executorInfo.mutable_command()->mutable_environment()->CopyFrom(
- result.get());
- } else if (result.isError()) {
- LOG(WARNING) << "Slave environment decorator hook failed for module '"
- << name << "': " << result.error();
+ synchronized (mutex) {
+ foreachpair (const string& name, Hook* hook, availableHooks) {
+ const Result<Environment>& result =
+ hook->slaveExecutorEnvironmentDecorator(executorInfo);
+
+ // NOTE: If the hook returns None(), the environment won't be
+ // changed.
+ if (result.isSome()) {
+ executorInfo.mutable_command()->mutable_environment()->CopyFrom(
+ result.get());
+ } else if (result.isError()) {
+ LOG(WARNING) << "Slave environment decorator hook failed for module '"
+ << name << "': " << result.error();
+ }
}
+
+ return executorInfo.command().environment();
}
- return executorInfo.command().environment();
+ UNREACHABLE();
}