Deflake cpuacct cgroup test.

- Cgroup::PollControlFileForChange() is only used by cpuacct tests to check
  that CPU usage for the test's containing cgroups increases over time. In this
  context, sleeping between checks is counterproductive because the test uses
  no CPU while sleeping. Remove the sleep.

- Don't assume that the root cgroup's usage is initially non-zero, due to
  granularity issues.

PiperOrigin-RevId: 611594418
This commit is contained in:
Jamie Liu
2024-02-29 14:24:07 -08:00
committed by gVisor bot
parent 71212d503f
commit a88e82fa4a
2 changed files with 17 additions and 16 deletions
+13 -10
View File
@@ -550,8 +550,11 @@ TEST(CPUAcctCgroup, HierarchicalAccounting) {
Cgroup root = Cgroup::RootCgroup("/sys/fs/cgroup/cpuacct");
Cgroup child = ASSERT_NO_ERRNO_AND_VALUE(root.CreateChild("child1"));
// Root should have non-zero CPU usage since the test itself will be running
// in the root cgroup.
// The test starts in the root cgroup, so its CPU usage should be accounted
// there. Since the granularity of cpuacct.usage is unspecified and the test
// may not have run for very long yet, wait for it to be accounted.
ASSERT_NO_ERRNO(
root.PollControlFileForChange("cpuacct.usage", absl::Seconds(5)));
EXPECT_THAT(root.ReadIntegerControlFile("cpuacct.usage"),
IsPosixErrorOkAndHolds(Gt(0)));
@@ -564,7 +567,7 @@ TEST(CPUAcctCgroup, HierarchicalAccounting) {
ASSERT_NO_ERRNO_AND_VALUE(root.ReadIntegerControlFile("cpuacct.usage"));
ASSERT_NO_ERRNO(child.Enter(getpid()));
ASSERT_NO_ERRNO(
child.PollControlFileForChange("cpuacct.usage", absl::Seconds(60)));
child.PollControlFileForChange("cpuacct.usage", absl::Seconds(5)));
EXPECT_THAT(child.ReadIntegerControlFile("cpuacct.usage"),
IsPosixErrorOkAndHolds(Gt(0)));
@@ -577,7 +580,7 @@ TEST(CPUAcctCgroup, HierarchicalAccounting) {
// Root should continue to gain usage after the move since child is a
// subcgroup.
ASSERT_NO_ERRNO(
child.PollControlFileForChange("cpuacct.usage", absl::Seconds(60)));
child.PollControlFileForChange("cpuacct.usage", absl::Seconds(5)));
EXPECT_THAT(root.ReadIntegerControlFile("cpuacct.usage"),
IsPosixErrorOkAndHolds(Ge(after_move)));
}
@@ -592,7 +595,7 @@ TEST(CPUAcctCgroup, IndirectCharge) {
ASSERT_NO_ERRNO(child1.Enter(getpid()));
ASSERT_NO_ERRNO(
child1.PollControlFileForChange("cpuacct.usage", absl::Seconds(60)));
child1.PollControlFileForChange("cpuacct.usage", absl::Seconds(5)));
// Only root and child1 should have usage.
for (auto const& cg : {root, child1}) {
@@ -606,7 +609,7 @@ TEST(CPUAcctCgroup, IndirectCharge) {
ASSERT_NO_ERRNO(child2a.Enter(getpid()));
ASSERT_NO_ERRNO(
child2a.PollControlFileForChange("cpuacct.usage", absl::Seconds(60)));
child2a.PollControlFileForChange("cpuacct.usage", absl::Seconds(5)));
const int64_t snapshot_root =
ASSERT_NO_ERRNO_AND_VALUE(root.ReadIntegerControlFile("cpuacct.usage"));
@@ -618,7 +621,7 @@ TEST(CPUAcctCgroup, IndirectCharge) {
child2a.ReadIntegerControlFile("cpuacct.usage"));
ASSERT_NO_ERRNO(
child2a.PollControlFileForChange("cpuacct.usage", absl::Seconds(60)));
child2a.PollControlFileForChange("cpuacct.usage", absl::Seconds(5)));
// Root, child2 and child2a should've accumulated new usage. Child1 should
// not.
@@ -647,15 +650,15 @@ TEST(CPUAcctCgroup, NoDoubleAccounting) {
ASSERT_NO_ERRNO(a.Enter(getpid()));
ASSERT_NO_ERRNO(
a.PollControlFileForChange("cpuacct.usage", absl::Seconds(60)));
a.PollControlFileForChange("cpuacct.usage", absl::Seconds(5)));
ASSERT_NO_ERRNO(b.Enter(getpid()));
ASSERT_NO_ERRNO(
b.PollControlFileForChange("cpuacct.usage", absl::Seconds(60)));
b.PollControlFileForChange("cpuacct.usage", absl::Seconds(5)));
ASSERT_NO_ERRNO(root.Enter(getpid()));
ASSERT_NO_ERRNO(
root.PollControlFileForChange("cpuacct.usage", absl::Seconds(60)));
root.PollControlFileForChange("cpuacct.usage", absl::Seconds(5)));
// The usage for parent, a & b should now be frozen, since they no longer have
// any tasks. Root will continue to accumulate usage.
+4 -6
View File
@@ -93,7 +93,6 @@ PosixError Cgroup::PollControlFileForChange(absl::string_view name,
PosixError Cgroup::PollControlFileForChangeAfter(
absl::string_view name, absl::Duration timeout,
std::function<void()> body) const {
const absl::Duration poll_interval = absl::Milliseconds(10);
const absl::Time deadline = absl::Now() + timeout;
const std::string alias_path = absl::StrFormat("[cg#%d]/%s", id_, name);
@@ -107,6 +106,10 @@ PosixError Cgroup::PollControlFileForChangeAfter(
// resource usage this function is often waiting for, resulting in timeouts.
const DisableSave ds;
std::cerr << absl::StreamFormat(
"Waiting for control file '%s' to change from '%d'...",
alias_path, initial_value)
<< std::endl;
while (true) {
ASSIGN_OR_RETURN_ERRNO(const int64_t current_value,
ReadIntegerControlFile(name));
@@ -121,11 +124,6 @@ PosixError Cgroup::PollControlFileForChangeAfter(
return PosixError(ETIME, absl::StrCat(alias_path, " didn't change in ",
absl::FormatDuration(timeout)));
}
std::cerr << absl::StreamFormat(
"Waiting for control file '%s' to change from '%d'...",
alias_path, initial_value)
<< std::endl;
absl::SleepFor(poll_interval);
}
}