You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bb...@apache.org on 2020/01/24 13:27:12 UTC
[mesos] branch 1.9.x updated (4575c9b -> 298d1d7)
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a change to branch 1.9.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 4575c9b Added MESOS-9968 to the 1.9.1 CHANGELOG.
new 6035256 Decoupled detection of generated executors from Mesos install location.
new 631de3d Sync'd whether an executor was generated to and from disk.
new 298d1d7 Remembered whether an executor was agent-generated.
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/slave/paths.cpp | 15 ++++++++++++
src/slave/paths.hpp | 7 ++++++
src/slave/slave.cpp | 61 +++++++++++++++++++++++++++++-------------------
src/slave/slave.hpp | 14 +++++++----
src/slave/state.cpp | 39 +++++++++++++++++++++++++++++++
src/slave/state.hpp | 1 +
src/tests/mock_slave.cpp | 8 ++++---
src/tests/mock_slave.hpp | 8 ++++---
8 files changed, 119 insertions(+), 34 deletions(-)
[mesos] 01/03: Decoupled detection of generated executors from
Mesos install location.
Posted by bb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch 1.9.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 603525654d2aea5ed46c47a9da2ce2dce07188ab
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Thu Jan 23 14:19:43 2020 +0100
Decoupled detection of generated executors from Mesos install location.
We previously were detecting executors generated for command tasks by
checking whether their command matched the full path of
`mesos-executor`. This approach can lead to misdetection if e.g.,
during an upgrade a new installation location is choosen.
This patch adjusts the heuristic by now only relying on the fact that
the executor command should end in `mesos-executor`. In order to cut
down on false positives we now additionally check that the executor name
looks similar to the ones we generate for command tasks.
Review: https://reviews.apache.org/r/72033/
---
src/slave/slave.cpp | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 96890d3..5e3ae0d 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -10562,13 +10562,9 @@ Executor::Executor(
// hacky. We rely on the fact that docker executor launch command is
// set in the docker containerizer so that this check is still valid
// in the slave.
- Result<string> executorPath =
- os::realpath(path::join(slave->flags.launcher_dir, MESOS_EXECUTOR));
-
- if (executorPath.isSome()) {
- isGeneratedForCommandTask_ =
- strings::contains(info.command().value(), executorPath.get());
- }
+ isGeneratedForCommandTask_ =
+ strings::endsWith(info.command().value(), MESOS_EXECUTOR) &&
+ strings::startsWith(info.name(), "Command Executor");
}
[mesos] 02/03: Sync'd whether an executor was generated to and from
disk.
Posted by bb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch 1.9.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 631de3d4df9bd8e24edc6687c544d5bb45be0198
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Thu Jan 23 14:19:51 2020 +0100
Sync'd whether an executor was generated to and from disk.
This patch introduces an `ExecutorState` variable signifying whether an
executor was generated by the agent (and is thus unknown to the master).
Currently we still detect agent-generated executors with a heuristic,
but will adapt that heuristic in a follow-up path making us of the
additional state we now persist.
Review: https://reviews.apache.org/r/72034/
---
src/slave/paths.cpp | 15 +++++++++++++++
src/slave/paths.hpp | 7 +++++++
src/slave/slave.cpp | 10 ++++++++++
src/slave/state.cpp | 39 +++++++++++++++++++++++++++++++++++++++
src/slave/state.hpp | 1 +
5 files changed, 72 insertions(+)
diff --git a/src/slave/paths.cpp b/src/slave/paths.cpp
index 5afcc7e..1f3dd72 100644
--- a/src/slave/paths.cpp
+++ b/src/slave/paths.cpp
@@ -301,6 +301,21 @@ string getExecutorRunPath(
}
+string getExecutorGeneratedForCommandTaskPath(
+ const string& rootDir,
+ const SlaveID& slaveId,
+ const FrameworkID& frameworkId,
+ const ExecutorID& executorId)
+{
+ constexpr char EXECUTOR_GENERATED_FOR_COMMAND_TASK_PATH[] =
+ "executor_generated_for_command_task";
+
+ return path::join(
+ getExecutorPath(rootDir, slaveId, frameworkId, executorId),
+ EXECUTOR_GENERATED_FOR_COMMAND_TASK_PATH);
+}
+
+
string getExecutorHttpMarkerPath(
const string& rootDir,
const SlaveID& slaveId,
diff --git a/src/slave/paths.hpp b/src/slave/paths.hpp
index d7fcb13..7d43e9c 100644
--- a/src/slave/paths.hpp
+++ b/src/slave/paths.hpp
@@ -239,6 +239,13 @@ std::string getExecutorRunPath(
const ContainerID& containerId);
+std::string getExecutorGeneratedForCommandTaskPath(
+ const std::string& rootDir,
+ const SlaveID& slaveId,
+ const FrameworkID& frameworkId,
+ const ExecutorID& executorId);
+
+
std::string getExecutorHttpMarkerPath(
const std::string& rootDir,
const SlaveID& slaveId,
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 5e3ae0d..245fd71 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -10562,6 +10562,7 @@ Executor::Executor(
// hacky. We rely on the fact that docker executor launch command is
// set in the docker containerizer so that this check is still valid
// in the slave.
+ // TODO(bbannier): Initialize this field with a value passed to constructor.
isGeneratedForCommandTask_ =
strings::endsWith(info.command().value(), MESOS_EXECUTOR) &&
strings::startsWith(info.name(), "Command Executor");
@@ -10712,6 +10713,15 @@ void Executor::checkpointExecutor()
CHECK_SOME(state::checkpoint(path, info));
+ // Sync state of sentinel indicating whether the executor was
+ // generated by the agent.
+ CHECK_SOME(state::checkpoint(
+ paths::getExecutorGeneratedForCommandTaskPath(
+ slave->metaDir, slave->info.id(), frameworkId, id),
+ stringify(static_cast<int>(isGeneratedForCommandTask_)),
+ true));
+
+
// Create the meta executor directory.
// NOTE: This creates the 'latest' symlink in the meta directory.
Try<string> mkdir = paths::createExecutorDirectory(
diff --git a/src/slave/state.cpp b/src/slave/state.cpp
index 821132b..96f22b3 100644
--- a/src/slave/state.cpp
+++ b/src/slave/state.cpp
@@ -48,6 +48,7 @@
#include "messages/messages.hpp"
+#include "slave/constants.hpp"
#include "slave/paths.hpp"
#include "slave/state.hpp"
#include "slave/state.pb.h"
@@ -497,6 +498,44 @@ Try<ExecutorState> ExecutorState::recover(
state.info = executorInfo.get();
+ const string executorGeneratedForCommandTaskPath =
+ paths::getExecutorGeneratedForCommandTaskPath(
+ rootDir, slaveId, frameworkId, executorId);
+
+ if (os::exists(executorGeneratedForCommandTaskPath)) {
+ Try<string> read = os::read(executorGeneratedForCommandTaskPath);
+
+ if (read.isError()) {
+ return Error(
+ "Could not read '" + executorGeneratedForCommandTaskPath + "': " +
+ read.error());
+ }
+
+ Try<int> generatedForCommandTask = numify<int>(*read);
+
+ if (generatedForCommandTask.isError()) {
+ return Error(
+ "Could not parse '" + executorGeneratedForCommandTaskPath + "': " +
+ generatedForCommandTask.error());
+ }
+
+ state.generatedForCommandTask = *generatedForCommandTask;
+ } else {
+ // If we did not find persisted information on whether this executor was
+ // generated by the agent, use a heuristic.
+ //
+ // TODO(bbannier): Remove this code once we do not need to support
+ // versions anymore which do not persist this information.
+ // TODO(jieyu): The way we determine if an executor is generated for
+ // a command task (either command or docker executor) is really
+ // hacky. We rely on the fact that docker executor launch command is
+ // set in the docker containerizer so that this check is still valid
+ // in the slave.
+ state.generatedForCommandTask =
+ strings::endsWith(executorInfo->command().value(), MESOS_EXECUTOR) &&
+ strings::startsWith(executorInfo->name(), "Command Executor");
+ }
+
return state;
}
diff --git a/src/slave/state.hpp b/src/slave/state.hpp
index 6d6ae01..e80aac5 100644
--- a/src/slave/state.hpp
+++ b/src/slave/state.hpp
@@ -333,6 +333,7 @@ struct ExecutorState
Option<ContainerID> latest;
hashmap<ContainerID, RunState> runs;
unsigned int errors;
+ bool generatedForCommandTask = false;
};
[mesos] 03/03: Remembered whether an executor was agent-generated.
Posted by bb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch 1.9.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 298d1d70e5a4aa383650ce3c81f9db64aaded2d4
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Thu Jan 23 14:19:57 2020 +0100
Remembered whether an executor was agent-generated.
This patch adds code to pass on whether was generated in the agent from
the point where the executor is generated to the point where we create
an actual `slave::Executor` instance. This allows us to persist this
information in the executor state.
Review: https://reviews.apache.org/r/72035/
---
src/slave/slave.cpp | 49 +++++++++++++++++++++++++++---------------------
src/slave/slave.hpp | 14 ++++++++++----
src/tests/mock_slave.cpp | 8 +++++---
src/tests/mock_slave.hpp | 8 +++++---
4 files changed, 48 insertions(+), 31 deletions(-)
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 245fd71..2c2b5a9 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -2047,13 +2047,16 @@ void Slave::runTask(
const ExecutorInfo executorInfo = getExecutorInfo(frameworkInfo, task);
+ bool executorGeneratedForCommandTask = !task.has_executor();
+
run(frameworkInfo,
executorInfo,
task,
None(),
resourceVersionUuids,
pid,
- launchExecutor);
+ launchExecutor,
+ executorGeneratedForCommandTask);
}
@@ -2064,7 +2067,8 @@ void Slave::run(
Option<TaskGroupInfo> taskGroup,
const vector<ResourceVersionUUID>& resourceVersionUuids,
const UPID& pid,
- const Option<bool>& launchExecutor)
+ const Option<bool>& launchExecutor,
+ bool executorGeneratedForCommandTask)
{
CHECK_NE(task.isSome(), taskGroup.isSome())
<< "Either task or task group should be set but not both";
@@ -2382,7 +2386,8 @@ void Slave::run(
task,
taskGroup,
resourceVersionUuids,
- launchExecutor))
+ launchExecutor,
+ executorGeneratedForCommandTask))
.onFailed(defer(self(), [=](const string& failure) {
Framework* _framework = getFramework(frameworkId);
if (_framework == nullptr) {
@@ -2612,7 +2617,8 @@ void Slave::__run(
const Option<TaskInfo>& task,
const Option<TaskGroupInfo>& taskGroup,
const vector<ResourceVersionUUID>& resourceVersionUuids,
- const Option<bool>& launchExecutor)
+ const Option<bool>& launchExecutor,
+ bool executorGeneratedForCommandTask)
{
CHECK_NE(task.isSome(), taskGroup.isSome())
<< "Either task or task group should be set but not both";
@@ -3064,7 +3070,8 @@ void Slave::__run(
// or we are in the legacy case of launching one if there wasn't
// one already. Either way, let's launch executor now.
if (executor == nullptr) {
- Try<Executor*> added = framework->addExecutor(executorInfo);
+ Try<Executor*> added =
+ framework->addExecutor(executorInfo, executorGeneratedForCommandTask);
if (added.isError()) {
CHECK(framework->getExecutor(executorId) == nullptr);
@@ -3729,13 +3736,17 @@ void Slave::runTaskGroup(
return;
}
+ // Executors for task groups are injected by the master, not the agent.
+ constexpr bool executorGeneratedForCommandTask = false;
+
run(frameworkInfo,
executorInfo,
None(),
taskGroupInfo,
resourceVersionUuids,
UPID(),
- launchExecutor);
+ launchExecutor,
+ executorGeneratedForCommandTask);
}
@@ -9933,7 +9944,9 @@ void Framework::checkpointFramework() const
}
-Try<Executor*> Framework::addExecutor(const ExecutorInfo& executorInfo)
+Try<Executor*> Framework::addExecutor(
+ const ExecutorInfo& executorInfo,
+ bool isGeneratedForCommandTask)
{
// Verify that Resource.AllocationInfo is set, if coming
// from a MULTI_ROLE master this will be set, otherwise
@@ -9989,7 +10002,8 @@ Try<Executor*> Framework::addExecutor(const ExecutorInfo& executorInfo)
containerId,
directory.get(),
user,
- info.checkpoint());
+ info.checkpoint(),
+ isGeneratedForCommandTask);
if (executor->checkpoint) {
executor->checkpointExecutor();
@@ -10184,7 +10198,8 @@ void Framework::recoverExecutor(
latest,
directory,
info.user(),
- info.checkpoint());
+ info.checkpoint(),
+ state.generatedForCommandTask);
// Recover the libprocess PID if possible for PID based executors.
if (run->http.isSome()) {
@@ -10528,7 +10543,8 @@ Executor::Executor(
const ContainerID& _containerId,
const string& _directory,
const Option<string>& _user,
- bool _checkpoint)
+ bool _checkpoint,
+ bool isGeneratedForCommandTask)
: state(REGISTERING),
slave(_slave),
id(_info.executor_id()),
@@ -10539,7 +10555,8 @@ Executor::Executor(
user(_user),
checkpoint(_checkpoint),
http(None()),
- pid(None())
+ pid(None()),
+ isGeneratedForCommandTask_(isGeneratedForCommandTask)
{
CHECK_NOTNULL(slave);
@@ -10556,16 +10573,6 @@ Executor::Executor(
completedTasks =
circular_buffer<shared_ptr<Task>>(MAX_COMPLETED_TASKS_PER_EXECUTOR);
-
- // TODO(jieyu): The way we determine if an executor is generated for
- // a command task (either command or docker executor) is really
- // hacky. We rely on the fact that docker executor launch command is
- // set in the docker containerizer so that this check is still valid
- // in the slave.
- // TODO(bbannier): Initialize this field with a value passed to constructor.
- isGeneratedForCommandTask_ =
- strings::endsWith(info.command().value(), MESOS_EXECUTOR) &&
- strings::startsWith(info.name(), "Command Executor");
}
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 77b5bc0..b281714 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -172,7 +172,8 @@ public:
Option<TaskGroupInfo> taskGroup,
const std::vector<ResourceVersionUUID>& resourceVersionUuids,
const process::UPID& pid,
- const Option<bool>& launchExecutor);
+ const Option<bool>& launchExecutor,
+ bool executorGeneratedForCommandTask);
// Made 'virtual' for Slave mocking.
//
@@ -196,7 +197,8 @@ public:
const Option<TaskInfo>& task,
const Option<TaskGroupInfo>& taskGroup,
const std::vector<ResourceVersionUUID>& resourceVersionUuids,
- const Option<bool>& launchExecutor);
+ const Option<bool>& launchExecutor,
+ bool executorGeneratedForCommandTask);
// This is called when the resource limits of the container have
// been updated for the given tasks and task groups. If the update is
@@ -938,7 +940,8 @@ public:
const ContainerID& containerId,
const std::string& directory,
const Option<std::string>& user,
- bool checkpoint);
+ bool checkpoint,
+ bool isGeneratedForCommandTask);
~Executor();
@@ -1138,7 +1141,10 @@ public:
const FrameworkID id() const { return info.id(); }
- Try<Executor*> addExecutor(const ExecutorInfo& executorInfo);
+ Try<Executor*> addExecutor(
+ const ExecutorInfo& executorInfo,
+ bool isGeneratedForCommandTask);
+
Executor* getExecutor(const ExecutorID& executorId) const;
Executor* getExecutor(const TaskID& taskId) const;
diff --git a/src/tests/mock_slave.cpp b/src/tests/mock_slave.cpp
index 71be957..2c9e26f 100644
--- a/src/tests/mock_slave.cpp
+++ b/src/tests/mock_slave.cpp
@@ -133,7 +133,7 @@ MockSlave::MockSlave(
.WillRepeatedly(Invoke(this, &MockSlave::unmocked_runTask));
EXPECT_CALL(*this, _run(_, _, _, _, _, _))
.WillRepeatedly(Invoke(this, &MockSlave::unmocked__run));
- EXPECT_CALL(*this, __run(_, _, _, _, _, _))
+ EXPECT_CALL(*this, __run(_, _, _, _, _, _, _))
.WillRepeatedly(Invoke(this, &MockSlave::unmocked___run));
EXPECT_CALL(*this, runTaskGroup(_, _, _, _, _, _))
.WillRepeatedly(Invoke(this, &MockSlave::unmocked_runTaskGroup));
@@ -222,7 +222,8 @@ void MockSlave::unmocked___run(
const Option<TaskInfo>& task,
const Option<TaskGroupInfo>& taskGroup,
const std::vector<ResourceVersionUUID>& resourceVersionUuids,
- const Option<bool>& launchExecutor)
+ const Option<bool>& launchExecutor,
+ bool executorGeneratedForCommandTask)
{
slave::Slave::__run(
frameworkInfo,
@@ -230,7 +231,8 @@ void MockSlave::unmocked___run(
task,
taskGroup,
resourceVersionUuids,
- launchExecutor);
+ launchExecutor,
+ executorGeneratedForCommandTask);
}
diff --git a/src/tests/mock_slave.hpp b/src/tests/mock_slave.hpp
index eb31a0f..58daefa 100644
--- a/src/tests/mock_slave.hpp
+++ b/src/tests/mock_slave.hpp
@@ -153,13 +153,14 @@ public:
const std::vector<ResourceVersionUUID>& resourceVersionUuids,
const Option<bool>& launchExecutor);
- MOCK_METHOD6(__run, void(
+ MOCK_METHOD7(__run, void(
const FrameworkInfo& frameworkInfo,
const ExecutorInfo& executorInfo,
const Option<TaskInfo>& task,
const Option<TaskGroupInfo>& taskGroup,
const std::vector<ResourceVersionUUID>& resourceVersionUuids,
- const Option<bool>& launchExecutor));
+ const Option<bool>& launchExecutor,
+ bool executorGeneratedForCommandTask));
void unmocked___run(
const FrameworkInfo& frameworkInfo,
@@ -167,7 +168,8 @@ public:
const Option<TaskInfo>& task,
const Option<TaskGroupInfo>& taskGroup,
const std::vector<ResourceVersionUUID>& resourceVersionUuids,
- const Option<bool>& launchExecutor);
+ const Option<bool>& launchExecutor,
+ bool executorGeneratedForCommandTask);
MOCK_METHOD6(runTaskGroup, void(
const process::UPID& from,