Introduce new flag --experimental_worker_use_cgroups_on_linux that uses cgroups to track memory usage for singleplex workers (on Linux) without imposing any hard limits.
TODOs:
- Figure out how to get around cases where Blaze is put automatically put in a root cgroup as a consequence of the session it was run from (e.g. ssh). One possibility is using `systemd-run`.
RELNOTES: Introduce new flag --experimental_worker_use_cgroups_on_linux that uses cgroups to track memory usage for singleplex workers (on Linux).
PiperOrigin-RevId: 605270861
Change-Id: I6c6cffb0568de7d4ccfbc4624c579bc3d64fadb4
diff --git a/.bazelci/postsubmit.yml b/.bazelci/postsubmit.yml
index 7c0b70b..7f6b4b1 100644
--- a/.bazelci/postsubmit.yml
+++ b/.bazelci/postsubmit.yml
@@ -193,6 +193,8 @@
# MacOS does not have cgroups so it can't support hardened sandbox
- "-//src/test/java/com/google/devtools/build/lib/sandbox:CgroupsInfoTest"
- "-//src/test/shell/integration:bazel_hardened_sandboxed_worker_test"
+ - "-//src/test/shell/integration:bazel_sandboxed_worker_with_cgroups_test"
+ - "-//src/test/shell/integration:bazel_worker_with_cgroups_test"
# https://github.com/bazelbuild/bazel/issues/16526
- "-//src/test/shell/bazel:starlark_repository_test"
# https://github.com/bazelbuild/bazel/issues/17407
@@ -252,6 +254,8 @@
# MacOS does not have cgroups so it can't support hardened sandbox
- "-//src/test/java/com/google/devtools/build/lib/sandbox:CgroupsInfoTest"
- "-//src/test/shell/integration:bazel_hardened_sandboxed_worker_test"
+ - "-//src/test/shell/integration:bazel_sandboxed_worker_with_cgroups_test"
+ - "-//src/test/shell/integration:bazel_worker_with_cgroups_test"
# https://github.com/bazelbuild/bazel/issues/16525
- "-//src/test/java/com/google/devtools/build/lib/buildtool:KeepGoingTest"
- "-//src/test/java/com/google/devtools/build/lib/buildtool:DanglingSymlinkTest"
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index e13c4f1..6e69075 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -196,6 +196,8 @@
# MacOS does not have cgroups so it can't support hardened sandbox
- "-//src/test/java/com/google/devtools/build/lib/sandbox:CgroupsInfoTest"
- "-//src/test/shell/integration:bazel_hardened_sandboxed_worker_test"
+ - "-//src/test/shell/integration:bazel_sandboxed_worker_with_cgroups_test"
+ - "-//src/test/shell/integration:bazel_worker_with_cgroups_test"
# https://github.com/bazelbuild/bazel/issues/16526
- "-//src/test/shell/bazel:starlark_repository_test"
# https://github.com/bazelbuild/bazel/issues/17407
@@ -256,6 +258,8 @@
# MacOS does not have cgroups so it can't support hardened sandbox
- "-//src/test/java/com/google/devtools/build/lib/sandbox:CgroupsInfoTest"
- "-//src/test/shell/integration:bazel_hardened_sandboxed_worker_test"
+ - "-//src/test/shell/integration:bazel_sandboxed_worker_with_cgroups_test"
+ - "-//src/test/shell/integration:bazel_worker_with_cgroups_test"
# https://github.com/bazelbuild/bazel/issues/16525
- "-//src/test/java/com/google/devtools/build/lib/buildtool:KeepGoingTest"
- "-//src/test/java/com/google/devtools/build/lib/buildtool:DanglingSymlinkTest"
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
index f4079d9..5077a3b 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
@@ -329,8 +329,9 @@
CgroupsInfo.getBlazeSpawnsCgroup()
.createIndividualSpawnCgroup(
"sandbox_" + context.getId(), sandboxOptions.memoryLimitMb);
- cgroupsDir = sandboxCgroup.getCgroupDir().toString();
- commandLineBuilder.setCgroupsDir(cgroupsDir);
+ if (sandboxCgroup.exists()) {
+ commandLineBuilder.setCgroupsDir(sandboxCgroup.getCgroupDir().toString());
+ }
}
if (useHermeticTmp) {
diff --git a/src/main/java/com/google/devtools/build/lib/worker/BUILD b/src/main/java/com/google/devtools/build/lib/worker/BUILD
index 13ff305..aa88375 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/worker/BUILD
@@ -106,6 +106,7 @@
"//src/main/java/com/google/devtools/build/lib/exec:tree_deleter",
"//src/main/java/com/google/devtools/build/lib/exec/local",
"//src/main/java/com/google/devtools/build/lib/runtime/commands/events",
+ "//src/main/java/com/google/devtools/build/lib/sandbox:cgroups_info",
"//src/main/java/com/google/devtools/build/lib/sandbox:linux_sandbox_util",
"//src/main/java/com/google/devtools/build/lib/sandbox:sandbox_helpers",
"//src/main/java/com/google/devtools/build/lib/sandbox:sandbox_options",
@@ -159,8 +160,10 @@
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/metrics:resource_collector",
+ "//src/main/java/com/google/devtools/build/lib/sandbox:cgroups_info",
"//src/main/java/com/google/devtools/build/lib/util:os",
"//third_party:guava",
+ "//third_party:jsr305",
],
)
@@ -186,6 +189,7 @@
":singleplex_worker",
":worker",
":worker_key",
+ ":worker_options",
":worker_process_status",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/exec:tree_deleter",
@@ -237,11 +241,13 @@
":worker_process_status",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/events",
+ "//src/main/java/com/google/devtools/build/lib/sandbox:cgroups_info",
"//src/main/java/com/google/devtools/build/lib/sandbox:sandbox_helpers",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/protobuf:worker_protocol_java_proto",
"//third_party:guava",
+ "//third_party:jsr305",
],
)
@@ -267,6 +273,7 @@
":worker",
":worker_exec_root",
":worker_key",
+ ":worker_options",
":worker_process_status",
":worker_protocol",
"//src/main/java/com/google/devtools/build/lib/actions",
diff --git a/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java b/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java
index cc056fe..b4d5f9e 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java
@@ -37,7 +37,6 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
-import java.io.File;
import java.io.IOException;
import java.util.Map.Entry;
import java.util.Set;
@@ -95,10 +94,9 @@
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
private final WorkerExecRoot workerExecRoot;
+
/** Options specific to hardened sandbox, null if not using that. */
@Nullable private final WorkerSandboxOptions hardenedSandboxOptions;
- /** If non-null, a directory that allows cgroup control. */
- private String cgroupsDir;
private Path inaccessibleHelperDir;
private Path inaccessibleHelperFile;
@@ -109,9 +107,10 @@
int workerId,
Path workDir,
Path logFile,
+ WorkerOptions workerOptions,
@Nullable WorkerSandboxOptions hardenedSandboxOptions,
TreeDeleter treeDeleter) {
- super(workerKey, workerId, workDir, logFile);
+ super(workerKey, workerId, workDir, logFile, workerOptions);
this.workerExecRoot =
new WorkerExecRoot(
workDir,
@@ -177,6 +176,18 @@
@Override
protected Subprocess createProcess() throws IOException, UserExecException {
ImmutableList<String> args = makeExecPathAbsolute(workerKey.getArgs());
+
+ // We put the sandbox inside a unique subdirectory using the worker's ID.
+ if (options.useCgroupsOnLinux || hardenedSandboxOptions != null) {
+ // In the event that the memory limit is 0, we defer to using Blaze's WorkerLifecycleManager
+ // to kill workers rather than cgroup's OOM killer.
+ cgroup =
+ CgroupsInfo.getBlazeSpawnsCgroup()
+ .createIndividualSpawnCgroup(
+ "worker_sandbox_" + workerId,
+ hardenedSandboxOptions != null ? hardenedSandboxOptions.memoryLimit() : 0);
+ }
+
// TODO(larsrc): Check that execRoot and outputBase are not under /tmp
if (hardenedSandboxOptions != null) {
// In hardened mode, we bindmount a temp dir. We put the mount dir in the parent directory to
@@ -195,14 +206,8 @@
.setUseFakeHostname(this.hardenedSandboxOptions.fakeHostname())
.setCreateNetworkNamespace(NETNS);
- if (hardenedSandboxOptions.memoryLimit() > 0) {
- // We put the sandbox inside a unique subdirectory using the worker's ID.
- CgroupsInfo workerCgroup =
- CgroupsInfo.getBlazeSpawnsCgroup()
- .createIndividualSpawnCgroup(
- "worker_sandbox_" + workerId, hardenedSandboxOptions.memoryLimit());
- cgroupsDir = workerCgroup.getCgroupDir().toString();
- commandLineBuilder.setCgroupsDir(cgroupsDir);
+ if (cgroup != null && cgroup.exists()) {
+ commandLineBuilder.setCgroupsDir(cgroup.getCgroupDir().toString());
}
if (this.hardenedSandboxOptions.fakeUsername()) {
@@ -211,7 +216,17 @@
args = commandLineBuilder.buildForCommand(args);
}
- return createProcessBuilder(args).start();
+
+ Subprocess process = createProcessBuilder(args).start();
+
+ // If using hardened sandbox (aka linux-sandbox), the linux-sandbox parent process moves the
+ // sandboxed children processes (pid 1, 2) into the cgroup. But we still need to move the
+ // linux-sandbox process into the worker cgroup. On the other hand, without linux-sandbox, Blaze
+ // needs to do this itself for the spawned worker process.
+ if (cgroup != null && cgroup.exists()) {
+ cgroup.addProcess(process.getProcessId());
+ }
+ return process;
}
@Override
@@ -228,9 +243,9 @@
@Override
public void finishExecution(Path execRoot, SandboxOutputs outputs) throws IOException {
super.finishExecution(execRoot, outputs);
- if (cgroupsDir != null) {
+ if (cgroup != null && cgroup.exists()) {
// This is only to not leave too much behind in the cgroups tree, can ignore errors.
- new File(cgroupsDir).delete();
+ cgroup.getCgroupDir().delete();
}
workerExecRoot.copyOutputs(execRoot, outputs);
}
@@ -245,9 +260,9 @@
if (inaccessibleHelperDir != null) {
inaccessibleHelperDir.delete();
}
- if (cgroupsDir != null) {
+ if (cgroup != null && cgroup.exists()) {
// This is only to not leave too much behind in the cgroups tree, can ignore errors.
- new File(cgroupsDir).delete();
+ cgroup.getCgroupDir().delete();
}
workDir.deleteTree();
} catch (IOException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/worker/SingleplexWorker.java b/src/main/java/com/google/devtools/build/lib/worker/SingleplexWorker.java
index 91eb3dd..ccf8a1c 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/SingleplexWorker.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/SingleplexWorker.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.UserExecException;
+import com.google.devtools.build.lib.sandbox.CgroupsInfo;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs;
import com.google.devtools.build.lib.shell.Subprocess;
@@ -68,14 +69,28 @@
*/
protected Thread shutdownHook;
- SingleplexWorker(WorkerKey workerKey, int workerId, final Path workDir, Path logFile) {
+ protected WorkerOptions options;
+
+ SingleplexWorker(
+ WorkerKey workerKey, int workerId, final Path workDir, Path logFile, WorkerOptions options) {
super(workerKey, workerId, logFile, new WorkerProcessStatus());
this.workDir = workDir;
+ this.options = options;
}
protected Subprocess createProcess() throws IOException, InterruptedException, UserExecException {
ImmutableList<String> args = makeExecPathAbsolute(workerKey.getArgs());
- return createProcessBuilder(args).start();
+ Subprocess process = createProcessBuilder(args).start();
+ if (options.useCgroupsOnLinux && CgroupsInfo.isSupported()) {
+ cgroup =
+ CgroupsInfo.getBlazeSpawnsCgroup()
+ .createIndividualSpawnCgroup(
+ /* dirName= */ "worker_" + workerId, /* memoryLimitMb= */ 0);
+ if (cgroup.exists()) {
+ cgroup.addProcess(process.getProcessId());
+ }
+ }
+ return process;
}
protected SubprocessBuilder createProcessBuilder(ImmutableList<String> argv) {
diff --git a/src/main/java/com/google/devtools/build/lib/worker/Worker.java b/src/main/java/com/google/devtools/build/lib/worker/Worker.java
index 8312196..345db1b 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/Worker.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/Worker.java
@@ -16,6 +16,7 @@
import com.google.common.hash.HashCode;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.sandbox.CgroupsInfo;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs;
import com.google.devtools.build.lib.vfs.Path;
@@ -26,6 +27,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.SortedMap;
+import javax.annotation.Nullable;
/**
* An abstract superclass for persistent workers. Workers execute actions in long-running processes
@@ -51,6 +53,8 @@
protected final WorkerProcessStatus status;
+ @Nullable protected CgroupsInfo cgroup = null;
+
/**
* Returns a unique id for this worker. This is used to distinguish different worker processes in
* logs and messages.
@@ -73,6 +77,11 @@
return status;
}
+ @Nullable
+ public CgroupsInfo getCgroup() {
+ return cgroup;
+ }
+
HashCode getWorkerFilesCombinedHash() {
return workerKey.getWorkerFilesCombinedHash();
}
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java
index e573a2d..56d1202 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java
@@ -46,6 +46,7 @@
private static final AtomicInteger pidCounter = new AtomicInteger(1);
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
+ protected final WorkerOptions workerOptions;
private final Path workerBaseDir;
private final TreeDeleter treeDeleter;
@@ -57,15 +58,17 @@
*/
@Nullable private final WorkerSandboxOptions hardenedSandboxOptions;
- public WorkerFactory(Path workerBaseDir) {
- this(workerBaseDir, null, null);
+ public WorkerFactory(Path workerBaseDir, WorkerOptions workerOptions) {
+ this(workerBaseDir, workerOptions, null, null);
}
public WorkerFactory(
Path workerBaseDir,
+ WorkerOptions workerOptions,
@Nullable WorkerSandboxOptions hardenedSandboxOptions,
@Nullable TreeDeleter treeDeleter) {
this.workerBaseDir = workerBaseDir;
+ this.workerOptions = workerOptions;
this.hardenedSandboxOptions = hardenedSandboxOptions;
this.treeDeleter = treeDeleter;
}
@@ -94,7 +97,13 @@
Path workDir = getSandboxedWorkerPath(key, workerId);
worker =
new SandboxedWorker(
- key, workerId, workDir, logFile, hardenedSandboxOptions, treeDeleter);
+ key,
+ workerId,
+ workDir,
+ logFile,
+ workerOptions,
+ hardenedSandboxOptions,
+ treeDeleter);
}
} else if (key.isMultiplex()) {
WorkerMultiplexer workerMultiplexer = WorkerMultiplexerManager.getInstance(key, logFile);
@@ -102,7 +111,7 @@
new WorkerProxy(
key, workerId, workerMultiplexer.getLogFile(), workerMultiplexer, key.getExecRoot());
} else {
- worker = new SingleplexWorker(key, workerId, key.getExecRoot(), logFile);
+ worker = new SingleplexWorker(key, workerId, key.getExecRoot(), logFile, workerOptions);
}
String msg =
@@ -150,7 +159,7 @@
}
String msg =
String.format(
- "Destroying %s %s (id %d, key hash %d) with cause: %s %s",
+ "Destroying %s %s (id %d, key hash %d) with cause: %s %s\n",
key.getMnemonic(),
key.getWorkerTypeName(),
workerId,
@@ -240,6 +249,7 @@
}
WorkerFactory that = (WorkerFactory) o;
return workerBaseDir.equals(that.workerBaseDir)
+ && workerOptions.useCgroupsOnLinux == that.workerOptions.useCgroupsOnLinux
&& Objects.equals(this.hardenedSandboxOptions, that.hardenedSandboxOptions);
}
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
index 75c038e..59ac76c 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
@@ -33,6 +33,7 @@
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.commands.events.CleanStartingEvent;
import com.google.devtools.build.lib.sandbox.AsynchronousTreeDeleter;
+import com.google.devtools.build.lib.sandbox.CgroupsInfo;
import com.google.devtools.build.lib.sandbox.LinuxSandboxUtil;
import com.google.devtools.build.lib.sandbox.SandboxHelpers;
import com.google.devtools.build.lib.sandbox.SandboxOptions;
@@ -115,7 +116,7 @@
treeDeleter = new AsynchronousTreeDeleter(trashBase);
}
WorkerFactory newWorkerFactory =
- new WorkerFactory(workerDir, workerSandboxOptions, treeDeleter);
+ new WorkerFactory(workerDir, options, workerSandboxOptions, treeDeleter);
if (!newWorkerFactory.equals(workerFactory)) {
if (workerDir.exists()) {
try {
@@ -170,6 +171,10 @@
WorkerProcessMetricsCollector.instance().clear();
}
+ // Override the flag value if we can't actually use cgroups so that we at least fallback to ps.
+ boolean useCgroupsOnLinux = options.useCgroupsOnLinux && CgroupsInfo.isSupported();
+ WorkerProcessMetricsCollector.instance().setUseCgroupsOnLinux(useCgroupsOnLinux);
+
// Start collecting after a pool is defined
workerLifecycleManager = new WorkerLifecycleManager(workerPool, options);
workerLifecycleManager.setReporter(env.getReporter());
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerOptions.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerOptions.java
index 271f2db..10a960b 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerOptions.java
@@ -55,7 +55,8 @@
}
int pos = input.indexOf('=');
if (pos < 0) {
- return Maps.immutableEntry("", valueConverter.convert(input, /*conversionContext=*/ null));
+ return Maps.immutableEntry(
+ "", valueConverter.convert(input, /* conversionContext= */ null));
}
String name = input.substring(0, pos);
String value = input.substring(pos + 1);
@@ -63,7 +64,8 @@
return Maps.immutableEntry(name, null);
}
- return Maps.immutableEntry(name, valueConverter.convert(value, /*conversionContext=*/ null));
+ return Maps.immutableEntry(
+ name, valueConverter.convert(value, /* conversionContext= */ null));
}
@Override
@@ -198,6 +200,18 @@
public int totalWorkerMemoryLimitMb;
@Option(
+ name = "experimental_worker_use_cgroups_on_linux",
+ defaultValue = "false",
+ // List as undocumented since we will want to make this the default eventually.
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ effectTags = {OptionEffectTag.EXECUTION},
+ help =
+ "On linux, run all workers in its own cgroup (without any limits set) and use the"
+ + " cgroup's own resource accounting for memory measurements. This is overridden by"
+ + " --experimental_worker_sandbox_hardening for sandboxed workers.")
+ public boolean useCgroupsOnLinux;
+
+ @Option(
name = "experimental_worker_sandbox_hardening",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollector.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollector.java
index d80bb78..be66585 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollector.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollector.java
@@ -24,8 +24,10 @@
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.WorkerMetrics;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.WorkerMetrics.WorkerStatus;
import com.google.devtools.build.lib.clock.Clock;
+import com.google.devtools.build.lib.metrics.CgroupsInfoCollector;
import com.google.devtools.build.lib.metrics.PsInfoCollector;
import com.google.devtools.build.lib.metrics.ResourceSnapshot;
+import com.google.devtools.build.lib.sandbox.CgroupsInfo;
import com.google.devtools.build.lib.util.OS;
import java.time.Instant;
import java.util.ArrayList;
@@ -33,6 +35,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import javax.annotation.Nullable;
/** Collects and populates system metrics about persistent workers. */
public class WorkerProcessMetricsCollector {
@@ -40,16 +43,33 @@
/** The metrics collector (a static singleton instance). Inactive by default. */
private static final WorkerProcessMetricsCollector instance = new WorkerProcessMetricsCollector();
+ private final PsInfoCollector psInfoCollector;
+ private final CgroupsInfoCollector cgroupsInfoCollector;
+
private Clock clock;
/**
* Mapping of worker process ids to their process metrics. This contains all workers that have
* been alive at any point during the build.
*/
- private final Map<Long, WorkerProcessMetrics> processIdToWorkerProcessMetrics =
+ private final Map<Long, WorkerProcessMetrics> pidToWorkerProcessMetrics =
new ConcurrentHashMap<>();
- private WorkerProcessMetricsCollector() {}
+ private final Map<Long, CgroupsInfo> pidToCgroups = new ConcurrentHashMap<>();
+
+ private boolean useCgroupsOnLinux = false;
+
+ private WorkerProcessMetricsCollector() {
+ psInfoCollector = PsInfoCollector.instance();
+ cgroupsInfoCollector = CgroupsInfoCollector.instance();
+ }
+
+ @VisibleForTesting
+ WorkerProcessMetricsCollector(
+ PsInfoCollector psInfoCollector, CgroupsInfoCollector cgroupsInfoCollector) {
+ this.psInfoCollector = psInfoCollector;
+ this.cgroupsInfoCollector = cgroupsInfoCollector;
+ }
public static WorkerProcessMetricsCollector instance() {
return instance;
@@ -59,10 +79,14 @@
this.clock = clock;
}
+ public void setUseCgroupsOnLinux(boolean useCgroupsOnLinux) {
+ this.useCgroupsOnLinux = useCgroupsOnLinux;
+ }
+
ResourceSnapshot collectResourceUsage() {
// Only collect for process we know are alive.
ImmutableSet<Long> alivePids =
- processIdToWorkerProcessMetrics.entrySet().stream()
+ pidToWorkerProcessMetrics.entrySet().stream()
.filter(e -> !e.getValue().getStatus().isKilled())
.map(e -> e.getKey())
.collect(toImmutableSet());
@@ -73,13 +97,27 @@
* Collects memory usage of all ancestors of processes by pid. If a pid does not allow collecting
* memory usage, it is silently ignored.
*/
- ResourceSnapshot collectResourceUsage(OS os, ImmutableSet<Long> processIds) {
+ @VisibleForTesting
+ public ResourceSnapshot collectResourceUsage(OS os, ImmutableSet<Long> alivePids) {
// TODO(b/181317827): Support Windows.
- if (processIds.isEmpty()) {
+ if (alivePids.isEmpty()) {
return ResourceSnapshot.createEmpty(clock.now());
}
- if (os == OS.LINUX || os == OS.DARWIN) {
- return PsInfoCollector.instance().collectResourceUsage(processIds, clock);
+ if (os.equals(OS.DARWIN)) {
+ return psInfoCollector.collectResourceUsage(alivePids, clock);
+ }
+ if (os.equals(OS.LINUX)) {
+ if (useCgroupsOnLinux) {
+ // Remove the killed pids so that we only collect from the cgroups that are alive.
+ for (long pid : ImmutableSet.copyOf(pidToCgroups.keySet())) {
+ if (!alivePids.contains(pid)) {
+ pidToCgroups.remove(pid);
+ }
+ }
+ return cgroupsInfoCollector.collectResourceUsage(pidToCgroups, clock);
+ }
+ // Default to using ps if cgroups is not enabled.
+ return psInfoCollector.collectResourceUsage(alivePids, clock);
}
return ResourceSnapshot.createEmpty(clock.now());
}
@@ -94,19 +132,23 @@
ResourceSnapshot resourceSnapshot = collectResourceUsage();
ImmutableMap<Long, Integer> pidToMemoryInKb = resourceSnapshot.getPidToMemoryInKb();
+
Instant collectionTime = resourceSnapshot.getCollectionTime();
ImmutableList.Builder<WorkerProcessMetrics> workerMetrics = new ImmutableList.Builder<>();
- for (Map.Entry<Long, WorkerProcessMetrics> entry : processIdToWorkerProcessMetrics.entrySet()) {
+ for (Map.Entry<Long, WorkerProcessMetrics> entry : pidToWorkerProcessMetrics.entrySet()) {
WorkerProcessMetrics workerMetric = entry.getValue();
- Long pid = workerMetric.getProcessId();
- boolean isMeasurable = pidToMemoryInKb.containsKey(pid);
- if (!isMeasurable && workerMetric.getStatus().isKilled()) {
- // If it is not measurable and previously killed by Bazel, we don't do anything.
+ if (workerMetric.getStatus().isKilled()) {
+ // If it was previously killed by Bazel, we don't do anything.
workerMetrics.add(workerMetric);
continue;
- } else if (!isMeasurable) {
+ }
+
+ Long pid = workerMetric.getProcessId();
+ int memoryInKb = pidToMemoryInKb.getOrDefault(pid, 0);
+
+ if (memoryInKb == 0) {
// If it is not measurable, not killed by Bazel but has executed actions, then we assume
// that something has happened to the worker process that is not accounted for by Bazel
// and set this to KILLED_UNKNOWN. If a separate thread comes along to update the status
@@ -121,8 +163,7 @@
// If it is measurable, we want to update the collected metrics.
workerMetric.addCollectedMetrics(
- /* memoryInKb= */ pidToMemoryInKb.getOrDefault(pid, 0),
- /* collectionTime= */ collectionTime);
+ /* memoryInKb= */ memoryInKb, /* collectionTime= */ collectionTime);
workerMetrics.add(workerMetric);
}
@@ -130,7 +171,7 @@
}
public void onWorkerFinishExecution(long processId) {
- WorkerProcessMetrics wpm = processIdToWorkerProcessMetrics.get(processId);
+ WorkerProcessMetrics wpm = pidToWorkerProcessMetrics.get(processId);
if (wpm == null) {
return;
}
@@ -140,15 +181,6 @@
public static final int MAX_PUBLISHED_WORKER_METRICS = 50;
/** Returns a prioritized and limited list of WorkerMetrics to be published to the BEP. */
- public ImmutableList<WorkerMetrics> getPublishedWorkerMetrics() {
- return collectMetrics().stream()
- .map(WorkerProcessMetrics::toProto)
- .sorted(new WorkerMetricsPublishComparator())
- .limit(MAX_PUBLISHED_WORKER_METRICS)
- .sorted(Comparator.comparingInt(m -> m.getWorkerIdsList().get(0)))
- .collect(toImmutableList());
- }
-
public static ImmutableList<WorkerMetrics> limitWorkerMetricsToPublish(
ImmutableList<WorkerMetrics> metrics, int limit) {
return metrics.stream()
@@ -197,12 +229,12 @@
}
public void clear() {
- processIdToWorkerProcessMetrics.clear();
+ pidToWorkerProcessMetrics.clear();
}
@VisibleForTesting
- Map<Long, WorkerProcessMetrics> getProcessIdToWorkerProcessMetrics() {
- return processIdToWorkerProcessMetrics;
+ Map<Long, WorkerProcessMetrics> getPidToWorkerProcessMetrics() {
+ return pidToWorkerProcessMetrics;
}
/**
@@ -216,9 +248,10 @@
String mnemonic,
boolean isMultiplex,
boolean isSandboxed,
- int workerKeyHash) {
+ int workerKeyHash,
+ @Nullable CgroupsInfo cgroup) {
WorkerProcessMetrics workerMetric =
- processIdToWorkerProcessMetrics.computeIfAbsent(
+ pidToWorkerProcessMetrics.computeIfAbsent(
processId,
(pid) ->
new WorkerProcessMetrics(
@@ -229,7 +262,9 @@
isMultiplex,
isSandboxed,
workerKeyHash));
-
+ if (cgroup != null) {
+ pidToCgroups.putIfAbsent(processId, cgroup);
+ }
workerMetric.setLastCallTime(Instant.ofEpochMilli(clock.currentTimeMillis()));
workerMetric.maybeAddWorkerId(workerId, status);
}
@@ -237,17 +272,17 @@
/** Removes all WorkerProcessMetrics that were marked as killed. */
public void clearKilledWorkerProcessMetrics() {
List<Long> pidsToRemove = new ArrayList<>();
- for (Map.Entry<Long, WorkerProcessMetrics> entry : processIdToWorkerProcessMetrics.entrySet()) {
+ for (Map.Entry<Long, WorkerProcessMetrics> entry : pidToWorkerProcessMetrics.entrySet()) {
if (entry.getValue().getStatus().isKilled()) {
pidsToRemove.add(entry.getKey());
}
}
- processIdToWorkerProcessMetrics.keySet().removeAll(pidsToRemove);
+ pidToWorkerProcessMetrics.keySet().removeAll(pidsToRemove);
}
/** To reset states in each WorkerProcessMetric before each command where applicable. */
public void beforeCommand() {
- processIdToWorkerProcessMetrics.values().forEach(m -> m.onBeforeCommand());
+ pidToWorkerProcessMetrics.values().forEach(m -> m.onBeforeCommand());
}
// TODO(b/238416583) Add deregister function
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
index dbc5073..599b881 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
@@ -616,7 +616,8 @@
workerKey.getMnemonic(),
workerKey.isMultiplex(),
workerKey.isSandboxed(),
- workerKey.hashCode());
+ workerKey.hashCode(),
+ worker.getCgroup());
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/actions/BUILD b/src/test/java/com/google/devtools/build/lib/actions/BUILD
index d784af0..c4dad0c 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/actions/BUILD
@@ -68,13 +68,13 @@
"//src/main/java/com/google/devtools/build/lib/testing/common:directory_listing_helper",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
- "//src/main/java/com/google/devtools/build/lib/util:string",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
"//src/main/java/com/google/devtools/build/lib/worker",
"//src/main/java/com/google/devtools/build/lib/worker:worker_factory",
"//src/main/java/com/google/devtools/build/lib/worker:worker_key",
+ "//src/main/java/com/google/devtools/build/lib/worker:worker_options",
"//src/main/java/com/google/devtools/build/lib/worker:worker_pool_impl",
"//src/main/java/com/google/devtools/build/lib/worker:worker_process_status",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java
index bb626e6..9cc18f7 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java
@@ -45,6 +45,7 @@
import com.google.devtools.build.lib.worker.Worker;
import com.google.devtools.build.lib.worker.WorkerFactory;
import com.google.devtools.build.lib.worker.WorkerKey;
+import com.google.devtools.build.lib.worker.WorkerOptions;
import com.google.devtools.build.lib.worker.WorkerPoolImpl;
import com.google.devtools.build.lib.worker.WorkerProcessStatus;
import com.google.devtools.build.lib.worker.WorkerProcessStatus.Status;
@@ -95,7 +96,7 @@
private WorkerPoolImpl createWorkerPool() {
return new WorkerPoolImpl(
new WorkerPoolImpl.WorkerPoolConfig(
- new WorkerFactory(fs.getPath("/workerBase")) {
+ new WorkerFactory(fs.getPath("/workerBase"), new WorkerOptions()) {
@Override
public Worker create(WorkerKey key) {
return worker;
diff --git a/src/test/java/com/google/devtools/build/lib/worker/BUILD b/src/test/java/com/google/devtools/build/lib/worker/BUILD
index 73d83aa..5379b61 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/worker/BUILD
@@ -43,8 +43,6 @@
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
- "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
- "//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/sandbox:sandbox_helpers",
"//src/main/java/com/google/devtools/build/lib/shell",
@@ -54,7 +52,6 @@
"//src/main/java/com/google/devtools/build/lib/worker:worker_key",
"//src/main/java/com/google/devtools/build/lib/worker:worker_options",
"//src/main/java/com/google/devtools/build/lib/worker:worker_spawn_runner",
- "//src/main/java/net/starlark/java/syntax",
"//src/test/java/com/google/devtools/build/lib/actions/util",
"//third_party:guava",
],
@@ -103,6 +100,7 @@
"//src/main/java/com/google/devtools/build/lib/sandbox:sandbox_helpers",
"//src/main/java/com/google/devtools/build/lib/sandbox:tree_deleter",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
+ "//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/util:resource_converter",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
diff --git a/src/test/java/com/google/devtools/build/lib/worker/SandboxedWorkerProxyTest.java b/src/test/java/com/google/devtools/build/lib/worker/SandboxedWorkerProxyTest.java
index 1336f16..692b0b8 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/SandboxedWorkerProxyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/SandboxedWorkerProxyTest.java
@@ -162,7 +162,7 @@
WorkerKey key =
TestUtils.createWorkerKeyFromOptions(
PROTO, globalOutputBase, options, true, spawn, "worker.sh");
- WorkerFactory factory = new WorkerFactory(workerBaseDir);
+ WorkerFactory factory = new WorkerFactory(workerBaseDir, options);
return (SandboxedWorkerProxy) factory.create(key);
}
@@ -187,7 +187,7 @@
super.process = new FakeSubprocess(serverInputStream);
}
});
- WorkerFactory factory = new WorkerFactory(workerBaseDir);
+ WorkerFactory factory = new WorkerFactory(workerBaseDir, options);
return (SandboxedWorkerProxy) factory.create(key);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/worker/SimpleWorkerPoolTest.java b/src/test/java/com/google/devtools/build/lib/worker/SimpleWorkerPoolTest.java
index e060046..e2b4928 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/SimpleWorkerPoolTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/SimpleWorkerPoolTest.java
@@ -36,16 +36,17 @@
private static FileSystem fileSystem;
private static class TestWorker extends SingleplexWorker {
- TestWorker(WorkerKey workerKey, int workerId, Path workDir, Path logFile) {
- super(workerKey, workerId, workDir, logFile);
+ TestWorker(
+ WorkerKey workerKey, int workerId, Path workDir, Path logFile, WorkerOptions options) {
+ super(workerKey, workerId, workDir, logFile, options);
}
}
private static class TestWorkerFactory extends WorkerFactory {
private int workerIds = 1;
- public TestWorkerFactory(Path workerBaseDir) {
- super(workerBaseDir);
+ public TestWorkerFactory(Path workerBaseDir, WorkerOptions workerOptions) {
+ super(workerBaseDir, workerOptions);
}
@Override
@@ -55,7 +56,8 @@
workerKey,
workerIds++,
fileSystem.getPath("/workDir"),
- fileSystem.getPath("/logDir")));
+ fileSystem.getPath("/logDir"),
+ workerOptions));
}
@Override
@@ -67,7 +69,7 @@
@Before
public void setUp() throws Exception {
fileSystem = new InMemoryFileSystem(BlazeClock.instance(), DigestHashFunction.SHA256);
- workerFactory = new TestWorkerFactory(null);
+ workerFactory = new TestWorkerFactory(null, new WorkerOptions());
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/worker/TestUtils.java b/src/test/java/com/google/devtools/build/lib/worker/TestUtils.java
index 0a4783b..c7c4355 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/TestUtils.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/TestUtils.java
@@ -219,8 +219,9 @@
int workerId,
final Path workDir,
Path logFile,
- FakeSubprocess fakeSubprocess) {
- super(workerKey, workerId, workDir, logFile);
+ FakeSubprocess fakeSubprocess,
+ WorkerOptions options) {
+ super(workerKey, workerId, workDir, logFile, options);
this.fakeSubprocess = fakeSubprocess;
}
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java
index a920a07..00bf258 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java
@@ -51,7 +51,7 @@
@Test
public void sandboxedWorkerPathEndsWithWorkspaceName() throws Exception {
Path workerBaseDir = fs.getPath("/outputbase/bazel-workers");
- WorkerFactory workerFactory = new WorkerFactory(workerBaseDir);
+ WorkerFactory workerFactory = new WorkerFactory(workerBaseDir, new WorkerOptions());
WorkerKey workerKey = createWorkerKey(/* mustBeSandboxed= */ true, /* multiplex= */ false);
Path sandboxedWorkerPath = workerFactory.getSandboxedWorkerPath(workerKey, 1);
@@ -62,7 +62,7 @@
@Test
public void workerCreationTypeCheck() throws Exception {
Path workerBaseDir = fs.getPath("/outputbase/bazel-workers");
- WorkerFactory workerFactory = new WorkerFactory(workerBaseDir);
+ WorkerFactory workerFactory = new WorkerFactory(workerBaseDir, new WorkerOptions());
WorkerKey sandboxedWorkerKey =
createWorkerKey(/* mustBeSandboxed= */ true, /* multiplex= */ false);
Worker sandboxedWorker = workerFactory.create(sandboxedWorkerKey);
@@ -86,7 +86,7 @@
@Test
public void testMultiplexWorkersShareLogfiles() throws Exception {
Path workerBaseDir = fs.getPath("/outputbase/bazel-workers");
- WorkerFactory workerFactory = new WorkerFactory(workerBaseDir);
+ WorkerFactory workerFactory = new WorkerFactory(workerBaseDir, new WorkerOptions());
WorkerKey workerKey1 =
createWorkerKey(/* mustBeSandboxed= */ false, /* multiplex= */ true, "arg1");
@@ -103,7 +103,7 @@
@Test
public void testCreate_createsWorkerDirectory() throws Exception {
Path workerBaseDir = fs.getPath("/outputbase/bazel-workers");
- WorkerFactory workerFactory = new WorkerFactory(workerBaseDir);
+ WorkerFactory workerFactory = new WorkerFactory(workerBaseDir, new WorkerOptions());
WorkerKey sandboxedWorkerKey = createWorkerKey(/* mustBeSandboxed */ true, /* proxied */ false);
assertThat(workerBaseDir.isDirectory()).isFalse();
workerFactory.create(sandboxedWorkerKey);
@@ -118,7 +118,7 @@
@Test
public void testDoomedWorkerValidation() throws Exception {
Path workerBaseDir = fs.getPath("/outputbase/bazel-workers");
- WorkerFactory workerFactory = new WorkerFactory(workerBaseDir);
+ WorkerFactory workerFactory = new WorkerFactory(workerBaseDir, new WorkerOptions());
WorkerKey workerKey =
createWorkerKey(/* mustBeSandboxed= */ false, /* multiplex= */ false, "arg1");
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerLifecycleManagerTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerLifecycleManagerTest.java
index c068be2..44d928c 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerLifecycleManagerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerLifecycleManagerTest.java
@@ -58,7 +58,7 @@
@Before
public void setUp() throws Exception {
fileSystem = new InMemoryFileSystem(BlazeClock.instance(), DigestHashFunction.SHA256);
-
+ WorkerOptions options = new WorkerOptions();
doAnswer(
args -> {
WorkerKey key = args.getArgument(0);
@@ -78,7 +78,8 @@
key,
workerIds++,
fileSystem.getPath("/workDir"),
- fileSystem.getPath("/logDir")));
+ fileSystem.getPath("/logDir"),
+ options));
})
.when(factoryMock)
.makeObject(any());
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java
index 9c3ac4f..0d46475 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java
@@ -41,6 +41,8 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.devtools.common.options.Options;
+import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsParsingResult;
import java.io.IOException;
import org.junit.Rule;
@@ -141,6 +143,35 @@
}
@Test
+ public void buildStarting_restartsOnUseCgroupsOnLinuxChanges()
+ throws IOException, AbruptExitException, OptionsParsingException {
+ WorkerModule module = new WorkerModule();
+ WorkerOptions options =
+ Options.parse(WorkerOptions.class, "--noexperimental_worker_use_cgroups_on_linux")
+ .getOptions();
+ when(request.getOptions(WorkerOptions.class)).thenReturn(options);
+ setupEnvironment("/outputRoot");
+
+ module.beforeCommand(env);
+ // Check that new pools/factories are made with default options
+ module.buildStarting(BuildStartingEvent.create(env, request));
+ assertThat(storedEventHandler.getEvents()).isEmpty();
+
+ WorkerPool oldPool = module.workerPool;
+ WorkerOptions newOptions =
+ Options.parse(WorkerOptions.class, "--experimental_worker_use_cgroups_on_linux")
+ .getOptions();
+ when(request.getOptions(WorkerOptions.class)).thenReturn(newOptions);
+
+ module.beforeCommand(env);
+ module.buildStarting(BuildStartingEvent.create(env, request));
+ assertThat(storedEventHandler.getEvents()).hasSize(1);
+ assertThat(storedEventHandler.getEvents().get(0).getMessage())
+ .contains("Worker factory configuration has changed");
+ assertThat(module.workerPool).isNotSameInstanceAs(oldPool);
+ }
+
+ @Test
public void buildStarting_clearsLogsOnFactoryCreation() throws IOException, AbruptExitException {
WorkerModule module = new WorkerModule();
WorkerOptions options = WorkerOptions.DEFAULTS;
@@ -160,7 +191,6 @@
assertThat(oldLog.exists()).isFalse();
}
-
@Test
public void buildStarting_restartsOnNumMultiplexWorkersChanges()
throws IOException, AbruptExitException {
@@ -228,7 +258,7 @@
workerDir.getParentDirectory().setWritable(false);
// But an actual worker cannot be created.
- WorkerKey key = TestUtils.createWorkerKey(fs, "Work", /* proxied=*/ false);
+ WorkerKey key = TestUtils.createWorkerKey(fs, "Work", /* proxied= */ false);
assertThrows(IOException.class, () -> module.workerPool.borrowObject(key));
}
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java
index 17212b0..da1327a 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java
@@ -49,9 +49,12 @@
private FileSystem fileSystem;
private int workerIds = 1;
+ private final WorkerOptions options = new WorkerOptions();
+
private static class TestWorker extends SingleplexWorker {
- TestWorker(WorkerKey workerKey, int workerId, Path workDir, Path logFile) {
- super(workerKey, workerId, workDir, logFile);
+ TestWorker(
+ WorkerKey workerKey, int workerId, Path workDir, Path logFile, WorkerOptions options) {
+ super(workerKey, workerId, workDir, logFile, options);
}
}
@@ -65,7 +68,8 @@
arg.getArgument(0),
workerIds++,
fileSystem.getPath("/workDir"),
- fileSystem.getPath("/logDir"))))
+ fileSystem.getPath("/logDir"),
+ options)))
.when(factoryMock)
.makeObject(any());
doAnswer(
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollectorTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollectorTest.java
index bdfe29c..e1d6404 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollectorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollectorTest.java
@@ -18,14 +18,20 @@
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.WorkerMetrics;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.WorkerMetrics.WorkerStatus;
import com.google.devtools.build.lib.clock.Clock;
+import com.google.devtools.build.lib.metrics.CgroupsInfoCollector;
+import com.google.devtools.build.lib.metrics.PsInfoCollector;
import com.google.devtools.build.lib.metrics.ResourceSnapshot;
+import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.worker.WorkerProcessMetricsCollector.WorkerMetricsPublishComparator;
import com.google.devtools.build.lib.worker.WorkerProcessStatus.Status;
import java.time.Instant;
@@ -38,12 +44,17 @@
@RunWith(JUnit4.class)
public class WorkerProcessMetricsCollectorTest {
- private final WorkerProcessMetricsCollector spyCollector =
- spy(WorkerProcessMetricsCollector.instance());
+ private PsInfoCollector psInfoCollector;
+ private CgroupsInfoCollector cgroupsInfoCollector;
+
+ private WorkerProcessMetricsCollector spyCollector;
ManualClock clock = new ManualClock();
@Before
public void setUp() {
+ psInfoCollector = mock(PsInfoCollector.class);
+ cgroupsInfoCollector = mock(CgroupsInfoCollector.class);
+ spyCollector = spy(new WorkerProcessMetricsCollector(psInfoCollector, cgroupsInfoCollector));
spyCollector.clear();
spyCollector.setClock(clock);
}
@@ -106,9 +117,9 @@
JAVAC_MNEMONIC,
/* isMultiplex= */ true,
/* isSandboxed= */ false,
- WORKER_KEY_HASH_1);
- assertThat(spyCollector.getProcessIdToWorkerProcessMetrics().keySet())
- .containsExactly(PROCESS_ID_1);
+ WORKER_KEY_HASH_1,
+ /* cgroup= */ null);
+ assertThat(spyCollector.getPidToWorkerProcessMetrics().keySet()).containsExactly(PROCESS_ID_1);
spyCollector.registerWorker(
WORKER_ID_2,
PROCESS_ID_2,
@@ -116,11 +127,12 @@
CPP_COMPILE_MNEMONIC,
/* isMultiplex= */ false,
/* isSandboxed= */ true,
- WORKER_KEY_HASH_2);
- assertThat(spyCollector.getProcessIdToWorkerProcessMetrics().keySet())
+ WORKER_KEY_HASH_2,
+ /* cgroup= */ null);
+ assertThat(spyCollector.getPidToWorkerProcessMetrics().keySet())
.containsExactly(PROCESS_ID_1, PROCESS_ID_2);
assertWorkerMetricContains(
- spyCollector.getProcessIdToWorkerProcessMetrics().get(PROCESS_ID_1),
+ spyCollector.getPidToWorkerProcessMetrics().get(PROCESS_ID_1),
ImmutableList.of(WORKER_ID_1),
PROCESS_ID_1,
JAVAC_MNEMONIC,
@@ -132,7 +144,7 @@
/* expectedLastCallTime= */ DEFAULT_CLOCK_START_INSTANT,
/* expectedCollectedTime= */ null);
assertWorkerMetricContains(
- spyCollector.getProcessIdToWorkerProcessMetrics().get(PROCESS_ID_2),
+ spyCollector.getPidToWorkerProcessMetrics().get(PROCESS_ID_2),
ImmutableList.of(WORKER_ID_2),
PROCESS_ID_2,
CPP_COMPILE_MNEMONIC,
@@ -154,11 +166,11 @@
JAVAC_MNEMONIC,
/* isMultiplex= */ true,
/* isSandboxed= */ true,
- WORKER_KEY_HASH_1);
- assertThat(spyCollector.getProcessIdToWorkerProcessMetrics().keySet())
- .containsExactly(PROCESS_ID_1);
+ WORKER_KEY_HASH_1,
+ /* cgroup= */ null);
+ assertThat(spyCollector.getPidToWorkerProcessMetrics().keySet()).containsExactly(PROCESS_ID_1);
assertWorkerMetricContains(
- spyCollector.getProcessIdToWorkerProcessMetrics().get(PROCESS_ID_1),
+ spyCollector.getPidToWorkerProcessMetrics().get(PROCESS_ID_1),
ImmutableList.of(WORKER_ID_1),
PROCESS_ID_1,
JAVAC_MNEMONIC,
@@ -180,11 +192,11 @@
JAVAC_MNEMONIC,
/* isMultiplex= */ true,
/* isSandboxed= */ true,
- WORKER_KEY_HASH_1);
- assertThat(spyCollector.getProcessIdToWorkerProcessMetrics().keySet())
- .containsExactly(PROCESS_ID_1);
+ WORKER_KEY_HASH_1,
+ /* cgroup= */ null);
+ assertThat(spyCollector.getPidToWorkerProcessMetrics().keySet()).containsExactly(PROCESS_ID_1);
assertWorkerMetricContains(
- spyCollector.getProcessIdToWorkerProcessMetrics().get(PROCESS_ID_1),
+ spyCollector.getPidToWorkerProcessMetrics().get(PROCESS_ID_1),
ImmutableList.of(WORKER_ID_1, WORKER_ID_2),
PROCESS_ID_1,
JAVAC_MNEMONIC,
@@ -206,11 +218,11 @@
JAVAC_MNEMONIC,
/* isMultiplex= */ true,
/* isSandboxed= */ true,
- WORKER_KEY_HASH_1);
- assertThat(spyCollector.getProcessIdToWorkerProcessMetrics().keySet())
- .containsExactly(PROCESS_ID_1);
+ WORKER_KEY_HASH_1,
+ /* cgroup= */ null);
+ assertThat(spyCollector.getPidToWorkerProcessMetrics().keySet()).containsExactly(PROCESS_ID_1);
assertWorkerMetricContains(
- spyCollector.getProcessIdToWorkerProcessMetrics().get(PROCESS_ID_1),
+ spyCollector.getPidToWorkerProcessMetrics().get(PROCESS_ID_1),
ImmutableList.of(WORKER_ID_1),
PROCESS_ID_1,
JAVAC_MNEMONIC,
@@ -233,11 +245,11 @@
JAVAC_MNEMONIC,
/* isMultiplex= */ true,
/* isSandboxed= */ true,
- WORKER_KEY_HASH_1);
- assertThat(spyCollector.getProcessIdToWorkerProcessMetrics().keySet())
- .containsExactly(PROCESS_ID_1);
+ WORKER_KEY_HASH_1,
+ /* cgroup= */ null);
+ assertThat(spyCollector.getPidToWorkerProcessMetrics().keySet()).containsExactly(PROCESS_ID_1);
assertWorkerMetricContains(
- spyCollector.getProcessIdToWorkerProcessMetrics().get(PROCESS_ID_1),
+ spyCollector.getPidToWorkerProcessMetrics().get(PROCESS_ID_1),
ImmutableList.of(WORKER_ID_1),
PROCESS_ID_1,
JAVAC_MNEMONIC,
@@ -260,8 +272,9 @@
JAVAC_MNEMONIC,
/* isMultiplex= */ true,
/* isSandboxed= */ false,
- WORKER_KEY_HASH_1);
- WorkerProcessMetricsCollector.instance().onWorkerFinishExecution(PROCESS_ID_1);
+ WORKER_KEY_HASH_1,
+ /* cgroup= */ null);
+ spyCollector.onWorkerFinishExecution(PROCESS_ID_1);
// Worker 2 simulates a measurable worker process that has not yet completed execution of any
// actions.
spyCollector.registerWorker(
@@ -271,7 +284,8 @@
CPP_COMPILE_MNEMONIC,
/* isMultiplex= */ false,
/* isSandboxed= */ true,
- WORKER_KEY_HASH_2);
+ WORKER_KEY_HASH_2,
+ /* cgroup= */ null);
// Worker 3 simulates a non-measurable worker that has not executed any actions.
spyCollector.registerWorker(
WORKER_ID_3,
@@ -280,7 +294,8 @@
PROTO_MNEMONIC,
/* isMultiplex= */ true,
/* isSandboxed= */ true,
- WORKER_KEY_HASH_3);
+ WORKER_KEY_HASH_3,
+ /* cgroup= */ null);
// Worker 4 simulates a non-measurable worker that has executed an action and was killed.
WorkerProcessStatus s4 = new WorkerProcessStatus();
spyCollector.registerWorker(
@@ -290,8 +305,9 @@
PROTO_MNEMONIC,
/* isMultiplex= */ true,
/* isSandboxed= */ true,
- WORKER_KEY_HASH_4);
- WorkerProcessMetricsCollector.instance().onWorkerFinishExecution(PROCESS_ID_4);
+ WORKER_KEY_HASH_4,
+ /* cgroup= */ null);
+ spyCollector.onWorkerFinishExecution(PROCESS_ID_4);
s4.maybeUpdateStatus(Status.KILLED_DUE_TO_MEMORY_PRESSURE);
// Worker 5 simulates a non-measurable worker that has executed an action, but was not killed.
WorkerProcessStatus s5 = new WorkerProcessStatus();
@@ -302,8 +318,9 @@
PROTO_MNEMONIC,
/* isMultiplex= */ true,
/* isSandboxed= */ true,
- WORKER_KEY_HASH_5);
- WorkerProcessMetricsCollector.instance().onWorkerFinishExecution(PROCESS_ID_5);
+ WORKER_KEY_HASH_5,
+ /* cgroup= */ null);
+ spyCollector.onWorkerFinishExecution(PROCESS_ID_5);
ImmutableMap<Long, Integer> memoryUsageMap =
ImmutableMap.of(
@@ -311,7 +328,7 @@
PROCESS_ID_2, 2345);
Instant collectionTime = DEFAULT_CLOCK_START_INSTANT.plusSeconds(10);
ResourceSnapshot resourceSnapshot = ResourceSnapshot.create(memoryUsageMap, collectionTime);
- doReturn(resourceSnapshot).when(spyCollector).collectResourceUsage(any(), any());
+ doReturn(resourceSnapshot).when(spyCollector).collectResourceUsage();
clock.setTime(collectionTime.toEpochMilli());
ImmutableList<WorkerProcessMetrics> metrics = spyCollector.collectMetrics();
@@ -320,7 +337,7 @@
assertThat(metrics.stream().flatMap(m -> m.getWorkerIds().stream()).collect(toImmutableSet()))
.containsExactly(WORKER_ID_1, WORKER_ID_2, WORKER_ID_3, WORKER_ID_4, WORKER_ID_5);
assertWorkerMetricContains(
- metrics.stream().filter(wm -> wm.getWorkerIds().contains(WORKER_ID_1)).findFirst().get(),
+ getWorkerProcessMetricsFromList(WORKER_ID_1, metrics),
ImmutableList.of(WORKER_ID_1),
PROCESS_ID_1,
JAVAC_MNEMONIC,
@@ -332,7 +349,7 @@
/* expectedLastCallTime= */ DEFAULT_CLOCK_START_INSTANT,
/* expectedCollectedTime= */ collectionTime);
assertWorkerMetricContains(
- metrics.stream().filter(wm -> wm.getWorkerIds().contains(WORKER_ID_2)).findFirst().get(),
+ getWorkerProcessMetricsFromList(WORKER_ID_2, metrics),
ImmutableList.of(WORKER_ID_2),
PROCESS_ID_2,
CPP_COMPILE_MNEMONIC,
@@ -344,9 +361,9 @@
/* expectedLastCallTime= */ DEFAULT_CLOCK_START_INSTANT,
/* expectedCollectedTime= */ collectionTime);
// Worker 3's metrics should not be included since it is both non-measurable and did not execute
- // any actions. It's status shouldn't be unknown because it is possible that
+ // any actions. Its status shouldn't be unknown because it is possible that
assertWorkerMetricContains(
- metrics.stream().filter(wm -> wm.getWorkerIds().contains(WORKER_ID_3)).findFirst().get(),
+ getWorkerProcessMetricsFromList(WORKER_ID_3, metrics),
ImmutableList.of(WORKER_ID_3),
PROCESS_ID_3,
PROTO_MNEMONIC,
@@ -358,7 +375,7 @@
/* expectedLastCallTime= */ DEFAULT_CLOCK_START_INSTANT,
/* expectedCollectedTime= */ null);
assertWorkerMetricContains(
- metrics.stream().filter(wm -> wm.getWorkerIds().contains(WORKER_ID_4)).findFirst().get(),
+ getWorkerProcessMetricsFromList(WORKER_ID_4, metrics),
ImmutableList.of(WORKER_ID_4),
PROCESS_ID_4,
PROTO_MNEMONIC,
@@ -370,7 +387,7 @@
/* expectedLastCallTime= */ DEFAULT_CLOCK_START_INSTANT,
/* expectedCollectedTime= */ null);
assertWorkerMetricContains(
- metrics.stream().filter(wm -> wm.getWorkerIds().contains(WORKER_ID_5)).findFirst().get(),
+ getWorkerProcessMetricsFromList(WORKER_ID_5, metrics),
ImmutableList.of(WORKER_ID_5),
PROCESS_ID_5,
PROTO_MNEMONIC,
@@ -387,6 +404,76 @@
}
@Test
+ public void testCollectResourceUsage_windows() {
+ Instant collectionTime = DEFAULT_CLOCK_START_INSTANT.plusSeconds(10);
+ clock.setTime(collectionTime.toEpochMilli());
+ when(psInfoCollector.collectResourceUsage(any(), any()))
+ .thenReturn(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 1000), collectionTime));
+ when(cgroupsInfoCollector.collectResourceUsage(any(), any()))
+ .thenReturn(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 2000), collectionTime));
+
+ ResourceSnapshot snapshot =
+ spyCollector.collectResourceUsage(OS.WINDOWS, ImmutableSet.of(PROCESS_ID_1));
+
+ // On non-linux and non-darwin, it should always return an empty snapshot.
+ assertThat(snapshot).isEqualTo(ResourceSnapshot.create(ImmutableMap.of(), collectionTime));
+ }
+
+ @Test
+ public void testCollectResourceUsage_darwin_usingPs() {
+ Instant collectionTime = DEFAULT_CLOCK_START_INSTANT.plusSeconds(10);
+ clock.setTime(collectionTime.toEpochMilli());
+ when(psInfoCollector.collectResourceUsage(any(), any()))
+ .thenReturn(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 1000), collectionTime));
+ when(cgroupsInfoCollector.collectResourceUsage(any(), any()))
+ .thenReturn(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 2000), collectionTime));
+
+ ResourceSnapshot snapshot =
+ spyCollector.collectResourceUsage(OS.DARWIN, ImmutableSet.of(PROCESS_ID_1));
+
+ // Should return the cgroup information rather than the ps information.
+ assertThat(snapshot)
+ .isEqualTo(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 1000), collectionTime));
+ }
+
+ @Test
+ public void testCollectResourceUsage_linux_usingPs() {
+ spyCollector.setUseCgroupsOnLinux(/* useCgroupsOnLinux= */ false);
+ Instant collectionTime = DEFAULT_CLOCK_START_INSTANT.plusSeconds(10);
+ clock.setTime(collectionTime.toEpochMilli());
+ when(psInfoCollector.collectResourceUsage(any(), any()))
+ .thenReturn(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 1000), collectionTime));
+ when(cgroupsInfoCollector.collectResourceUsage(any(), any()))
+ .thenReturn(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 2000), collectionTime));
+
+ ResourceSnapshot snapshot =
+ spyCollector.collectResourceUsage(OS.LINUX, ImmutableSet.of(PROCESS_ID_1));
+
+ // Should return the ps information rather than the cgroup information.
+ assertThat(snapshot)
+ .isEqualTo(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 1000), collectionTime));
+ }
+
+ @Test
+ public void testCollectResourceUsage_linux_usingCgroups() {
+
+ spyCollector.setUseCgroupsOnLinux(/* useCgroupsOnLinux= */ true);
+ Instant collectionTime = DEFAULT_CLOCK_START_INSTANT.plusSeconds(10);
+ clock.setTime(collectionTime.toEpochMilli());
+ when(psInfoCollector.collectResourceUsage(any(), any()))
+ .thenReturn(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 1000), collectionTime));
+ when(cgroupsInfoCollector.collectResourceUsage(any(), any()))
+ .thenReturn(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 2000), collectionTime));
+
+ ResourceSnapshot snapshot =
+ spyCollector.collectResourceUsage(OS.LINUX, ImmutableSet.of(PROCESS_ID_1));
+
+ // Should return the cgroup information rather than the ps information.
+ assertThat(snapshot)
+ .isEqualTo(ResourceSnapshot.create(ImmutableMap.of(PROCESS_ID_1, 2000), collectionTime));
+ }
+
+ @Test
public void testWorkerMetricsPublishComparator_compare() {
WorkerMetrics alive1 = newWorkerMetrics(1, WorkerStatus.ALIVE, 100);
WorkerMetrics alive2 = newWorkerMetrics(2, WorkerStatus.ALIVE, 200);
@@ -445,6 +532,11 @@
.build();
}
+ private WorkerProcessMetrics getWorkerProcessMetricsFromList(
+ int workerId, ImmutableList<WorkerProcessMetrics> metrics) {
+ return metrics.stream().filter(wm -> wm.getWorkerIds().contains(workerId)).findFirst().get();
+ }
+
private static final long DEFAULT_CLOCK_START_TIME = 0L;
private static final Instant DEFAULT_CLOCK_START_INSTANT =
Instant.ofEpochMilli(DEFAULT_CLOCK_START_TIME);
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java
index ba549aa..486e6f9 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java
@@ -108,7 +108,7 @@
doNothing()
.when(metricsCollector)
.registerWorker(
- anyInt(), anyLong(), any(), anyString(), anyBoolean(), anyBoolean(), anyInt());
+ anyInt(), anyLong(), any(), anyString(), anyBoolean(), anyBoolean(), anyInt(), any());
when(spawn.getLocalResources()).thenReturn(ResourceSet.createWithRamCpu(100, 1));
when(resourceManager.acquireResources(any(), any(), any())).thenReturn(resourceHandle);
when(resourceHandle.getWorker()).thenReturn(worker);
@@ -117,7 +117,7 @@
private WorkerPoolImpl createWorkerPool() {
return new WorkerPoolImpl(
new WorkerPoolConfig(
- new WorkerFactory(fs.getPath("/workerBase")) {
+ new WorkerFactory(fs.getPath("/workerBase"), options) {
@Override
public Worker create(WorkerKey key) {
return worker;
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerTest.java
index f45bca2..05b9336 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerTest.java
@@ -50,6 +50,7 @@
final FileSystem fs = new InMemoryFileSystem(DigestHashFunction.SHA256);
private TestWorker workerForCleanup = null;
+ private final WorkerOptions options = new WorkerOptions();
@After
public void destroyWorker() throws IOException {
@@ -78,7 +79,8 @@
int workerId = 1;
Path logFile = workerBaseDir.getRelative("test-log-file.log");
- TestWorker worker = new TestWorker(key, workerId, key.getExecRoot(), logFile, fakeSubprocess);
+ TestWorker worker =
+ new TestWorker(key, workerId, key.getExecRoot(), logFile, fakeSubprocess, options);
SandboxInputs sandboxInputs = null;
SandboxOutputs sandboxOutputs = null;
diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD
index 81dbdfd..f1056c2 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -513,6 +513,25 @@
)
sh_test(
+ name = "bazel_worker_with_cgroups_test",
+ size = "large",
+ srcs = ["bazel_worker_test.sh"],
+ args = [
+ "'--worker_sandboxing=no --experimental_worker_use_cgroups_on_linux'",
+ "non-sandboxed",
+ "proto",
+ ],
+ data = [
+ ":test-deps",
+ "//src/test/java/com/google/devtools/build/lib/worker:ExampleWorker_deploy.jar",
+ ],
+ shard_count = 3,
+ tags = [
+ "no_windows",
+ ],
+)
+
+sh_test(
name = "bazel_json_worker_test",
size = "large",
srcs = ["bazel_worker_test.sh"],
@@ -588,6 +607,25 @@
)
sh_test(
+ name = "bazel_sandboxed_worker_with_cgroups_test",
+ size = "large",
+ srcs = ["bazel_worker_test.sh"],
+ args = [
+ "'--worker_sandboxing --experimental_worker_use_cgroups_on_linux'",
+ "sandboxed",
+ "proto",
+ ],
+ data = [
+ ":test-deps",
+ "//src/test/java/com/google/devtools/build/lib/worker:ExampleWorker_deploy.jar",
+ ],
+ shard_count = 3,
+ tags = [
+ "no_windows",
+ ],
+)
+
+sh_test(
name = "server_logging_test",
size = "medium",
srcs = ["server_logging_test.sh"],
diff --git a/src/test/shell/integration/linux-sandbox_test.sh b/src/test/shell/integration/linux-sandbox_test.sh
index 8ce61d0..300a5c8 100755
--- a/src/test/shell/integration/linux-sandbox_test.sh
+++ b/src/test/shell/integration/linux-sandbox_test.sh
@@ -397,7 +397,7 @@
assert_equals 137 "$code" # SIGNAL_BASE + SIGTERM = 128 + 9
}
-# Tests that using cgruops v1 with linux_sandbox.cc works, if it's available
+# Tests that using cgroups v1 with linux_sandbox.cc works, if it's available
function test_cgroups1_memory_limit() {
if ! grep -E '^cgroup +[^ ]+ +cgroup +.*memory.*' /proc/mounts; then
echo "No cgroup memory controller mounted, skipping test"
