You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gr...@apache.org on 2020/04/07 01:49:51 UTC
[mesos] branch master updated (92f8768 -> 924a677)
This is an automated email from the ASF dual-hosted git repository.
grag pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 92f8768 Fixed build of tests broken in 9bd3ea4665402943af070e64327e8d7dc341e301.
new ad61259 Added agent-side validation for resource limits.
new be90edd Sent appropriate task status reason when task over memory request.
new 924a677 Updated CFS tests to avoid checking CPU usage.
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/common/protobuf_utils.cpp | 3 +-
.../mesos/isolators/cgroups/subsystems/memory.cpp | 24 +++++++-
src/slave/slave.cpp | 64 +++++++++++++++++++++-
src/slave/slave.hpp | 3 +
src/tests/containerizer/cgroups_isolator_tests.cpp | 59 ++------------------
src/tests/master_tests.cpp | 10 +++-
src/tests/master_validation_tests.cpp | 20 ++++++-
7 files changed, 120 insertions(+), 63 deletions(-)
[mesos] 03/03: Updated CFS tests to avoid checking CPU usage.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 924a6776ca2e474cc65fcb1d59938a0ff6ad46c8
Author: Greg Mann <gr...@mesosphere.io>
AuthorDate: Mon Apr 6 15:16:54 2020 -0700
Updated CFS tests to avoid checking CPU usage.
This verification is error-prone and leads to flaky test
failures. It is sufficient to verify that the cgroup values
are set correctly. We will avoid going so far as to confirm
that the kernel's scheduler is honoring those values.
Review: https://reviews.apache.org/r/72309/
---
src/tests/containerizer/cgroups_isolator_tests.cpp | 59 ++--------------------
1 file changed, 4 insertions(+), 55 deletions(-)
diff --git a/src/tests/containerizer/cgroups_isolator_tests.cpp b/src/tests/containerizer/cgroups_isolator_tests.cpp
index f4425f0..57158ae 100644
--- a/src/tests/containerizer/cgroups_isolator_tests.cpp
+++ b/src/tests/containerizer/cgroups_isolator_tests.cpp
@@ -386,8 +386,7 @@ TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_RevocableCpu)
// This test verifies that a task launched with 0.5 cpu and 32MB memory as its
// resource requests (but no resource limits specified) will have its CPU and
-// memory's soft & hard limits and OOM score adjustment set correctly, and it
-// cannot consume more cpu time than its CFS quota.
+// memory's soft & hard limits and OOM score adjustment set correctly.
TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_CFS_CommandTaskNoLimits)
{
Try<Owned<cluster::Master>> master = StartMaster();
@@ -436,17 +435,6 @@ TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_CFS_CommandTaskNoLimits)
AWAIT_READY(offers);
ASSERT_FALSE(offers->empty());
- // Generate random numbers to max out a single core. We'll run this
- // for 0.5 seconds of wall time so it should consume approximately
- // 300 ms of total cpu time when limited to 0.6 cpu. We use
- // /dev/urandom to prevent blocking on Linux when there's
- // insufficient entropy.
- string command =
- "cat /dev/urandom > /dev/null & "
- "export MESOS_TEST_PID=$! && "
- "sleep 0.5 && "
- "kill $MESOS_TEST_PID";
-
// We will launch a task with 0.5 cpu and 32MB memory, and the command
// executor will be given 0.1 cpu (`DEFAULT_EXECUTOR_CPUS`) and 32MB
// memory (DEFAULT_EXECUTOR_MEM) by default, so we need 0.6 cpu and 64MB
@@ -462,7 +450,7 @@ TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_CFS_CommandTaskNoLimits)
TaskInfo task = createTask(
offers.get()[0].slave_id(),
Resources::parse("cpus:0.5;mem:32").get(),
- command);
+ SLEEP_COMMAND(1000));
Future<TaskStatus> statusStarting;
Future<TaskStatus> statusRunning;
@@ -527,20 +515,6 @@ TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_CFS_CommandTaskNoLimits)
Try<int32_t> oomScoreAdj = numify<int32_t>(strings::trim(read.get()));
ASSERT_SOME_EQ(0, oomScoreAdj);
- Future<ResourceStatistics> usage = containerizer->usage(containerId);
- AWAIT_READY(usage);
-
- // Expect that no more than 400 ms of cpu time has been consumed. We
- // also check that at least 50 ms of cpu time has been consumed so
- // this test will fail if the host system is very heavily loaded.
- // This behavior is correct because under such conditions we aren't
- // actually testing the CFS cpu limiter.
- double cpuTime = usage->cpus_system_time_secs() +
- usage->cpus_user_time_secs();
-
- EXPECT_GE(0.4, cpuTime);
- EXPECT_LE(0.05, cpuTime);
-
driver.stop();
driver.join();
}
@@ -548,7 +522,7 @@ TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_CFS_CommandTaskNoLimits)
// This test verifies that a task launched with resource limits specified
// will have its CPU and memory's soft & hard limits and OOM score adjustment
-// set correctly, and it cannot consume more cpu time than its CFS quota.
+// set correctly.
TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_CFS_CommandTaskLimits)
{
Try<Owned<cluster::Master>> master = StartMaster();
@@ -603,17 +577,6 @@ TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_CFS_CommandTaskLimits)
AWAIT_READY(offers);
ASSERT_FALSE(offers->empty());
- // Generate random numbers to max out a single core. We'll run
- // this for 0.5 seconds of wall time so it should consume
- // approximately 300 ms of total cpu time when limited to 0.6
- // cpu. We use /dev/urandom to prevent blocking on Linux when
- // there's insufficient entropy.
- string command =
- "cat /dev/urandom > /dev/null & "
- "export MESOS_TEST_PID=$! && "
- "sleep 0.5 && "
- "kill $MESOS_TEST_PID";
-
// Launch a task with 0.2 cpu request, 0.5 cpu limit, half of
// host total memory - `DEFAULT_EXECUTOR_MEM` as memory request
// and half of host total memory as memory limit.
@@ -632,7 +595,7 @@ TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_CFS_CommandTaskLimits)
TaskInfo task = createTask(
offers.get()[0].slave_id(),
Resources::parse(resourceRequests).get(),
- command,
+ SLEEP_COMMAND(1000),
None(),
"test-task",
id::UUID::random().toString(),
@@ -718,20 +681,6 @@ TEST_F(CgroupsIsolatorTest, ROOT_CGROUPS_CFS_CommandTaskLimits)
EXPECT_GT(502, oomScoreAdj.get());
EXPECT_LT(498, oomScoreAdj.get());
- Future<ResourceStatistics> usage = containerizer->usage(containerId);
- AWAIT_READY(usage);
-
- // Expect that no more than 400 ms of cpu time has been consumed. We
- // also check that at least 50 ms of cpu time has been consumed so
- // this test will fail if the host system is very heavily loaded.
- // This behavior is correct because under such conditions we aren't
- // actually testing the CFS cpu limiter.
- double cpuTime = usage->cpus_system_time_secs() +
- usage->cpus_user_time_secs();
-
- EXPECT_GE(0.4, cpuTime);
- EXPECT_LE(0.05, cpuTime);
-
driver.stop();
driver.join();
}
[mesos] 01/03: Added agent-side validation for resource limits.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit ad612599df9e866b2a622baa4cdb659ece5c4574
Author: Greg Mann <gr...@mesosphere.io>
AuthorDate: Mon Apr 6 15:16:33 2020 -0700
Added agent-side validation for resource limits.
This prevents tasks from being launched on agents which would
not be capable of enforcing the specified limits.
Review: https://reviews.apache.org/r/72297/
---
src/slave/slave.cpp | 64 +++++++++++++++++++++++++++++++++--
src/slave/slave.hpp | 3 ++
src/tests/master_tests.cpp | 10 +++++-
src/tests/master_validation_tests.cpp | 20 +++++++++--
4 files changed, 91 insertions(+), 6 deletions(-)
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 6a48023..1a32c81 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -2168,6 +2168,34 @@ void Slave::runTask(
}
+Option<Error> Slave::validateResourceLimitsAndIsolators(
+ const vector<TaskInfo>& tasks)
+{
+ foreach (const TaskInfo& task, tasks) {
+ if (!(task.has_container() &&
+ task.container().type() == ContainerInfo::DOCKER)) {
+ if (task.limits().count("cpus") &&
+ !(strings::contains(flags.isolation, "cgroups/cpu") ||
+ strings::contains(flags.isolation, "cgroups/all"))) {
+ return Error(
+ "CPU limits can only be set on tasks launched in Mesos containers"
+ " when the agent has loaded the 'cgroups/cpu' isolator");
+ }
+
+ if (task.limits().count("mem") &&
+ !(strings::contains(flags.isolation, "cgroups/mem") ||
+ strings::contains(flags.isolation, "cgroups/all"))) {
+ return Error(
+ "Memory limits can only be set on tasks launched in Mesos"
+ " containers when the agent has loaded the 'cgroups/mem' isolator");
+ }
+ }
+ }
+
+ return None();
+}
+
+
void Slave::run(
const FrameworkInfo& frameworkInfo,
ExecutorInfo executorInfo,
@@ -2320,6 +2348,40 @@ void Slave::run(
}
}
+ CHECK_NOTNULL(framework);
+
+ Option<Error> error = validateResourceLimitsAndIsolators(tasks);
+ if (error.isSome()) {
+ // We report TASK_DROPPED to the framework because the task was
+ // never launched. For non-partition-aware frameworks, we report
+ // TASK_LOST for backward compatibility.
+ mesos::TaskState taskState = TASK_DROPPED;
+ if (!protobuf::frameworkHasCapability(
+ frameworkInfo, FrameworkInfo::Capability::PARTITION_AWARE)) {
+ taskState = TASK_LOST;
+ }
+
+ foreach (const TaskInfo& _task, tasks) {
+ const StatusUpdate update = protobuf::createStatusUpdate(
+ frameworkId,
+ info.id(),
+ _task.task_id(),
+ taskState,
+ TaskStatus::SOURCE_SLAVE,
+ id::UUID::random(),
+ error->message,
+ TaskStatus::REASON_GC_ERROR);
+
+ statusUpdate(update, UPID());
+ }
+
+ if (framework->idle()) {
+ removeFramework(framework);
+ }
+
+ return;
+ }
+
const ExecutorID& executorId = executorInfo.executor_id();
if (HookManager::hooksAvailable()) {
@@ -2342,8 +2404,6 @@ void Slave::run(
}
}
- CHECK_NOTNULL(framework);
-
// Track the pending task / task group to ensure the framework is
// not removed and the framework and top level executor directories
// are not scheduled for deletion before '_run()' is called.
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index d7e65e0..2cf45c6 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -158,6 +158,9 @@ public:
const process::UPID& from,
RunTaskMessage&& runTaskMessage);
+ Option<Error> validateResourceLimitsAndIsolators(
+ const std::vector<TaskInfo>& tasks);
+
// Made 'virtual' for Slave mocking.
virtual void runTask(
const process::UPID& from,
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 8cc8d20..785e5d5 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -4330,7 +4330,15 @@ TEST_F(MasterTest, TasksEndpoint)
TestContainerizer containerizer(&exec);
Owned<MasterDetector> detector = master.get()->createDetector();
- Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), &containerizer);
+
+ // We must enable the CPU and memory isolators on the agent so that it can
+ // accept resource limits.
+ slave::Flags flags = CreateSlaveFlags();
+ flags.isolation = "cgroups/cpu,cgroups/mem";
+
+ Try<Owned<cluster::Slave>> slave =
+ StartSlave(detector.get(), &containerizer, flags);
+
ASSERT_SOME(slave);
MockScheduler sched;
diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp
index 9efca42..816635a 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -5036,7 +5036,13 @@ TEST_F(TaskGroupValidationTest, ResourceLimits)
AWAIT_READY(offers);
ASSERT_FALSE(offers->empty());
- // Valid, no error.
+ // Erase the memory limit so we can be sure what error message to look for
+ // in the next test case.
+ task1.mutable_limits()->erase("mem");
+ task2.mutable_limits()->erase("mem");
+
+ // Error: the agent does not have any isolators loaded, so it can't enforce
+ // resource limits and does not launch the task group.
{
TaskGroupInfo taskGroup;
taskGroup.add_tasks()->CopyFrom(task1);
@@ -5061,10 +5067,18 @@ TEST_F(TaskGroupValidationTest, ResourceLimits)
driver.acceptOffers({offers->at(0).id()}, {operation}, filters);
AWAIT_READY(task1Status);
- EXPECT_EQ(TASK_STARTING, task1Status->state());
+ EXPECT_EQ(TASK_LOST, task1Status->state());
+ EXPECT_EQ(
+ "CPU limits can only be set on tasks launched in Mesos containers"
+ " when the agent has loaded the 'cgroups/cpu' isolator",
+ task1Status->message());
AWAIT_READY(task2Status);
- EXPECT_EQ(TASK_STARTING, task2Status->state());
+ EXPECT_EQ(TASK_LOST, task2Status->state());
+ EXPECT_EQ(
+ "CPU limits can only be set on tasks launched in Mesos containers"
+ " when the agent has loaded the 'cgroups/cpu' isolator",
+ task2Status->message());
}
}
[mesos] 02/03: Sent appropriate task status reason when task over
memory request.
Posted by gr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
grag pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit be90edd31a1833c5ed706b39f3a5547ae8153dd2
Author: Greg Mann <gr...@mesosphere.io>
AuthorDate: Mon Apr 6 15:16:45 2020 -0700
Sent appropriate task status reason when task over memory request.
Review: https://reviews.apache.org/r/72305/
---
src/common/protobuf_utils.cpp | 3 ++-
.../mesos/isolators/cgroups/subsystems/memory.cpp | 24 +++++++++++++++++++++-
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/src/common/protobuf_utils.cpp b/src/common/protobuf_utils.cpp
index 723d85a..8d1d5c4 100644
--- a/src/common/protobuf_utils.cpp
+++ b/src/common/protobuf_utils.cpp
@@ -254,7 +254,8 @@ StatusUpdate createStatusUpdate(
CHECK(
reason.get() == TaskStatus::REASON_CONTAINER_LIMITATION ||
reason.get() == TaskStatus::REASON_CONTAINER_LIMITATION_DISK ||
- reason.get() == TaskStatus::REASON_CONTAINER_LIMITATION_MEMORY)
+ reason.get() == TaskStatus::REASON_CONTAINER_LIMITATION_MEMORY ||
+ reason.get() == TaskStatus::REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED)
<< reason.get();
status->mutable_limitation()->mutable_resources()->CopyFrom(
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp b/src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp
index 15f87ba..60c7a89 100644
--- a/src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp
+++ b/src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp
@@ -699,11 +699,33 @@ void MemorySubsystemProcess::oomWaited(
? (double) usage->bytes() / Bytes::MEGABYTES : 0),
"*").get();
+ TaskStatus::Reason reason = TaskStatus::REASON_CONTAINER_LIMITATION_MEMORY;
+
+ // If the container has a hard limit set higher than the soft limit, then
+ // check if the memory usage is above the soft limit but less than the hard
+ // limit. If so, we send a task status reason to the scheduler which indicates
+ // that this container was preferentially OOM-killed because it exceeded its
+ // memory request without hitting its memory limit.
+ Try<Bytes> softLimit =
+ cgroups::memory::soft_limit_in_bytes(hierarchy, cgroup);
+
+ if (softLimit.isError()) {
+ LOG(ERROR) << "Failed to read 'memory.soft_limit_in_bytes': "
+ << softLimit.error();
+ } else if (softLimit.get() < limit.get()) {
+ if (!usage.isError() &&
+ !limit.isError() &&
+ usage.get() > softLimit.get() &&
+ usage.get() < limit.get()) {
+ reason = TaskStatus::REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED;
+ }
+ }
+
infos[containerId]->limitation.set(
protobuf::slave::createContainerLimitation(
mem,
message.str(),
- TaskStatus::REASON_CONTAINER_LIMITATION_MEMORY));
+ reason));
}