bmahler commented on code in PR #554:
URL: https://github.com/apache/mesos/pull/554#discussion_r1566284559
##########
src/linux/cgroups2.cpp:
##########
@@ -786,6 +787,23 @@ Try<Bytes> usage(const string& cgroup)
return Bytes(*contents);
}
+
+Try<Nothing> set_min(const string& cgroup, const Bytes& bytes)
+{
+ return cgroups2::write(cgroup, control::MIN, stringify(bytes.bytes()));
Review Comment:
we already have a write that takes uint64_t?
##########
src/linux/cgroups2.cpp:
##########
@@ -786,6 +787,23 @@ Try<Bytes> usage(const string& cgroup)
return Bytes(*contents);
}
+
+Try<Nothing> set_min(const string& cgroup, const Bytes& bytes)
+{
+ return cgroups2::write(cgroup, control::MIN, stringify(bytes.bytes()));
+}
+
+
+Try<Bytes> min(const string& cgroup)
+{
+ Try<string> contents = cgroups2::read<string>(cgroup, control::MIN);
+ if (contents.isError()) {
+ return Error("Failed to read 'memory.min': " + contents.error());
+ }
+
+ return Bytes::parse(strings::trim(*contents) + "B");
Review Comment:
no need to parse with a B here, can just cgroups::read a uint64_t and
directly construct Bytes with it?
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -325,6 +326,52 @@ TEST_F(Cgroups2Test, ROOT_CGROUPS2_MemoryUsage)
}
+TEST_F(Cgroups2Test, ROOT_CGROUPS2_MemoryMinimum)
+{
+ ASSERT_SOME(enable_controllers({"memory"}));
+
+ ASSERT_SOME(cgroups2::create(TEST_CGROUP));
+ ASSERT_SOME(cgroups2::controllers::enable(TEST_CGROUP, {"memory"}));
+
+ const Bytes bytes = Bytes(os::pagesize()) * 5;
+
+ // Does not exist for the root cgroup.
+ EXPECT_ERROR(cgroups2::memory::min(cgroups2::ROOT_CGROUP));
+ EXPECT_ERROR(cgroups2::memory::set_min(cgroups2::ROOT_CGROUP, bytes));
+
+ EXPECT_SOME(cgroups2::memory::set_min(TEST_CGROUP, bytes));
+ EXPECT_SOME_EQ(bytes, cgroups2::memory::min(TEST_CGROUP));
+}
+
+
+// Check that byte amounts written to the memory controller are rounded
+// down to the nearest page size.
+TEST_F(Cgroups2Test, ROOT_CGROUPS2_MemoryBytesRounding)
Review Comment:
is the plan to add other memory setters in here, hence not calling this a
Minimum test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@mesos.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
