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 2020/03/26 05:38:40 UTC
[mesos] branch master updated: Fixed locking and static PODs in
Hook manager.
This is an automated email from the ASF dual-hosted git repository.
bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new f3253f8 Fixed locking and static PODs in Hook manager.
f3253f8 is described below
commit f3253f87cd9a05d2fc8cc76821bffb9bc2c36b2a
Author: Dong Zhu <dz...@d2iq.com>
AuthorDate: Thu Mar 26 01:37:09 2020 -0400
Fixed locking and static PODs in Hook manager.
This patch aims to fix issue MESOS-9337:
1. add missing mutex acquisition for the potential race
2. change the global mutex and map variable to POD type
Signed-off-by: Dong Zhu <dz...@d2iq.com>
This closes #353
---
src/hook/manager.cpp | 93 ++++++++++++++++++++++++++++------------------------
1 file changed, 51 insertions(+), 42 deletions(-)
diff --git a/src/hook/manager.cpp b/src/hook/manager.cpp
index a1f1241..eb312c1 100644
--- a/src/hook/manager.cpp
+++ b/src/hook/manager.cpp
@@ -47,8 +47,9 @@ using mesos::modules::ModuleManager;
namespace mesos {
namespace internal {
-static std::mutex mutex;
-static LinkedHashMap<string, Hook*> availableHooks;
+static std::mutex* mutex = new std::mutex();
+static LinkedHashMap<string, Hook*>* availableHooks =
+ new LinkedHashMap<string, Hook*>();
Try<Nothing> HookManager::initialize(const string& hookList)
@@ -56,7 +57,7 @@ Try<Nothing> HookManager::initialize(const string& hookList)
synchronized (mutex) {
const vector<string> hooks = strings::split(hookList, ",");
foreach (const string& hook, hooks) {
- if (availableHooks.contains(hook)) {
+ if (availableHooks->contains(hook)) {
return Error("Hook module '" + hook + "' already loaded");
}
@@ -73,7 +74,7 @@ Try<Nothing> HookManager::initialize(const string& hookList)
}
// Add the hook module to the list of available hooks.
- availableHooks[hook] = module.get();
+ (*availableHooks)[hook] = module.get();
}
}
@@ -84,13 +85,13 @@ Try<Nothing> HookManager::initialize(const string& hookList)
Try<Nothing> HookManager::unload(const string& hookName)
{
synchronized (mutex) {
- if (!availableHooks.contains(hookName)) {
+ 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);
+ availableHooks->erase(hookName);
}
return Nothing();
@@ -100,7 +101,7 @@ Try<Nothing> HookManager::unload(const string& hookName)
bool HookManager::hooksAvailable()
{
synchronized (mutex) {
- return !availableHooks.empty();
+ return !availableHooks->empty();
}
}
@@ -116,7 +117,7 @@ Labels HookManager::masterLaunchTaskLabelDecorator(
TaskInfo taskInfo_ = taskInfo;
synchronized (mutex) {
- foreachpair (const string& name, Hook* hook, availableHooks) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
Result<Labels> result =
hook->masterLaunchTaskLabelDecorator(
taskInfo_,
@@ -145,7 +146,7 @@ Resources HookManager::masterLaunchTaskResourceDecorator(
TaskInfo taskInfo_ = taskInfo;
synchronized (mutex) {
- foreachpair (const string& name, Hook* hook, availableHooks) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
Result<Resources> result =
hook->masterLaunchTaskResourceDecorator(
taskInfo_,
@@ -165,11 +166,13 @@ Resources HookManager::masterLaunchTaskResourceDecorator(
void HookManager::masterSlaveLostHook(const SlaveInfo& slaveInfo)
{
- foreachpair (const string& name, Hook* hook, availableHooks) {
- Try<Nothing> result = hook->masterSlaveLostHook(slaveInfo);
- if (result.isError()) {
- LOG(WARNING) << "Master agent-lost hook failed for module '"
- << name << "': " << result.error();
+ synchronized (mutex) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
+ Try<Nothing> result = hook->masterSlaveLostHook(slaveInfo);
+ if (result.isError()) {
+ LOG(WARNING) << "Master agent-lost hook failed for module '"
+ << name << "': " << result.error();
+ }
}
}
}
@@ -184,7 +187,7 @@ Labels HookManager::slaveRunTaskLabelDecorator(
synchronized (mutex) {
TaskInfo taskInfo_ = taskInfo;
- foreachpair (const string& name, Hook* hook, availableHooks) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
const Result<Labels> result = hook->slaveRunTaskLabelDecorator(
taskInfo_, executorInfo, frameworkInfo, slaveInfo);
@@ -207,7 +210,7 @@ Environment HookManager::slaveExecutorEnvironmentDecorator(
ExecutorInfo executorInfo)
{
synchronized (mutex) {
- foreachpair (const string& name, Hook* hook, availableHooks) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
const Result<Environment> result =
hook->slaveExecutorEnvironmentDecorator(executorInfo);
@@ -240,18 +243,20 @@ Future<DockerTaskExecutorPrepareInfo>
// `DockerTaskExecutorPrepareInfo` can be deterministically resolved
// (the last hook takes priority).
vector<Future<Option<DockerTaskExecutorPrepareInfo>>> futures;
- futures.reserve(availableHooks.size());
-
- foreachvalue (Hook* hook, availableHooks) {
- // Chain together each hook.
- futures.push_back(
- hook->slavePreLaunchDockerTaskExecutorDecorator(
- taskInfo,
- executorInfo,
- containerName,
- containerWorkDirectory,
- mappedSandboxDirectory,
- env));
+ synchronized (mutex) {
+ futures.reserve(availableHooks->size());
+
+ foreachvalue (Hook* hook, *availableHooks) {
+ // Chain together each hook.
+ futures.push_back(
+ hook->slavePreLaunchDockerTaskExecutorDecorator(
+ taskInfo,
+ executorInfo,
+ containerName,
+ containerWorkDirectory,
+ mappedSandboxDirectory,
+ env));
+ }
}
return collect(futures)
@@ -274,11 +279,13 @@ void HookManager::slavePostFetchHook(
const ContainerID& containerId,
const string& directory)
{
- foreachpair (const string& name, Hook* hook, availableHooks) {
- Try<Nothing> result = hook->slavePostFetchHook(containerId, directory);
- if (result.isError()) {
- LOG(WARNING) << "Agent post fetch hook failed for module "
- << "'" << name << "': " << result.error();
+ synchronized (mutex) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
+ Try<Nothing> result = hook->slavePostFetchHook(containerId, directory);
+ if (result.isError()) {
+ LOG(WARNING) << "Agent post fetch hook failed for module "
+ << "'" << name << "': " << result.error();
+ }
}
}
}
@@ -288,12 +295,14 @@ void HookManager::slaveRemoveExecutorHook(
const FrameworkInfo& frameworkInfo,
const ExecutorInfo& executorInfo)
{
- foreachpair (const string& name, Hook* hook, availableHooks) {
- const Try<Nothing> result =
- hook->slaveRemoveExecutorHook(frameworkInfo, executorInfo);
- if (result.isError()) {
- LOG(WARNING) << "Agent remove executor hook failed for module '"
- << name << "': " << result.error();
+ synchronized (mutex) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
+ const Try<Nothing> result =
+ hook->slaveRemoveExecutorHook(frameworkInfo, executorInfo);
+ if (result.isError()) {
+ LOG(WARNING) << "Agent remove executor hook failed for module '"
+ << name << "': " << result.error();
+ }
}
}
}
@@ -304,7 +313,7 @@ TaskStatus HookManager::slaveTaskStatusDecorator(
TaskStatus status)
{
synchronized (mutex) {
- foreachpair (const string& name, Hook* hook, availableHooks) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
const Result<TaskStatus> result =
hook->slaveTaskStatusDecorator(frameworkId, status);
@@ -340,7 +349,7 @@ Resources HookManager::slaveResourcesDecorator(
SlaveInfo slaveInfo_ = slaveInfo;
synchronized (mutex) {
- foreachpair (const string& name, Hook* hook, availableHooks) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
const Result<Resources> result =
hook->slaveResourcesDecorator(slaveInfo_);
@@ -368,7 +377,7 @@ Attributes HookManager::slaveAttributesDecorator(
SlaveInfo slaveInfo_ = slaveInfo;
synchronized (mutex) {
- foreachpair (const string& name, Hook* hook, availableHooks) {
+ foreachpair (const string& name, Hook* hook, *availableHooks) {
const Result<Attributes> result =
hook->slaveAttributesDecorator(slaveInfo_);