You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by al...@apache.org on 2018/06/15 13:41:08 UTC
[3/3] mesos git commit: Removed `os::getenv()` calls from
`MesosExecutorDriver`.
Removed `os::getenv()` calls from `MesosExecutorDriver`.
This patch adds overloaded constructor for `MesosExecutorDriver` that
accepts `environment` parameter and stores it in the class variable.
This new constructor is needed to get rid of `os::getenv()` calls,
so that `MesosExecutorDriver` can be used in tests that require
thread safety.
Review: https://reviews.apache.org/r/67354/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/20edb8ca
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/20edb8ca
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/20edb8ca
Branch: refs/heads/master
Commit: 20edb8caa394b41521bc70f39664bb97fe2e1791
Parents: 3bf6eb7
Author: Andrei Budnik <ab...@mesosphere.com>
Authored: Fri Jun 15 15:23:27 2018 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Fri Jun 15 15:40:54 2018 +0200
----------------------------------------------------------------------
include/mesos/executor.hpp | 15 +++++++++++++++
src/exec/exec.cpp | 42 ++++++++++++++++++++++++++++++-----------
2 files changed, 46 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/20edb8ca/include/mesos/executor.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/executor.hpp b/include/mesos/executor.hpp
index d14c036..6a9e6fc 100644
--- a/include/mesos/executor.hpp
+++ b/include/mesos/executor.hpp
@@ -17,6 +17,7 @@
#ifndef __MESOS_EXECUTOR_HPP__
#define __MESOS_EXECUTOR_HPP__
+#include <map>
#include <mutex>
#include <string>
@@ -213,8 +214,20 @@ class MesosExecutorDriver : public ExecutorDriver
public:
// Creates a new driver that uses the specified Executor. Note, the
// executor pointer must outlive the driver.
+ //
+ // Note that the other constructor overload that accepts `environment`
+ // argument is preferable to this one in a multithreaded environment,
+ // because the implementation of this one accesses global environment
+ // which is unsafe due to a potential concurrent modification of the
+ // environment by another thread.
explicit MesosExecutorDriver(Executor* executor);
+ // Creates a new driver that uses the specified `Executor` and environment
+ // variables. Note, the executor pointer must outlive the driver.
+ explicit MesosExecutorDriver(
+ Executor* executor,
+ const std::map<std::string, std::string>& environment);
+
// This destructor will block indefinitely if
// MesosExecutorDriver::start was invoked successfully (possibly via
// MesosExecutorDriver::run) and MesosExecutorDriver::stop has not
@@ -246,6 +259,8 @@ private:
// Current status of the driver.
Status status;
+
+ std::map<std::string, std::string> environment;
};
} // namespace mesos {
http://git-wip-us.apache.org/repos/asf/mesos/blob/20edb8ca/src/exec/exec.cpp
----------------------------------------------------------------------
diff --git a/src/exec/exec.cpp b/src/exec/exec.cpp
index 65a671d..ca4f065 100644
--- a/src/exec/exec.cpp
+++ b/src/exec/exec.cpp
@@ -633,17 +633,36 @@ private:
MesosExecutorDriver::MesosExecutorDriver(mesos::Executor* _executor)
+ : MesosExecutorDriver(_executor, os::environment())
+{}
+
+
+MesosExecutorDriver::MesosExecutorDriver(
+ mesos::Executor* _executor,
+ const std::map<std::string, std::string>& _environment)
: executor(_executor),
process(nullptr),
latch(nullptr),
- status(DRIVER_NOT_STARTED)
+ status(DRIVER_NOT_STARTED),
+ environment(_environment)
{
GOOGLE_PROTOBUF_VERIFY_VERSION;
// Load any logging flags from the environment.
logging::Flags flags;
- Try<flags::Warnings> load = flags.load("MESOS_");
+ // Filter out environment variables whose keys don't start with "MESOS_".
+ //
+ // TODO(alexr): This should be supported by `FlagsBase`, see MESOS-9001.
+ std::map<std::string, std::string> env;
+
+ foreachpair (const string& key, const string& value, environment) {
+ if (strings::startsWith(key, "MESOS_")) {
+ env.emplace(key, value);
+ }
+ }
+
+ Try<flags::Warnings> load = flags.load(env, true);
if (load.isError()) {
status = DRIVER_ABORTED;
@@ -719,12 +738,13 @@ Status MesosExecutorDriver::start()
Option<string> value;
std::istringstream iss;
+ hashmap<string, string> env(environment);
// Check if this is local (for example, for testing).
- local = os::getenv("MESOS_LOCAL").isSome();
+ local = env.get("MESOS_LOCAL").isSome();
// Get slave PID from environment.
- value = os::getenv("MESOS_SLAVE_PID");
+ value = env.get("MESOS_SLAVE_PID");
if (value.isNone()) {
EXIT(EXIT_FAILURE)
<< "Expecting 'MESOS_SLAVE_PID' to be set in the environment";
@@ -734,7 +754,7 @@ Status MesosExecutorDriver::start()
CHECK(slave) << "Cannot parse MESOS_SLAVE_PID '" << value.get() << "'";
// Get slave ID from environment.
- value = os::getenv("MESOS_SLAVE_ID");
+ value = env.get("MESOS_SLAVE_ID");
if (value.isNone()) {
EXIT(EXIT_FAILURE)
<< "Expecting 'MESOS_SLAVE_ID' to be set in the environment";
@@ -742,7 +762,7 @@ Status MesosExecutorDriver::start()
slaveId.set_value(value.get());
// Get framework ID from environment.
- value = os::getenv("MESOS_FRAMEWORK_ID");
+ value = env.get("MESOS_FRAMEWORK_ID");
if (value.isNone()) {
EXIT(EXIT_FAILURE)
<< "Expecting 'MESOS_FRAMEWORK_ID' to be set in the environment";
@@ -750,7 +770,7 @@ Status MesosExecutorDriver::start()
frameworkId.set_value(value.get());
// Get executor ID from environment.
- value = os::getenv("MESOS_EXECUTOR_ID");
+ value = env.get("MESOS_EXECUTOR_ID");
if (value.isNone()) {
EXIT(EXIT_FAILURE)
<< "Expecting 'MESOS_EXECUTOR_ID' to be set in the environment";
@@ -758,7 +778,7 @@ Status MesosExecutorDriver::start()
executorId.set_value(value.get());
// Get working directory from environment.
- value = os::getenv("MESOS_DIRECTORY");
+ value = env.get("MESOS_DIRECTORY");
if (value.isNone()) {
EXIT(EXIT_FAILURE)
<< "Expecting 'MESOS_DIRECTORY' to be set in the environment";
@@ -771,7 +791,7 @@ Status MesosExecutorDriver::start()
// (in contrast to the others above) for backwards
// compatibility: agents < 0.28.0 do not set it.
Duration shutdownGracePeriod = DEFAULT_EXECUTOR_SHUTDOWN_GRACE_PERIOD;
- value = os::getenv("MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD");
+ value = env.get("MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD");
if (value.isSome()) {
Try<Duration> parse = Duration::parse(value.get());
if (parse.isError()) {
@@ -784,14 +804,14 @@ Status MesosExecutorDriver::start()
}
// Get checkpointing status from environment.
- value = os::getenv("MESOS_CHECKPOINT");
+ value = env.get("MESOS_CHECKPOINT");
checkpoint = value.isSome() && value.get() == "1";
Duration recoveryTimeout = RECOVERY_TIMEOUT;
// Get the recovery timeout if checkpointing is enabled.
if (checkpoint) {
- value = os::getenv("MESOS_RECOVERY_TIMEOUT");
+ value = env.get("MESOS_RECOVERY_TIMEOUT");
if (value.isSome()) {
Try<Duration> parse = Duration::parse(value.get());