bmahler commented on code in PR #554:
URL: https://github.com/apache/mesos/pull/554#discussion_r1566127015
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -52,6 +54,13 @@ namespace mesos {
namespace internal {
namespace tests {
+// System page size.
+//
+// Byte amounts written to the memory controller that are not aligned with the
+// system page size will be rounded up when read. Therefore, for simplicity,
+// we only test with multiples of the page size.
+const long PAGE_SIZE = sysconf(_SC_PAGE_SIZE);
Review Comment:
no need to store a constant for this, and let's use os/pagesize.hpp inline
for this:
```
Bytes(os::pagesize()) * 5
```
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -325,6 +334,24 @@ 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(5 * PAGE_SIZE);
Review Comment:
see above comment (multiplication can be moved outside the bytes here
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -52,6 +54,13 @@ namespace mesos {
namespace internal {
namespace tests {
+// System page size.
+//
+// Byte amounts written to the memory controller that are not aligned with the
+// system page size will be rounded up when read. Therefore, for simplicity,
Review Comment:
might be good to have the test just show what happens directly here:
1. Test a value that is not page size aligned (gets rounded down despite the
docs saying rounded up)
2. Test a page size aligned value, comes back the same
##########
src/linux/cgroups2.hpp:
##########
@@ -220,6 +220,19 @@ namespace memory {
// Current memory usage of a cgroup and its descendants in bytes.
Try<Bytes> usage(const std::string& cgroup);
+
+// Set the minimum memory that is guaranteed to not be reclaimed under any
+// conditions. If the value is larger than the parent cgroup's value, the
+// parent's value is the effective value.
Review Comment:
you mentioned that this depends on a configuration or something? would be
good to spell out this subtlety
could also be more clear about what effective value means here (i.e.
effective seems to suggest that you can actually set it higher but the parent's
will effectively limit it before that, as opposed to the value you set getting
rounded down to the parent's value)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
