From ebaff0a975da4ebbe7f870e95c7e1288225754a8 Mon Sep 17 00:00:00 2001 From: Alessandro Patti Date: Mon, 16 Oct 2023 03:01:28 +0200 Subject: [PATCH 1/6] [Databricks][Cgroups] Extended cgroup support --- .../lib/actions/ExecutionRequirements.java | 3 + .../google/devtools/build/lib/analysis/BUILD | 1 + .../config/BuildConfigurationValue.java | 13 ++ .../analysis/test/TestTargetProperties.java | 15 ++ .../google/devtools/build/lib/sandbox/BUILD | 5 + .../LinuxSandboxCommandLineBuilder.java | 10 +- .../sandbox/LinuxSandboxedSpawnRunner.java | 118 +++++++++- .../build/lib/sandbox/SandboxOptions.java | 24 ++ .../devtools/build/lib/sandbox/cgroups/BUILD | 30 +++ .../build/lib/sandbox/cgroups/Controller.java | 44 ++++ .../build/lib/sandbox/cgroups/Hierarchy.java | 56 +++++ .../build/lib/sandbox/cgroups/Mount.java | 53 +++++ .../lib/sandbox/cgroups/VirtualCGroup.java | 214 ++++++++++++++++++ .../lib/sandbox/cgroups/v1/LegacyCpu.java | 33 +++ .../lib/sandbox/cgroups/v1/LegacyMemory.java | 30 +++ .../lib/sandbox/cgroups/v2/UnifiedCpu.java | 32 +++ .../lib/sandbox/cgroups/v2/UnifiedMemory.java | 29 +++ .../google/devtools/build/lib/worker/BUILD | 2 + .../build/lib/worker/SandboxedWorker.java | 20 +- .../build/lib/worker/WorkerModule.java | 1 + src/main/tools/linux-sandbox-options.cc | 3 +- src/main/tools/linux-sandbox-options.h | 4 +- src/main/tools/linux-sandbox-pid1.cc | 11 +- .../LinuxSandboxCommandLineBuilderTest.java | 8 +- 24 files changed, 726 insertions(+), 33 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/BUILD create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Hierarchy.java create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Mount.java create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpu.java create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedCpu.java create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java index 2c5bc990972059..475610fe86efc4 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java @@ -301,6 +301,9 @@ public enum WorkerProtocolFormat { public static final String DIFFERENTIATE_WORKSPACE_CACHE = "internal-differentiate-workspace-cache"; + /** Disables cgroups for a spawn */ + public static final String NO_SUPPORTS_CGROUPS = "no-supports-cgroups"; + /** * Indicates that the action is compatible with path mapping, e.g., removing the configuration * segment from the paths of all inputs and outputs. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index ab8aa8a0993a12..a88194f809117d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -1691,6 +1691,7 @@ java_library( ":config/run_under", ":config/starlark_defined_config_transition", ":platform_options", + ":test/test_configuration", "//src/main/java/com/google/devtools/build/lib/actions:action_environment", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:build_configuration_event", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java index f54933eaafb5d2..d9e0e379a405ee 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java @@ -41,6 +41,7 @@ import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.starlarkbuildapi.BuildConfigurationApi; import com.google.devtools.build.lib.util.OS; +import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.RegexFilter; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyValue; @@ -988,4 +989,16 @@ public static BuildEvent buildEvent(@Nullable BuildConfigurationValue configurat public ImmutableSet getReservedActionMnemonics() { return reservedActionMnemonics; } + + public Map getTestResources(com.google.devtools.build.lib.packages.TestSize size) { + if (!buildOptions.contains(com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.class)) { + return ImmutableMap.of(); + } + return buildOptions + .get(com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.class) + .testResources + .stream() + .collect(ImmutableMap.toImmutableMap(e -> e.getFirst(), e -> e.getSecond().get(size))); + } + } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java index 3a9e9a348bc4dd..02c646dd6bfabe 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java @@ -92,6 +92,21 @@ private static ResourceSet getResourceSetFromSize(TestSize size) { Map executionInfo = Maps.newLinkedHashMap(); executionInfo.putAll(TargetUtils.getExecutionInfo(rule)); + Map requestedResources; + try { + requestedResources = parseTags(ruleContext.getLabel(), executionInfo); + } catch (UserExecException e) { + requestedResources = new HashMap<>(); + } + + Map testResources = ruleContext.getConfiguration().getTestResources(size); + for (Map.Entry request: testResources.entrySet()) { + if (requestedResources.containsKey(request.getKey())) { + continue; + } + executionInfo.put(String.format("resources:%s:%f", request.getKey(), request.getValue()), ""); + } + boolean incompatibleExclusiveTestSandboxed = false; testConfiguration = ruleContext.getFragment(TestConfiguration.class); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD index 6373e59a9342aa..c00328559326fc 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD +++ b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD @@ -38,6 +38,7 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/util", + "//src/main/java/com/google/devtools/build/lib/util:cpu_resource_converter", "//src/main/java/com/google/devtools/build/lib/util:ram_resource_converter", "//src/main/java/com/google/devtools/build/lib/util:resource_converter", "//src/main/java/com/google/devtools/build/lib/vfs", @@ -212,6 +213,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/actions:exec_exception", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", @@ -223,11 +225,14 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec/local", "//src/main/java/com/google/devtools/build/lib/exec/local:options", "//src/main/java/com/google/devtools/build/lib/profiler", + "//src/main/java/com/google/devtools/build/lib/sandbox/cgroups", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/util:pair", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/main/protobuf:failure_details_proto", + "//src/main/protobuf:failure_details_java_proto", "//third_party:flogger", "//third_party:guava", "//third_party:jsr305", diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java index ac9d0ddcd9f688..cb6fcd1af887d7 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java @@ -55,7 +55,7 @@ public class LinuxSandboxCommandLineBuilder { private boolean enablePseudoterminal = false; private String sandboxDebugPath = null; private boolean sigintSendsSigterm = false; - private String cgroupsDir; + private Set cgroupsDirs = ImmutableSet.of(); private LinuxSandboxCommandLineBuilder(Path linuxSandboxPath) { this.linuxSandboxPath = linuxSandboxPath; @@ -215,8 +215,8 @@ public LinuxSandboxCommandLineBuilder setSandboxDebugPath(String sandboxDebugPat * this directory, its parent directory, and the cgroup directory for the Bazel process. */ @CanIgnoreReturnValue - public LinuxSandboxCommandLineBuilder setCgroupsDir(String cgroupsDir) { - this.cgroupsDir = cgroupsDir; + public LinuxSandboxCommandLineBuilder setCgroupsDirs(Set cgroupsDirs) { + this.cgroupsDirs = cgroupsDirs; return this; } @@ -299,8 +299,8 @@ public ImmutableList buildForCommand(List commandArguments) { if (persistentProcess) { commandLineBuilder.add("-p"); } - if (cgroupsDir != null) { - commandLineBuilder.add("-C", cgroupsDir); + for (Path dir: cgroupsDirs) { + commandLineBuilder.add("-C", dir.toString()); } commandLineBuilder.add("--"); commandLineBuilder.addAll(commandArguments); 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 4f96f9c0317a8d..1850b09d4693c4 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 @@ -45,6 +45,8 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; +import com.google.devtools.build.lib.sandbox.cgroups.VirtualCGroup; +import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.shell.Command; import com.google.devtools.build.lib.shell.CommandException; import com.google.devtools.build.lib.util.OS; @@ -59,6 +61,7 @@ import java.time.Duration; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.SortedMap; import java.util.Set; import java.util.TreeMap; @@ -74,6 +77,8 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { private static final AtomicBoolean warnedAboutUnsupportedModificationCheck = new AtomicBoolean(); + private java.util.concurrent.ConcurrentHashMap> cgroups; + /** * Returns whether the linux sandbox is supported on the local machine by running a small command * in it. @@ -166,10 +171,98 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS this.localEnvProvider = new PosixLocalEnvProvider(cmdEnv.getClientEnv()); this.treeDeleter = treeDeleter; this.reporter = cmdEnv.getReporter(); + this.cgroups = new java.util.concurrent.ConcurrentHashMap<>(); this.slashTmp = cmdEnv.getRuntime().getFileSystem().getPath("/tmp"); this.knownPathsToMountUnderHermeticTmp = collectPathsToMountUnderHermeticTmp(cmdEnv); } + private Optional getCgroup(Spawn spawn, SpawnExecutionContext context) throws ExecException, IOException { + if (spawn.getExecutionInfo().get(ExecutionRequirements.NO_SUPPORTS_CGROUPS) != null) { + return Optional.empty(); + } + if (cgroups.containsKey(context.getId())) { + return cgroups.get(context.getId()); + } + + SandboxOptions sandboxOptions = getSandboxOptions(); + + VirtualCGroup cgroup = null; + long memoryLimit = sandboxOptions.memoryLimitMb * 1024L * 1024L; + float cpuLimit = sandboxOptions.cpuLimit; + + if (sandboxOptions.executionInfoLimit) { + ExecutionRequirements.ParseableRequirement requirement = ExecutionRequirements.RESOURCES; + for (String tag : spawn.getExecutionInfo().keySet()) { + try { + requirement = ExecutionRequirements.RESOURCES; + String name = null; + Float value = null; + + String extras = requirement.parseIfMatches(tag); + if (extras != null) { + int index = extras.indexOf(":"); + name = extras.substring(0, index); + value = Float.parseFloat(extras.substring(index + 1)); + } else { + requirement = ExecutionRequirements.CPU; + String cpus = requirement.parseIfMatches(tag); + if (cpus != null) { + name = "cpu"; + value = Float.parseFloat(cpus); + } + } + if (name == null) { + continue; + } + switch (name) { + case "memory": + memoryLimit = Math.round(value * 1024.0 * 1024.0); + break; + case "cpu": + cpuLimit = value; + break; + } + } catch (ExecutionRequirements.ParseableRequirement.ValidationException e) { + String message = + String.format( + "%s has a '%s' tag, but its value '%s' didn't pass validation: %s", + spawn.getTargetLabel(), + requirement.userFriendlyName(), + e.getTagValue(), + e.getMessage()); + FailureDetails.Spawn.Code code = FailureDetails.Spawn.Code.COMMAND_LINE_EXPANSION_FAILURE; + FailureDetails.FailureDetail details = FailureDetails.FailureDetail + .newBuilder() + .setMessage(message) + .setSpawn(FailureDetails.Spawn.newBuilder().setCode(code)) + .build(); + throw new UserExecException(e, details); + } + } + } + + // We put the sandbox inside a unique subdirectory using the context's ID. This ID is + // unique per spawn run by this spawn runner. + String scope = "sandbox_" + context.getId() + ".scope"; + if (memoryLimit > 0) { + if (cgroup == null) { + cgroup = VirtualCGroup.getInstance(this.reporter).child(scope); + } + cgroup.memory().setMaxBytes(memoryLimit); + } + + if (cpuLimit > 0) { + if (cgroup == null) { + cgroup = VirtualCGroup.getInstance(this.reporter).child(scope); + } + cgroup.cpu().setCpus(cpuLimit); + } + + cgroups.put(context.getId(), Optional.ofNullable(cgroup)); + + return cgroups.get(context.getId()); + } + private ImmutableSet collectPathsToMountUnderHermeticTmp(CommandEnvironment cmdEnv) { // If any path managed or tracked by Bazel is under /tmp, it needs to be explicitly mounted // into the sandbox when using hermetic /tmp. We attempt to collect an over-approximation of @@ -314,14 +407,12 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context commandLineBuilder.setSandboxDebugPath(sandboxDebugPath.getPathString()); } - if (sandboxOptions.memoryLimitMb > 0) { - CgroupsInfo cgroupsInfo = CgroupsInfo.getInstance(); - // We put the sandbox inside a unique subdirectory using the context's ID. This ID is - // unique per spawn run by this spawn runner. - cgroupsDir = - cgroupsInfo.createMemoryLimitCgroupDir( - "sandbox_" + context.getId(), sandboxOptions.memoryLimitMb); - commandLineBuilder.setCgroupsDir(cgroupsDir); + Optional cgroup = getCgroup(spawn, context); + if (cgroup.isPresent()) { + commandLineBuilder.setCgroupsDirs( + cgroup.get().paths().stream() + .map(p -> fileSystem.getPath(p.toString())) + .collect(ImmutableSet.toImmutableSet())); } if (!timeout.isZero()) { @@ -447,6 +538,13 @@ public void verifyPostCondition( if (getSandboxOptions().useHermetic) { checkForConcurrentModifications(context); } + Optional cgroup = cgroups.remove(context.getId()); + if (cgroup != null && cgroup.isPresent()) { + // We cannot leave the cgroups around and delete them only when we delete the sandboxes + // because linux has a hard limit of 65535 memory controllers. + // Ref. https://github.com/torvalds/linux/blob/58d4e450a490d5f02183f6834c12550ba26d3b47/include/linux/memcontrol.h#L69 + cgroup.get().delete(); + } } private void checkForConcurrentModifications(SpawnExecutionContext context) @@ -511,9 +609,7 @@ private boolean wasModifiedSinceDigest(FileContentsProxy proxy, Path path) throw @Override public void cleanupSandboxBase(Path sandboxBase, TreeDeleter treeDeleter) throws IOException { - if (cgroupsDir != null) { - new File(cgroupsDir).delete(); - } + VirtualCGroup.deleteInstance(); // Delete the inaccessible files synchronously, bypassing the treeDeleter. They are only a // couple of files that can be deleted fast, and ensuring they are gone at the end of every // build avoids annoying permission denied errors if the user happens to run "rm -rf" on the diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java index 6a23863c87dd60..f5070b2d830bb4 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.devtools.build.lib.util.CpuResourceConverter; import com.google.devtools.build.lib.util.OptionsUtils; import com.google.devtools.build.lib.util.RamResourceConverter; import com.google.devtools.build.lib.util.ResourceConverter; @@ -381,6 +382,29 @@ public ImmutableSet getInaccessiblePaths(FileSystem fs) { + " Requires cgroups v1 or v2 and permissions for the users to the cgroups dir.") public int memoryLimitMb; + @Option( + name = "experimental_sandbox_cpu_limit", + defaultValue = "0", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + converter = CpuResourceConverter.class, + help = + "If > 0, each Linux sandbox will be limited to the given amount of cpus." + + " Requires cgroups v1 or v2 and permissions for the users to the cgroups dir.") + public float cpuLimit; + + @Option( + name = "experimental_sandbox_execution_info_limit", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + help = + "If true, resources declared in the execution info that match a cgroup controller" + + " will be used to apply the limits. For example a target that declares" + + " cpu:3 and resources:memory:10, will run with at most 3 cpus and 10" + + " megabytes of memory.") + public boolean executionInfoLimit; + /** Converter for the number of threads used for asynchronous tree deletion. */ public static final class AsyncTreeDeletesConverter extends ResourceConverter.IntegerConverter { public AsyncTreeDeletesConverter() { diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/BUILD b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/BUILD new file mode 100644 index 00000000000000..aec4f8b667265c --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/BUILD @@ -0,0 +1,30 @@ +load("@rules_java//java:defs.bzl", "java_library") + +package( + default_applicable_licenses = ["//:license"], + default_visibility = ["//src:__subpackages__"], +) + +filegroup( + name = "srcs", + srcs = glob(["**"]), + visibility = ["//src:__subpackages__"], +) + +java_library( + name = "cgroups", + srcs = glob([ + "*.java", + "v1/*.java", + "v2/*.java", + ]), + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions:exec_exception", + "//src/main/java/com/google/devtools/build/lib/events", + "//src/main/protobuf:failure_details_java_proto", + "//third_party:auto_value", + "//third_party:flogger", + "//third_party:guava", + "//third_party:jsr305", + ], +) diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java new file mode 100644 index 00000000000000..23aa9ed2151e87 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java @@ -0,0 +1,44 @@ +package com.google.devtools.build.lib.sandbox.cgroups; + +import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.server.FailureDetails; + +import java.io.IOException; +import java.lang.reflect.Array; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.nio.file.Path; + +public interface Controller { + default boolean isLegacy() throws IOException { + return !getPath().resolve("cgroup.controllers").toFile().exists(); + } + + static T getDefault(Class clazz) { + InvocationHandler handler = new InvocationHandler() { + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws ExecException { + throw new ExecException("Cgroup requested by cgroups are not available!") { + protected FailureDetails.FailureDetail getFailureDetail(String message) { + return FailureDetails.FailureDetail.newBuilder().setMessage(message).build(); + } + }; + + } + }; + + return (T) Proxy.newProxyInstance(clazz.getClassLoader(), new Class[]{clazz}, handler); + } + + Path getPath() throws IOException; + + interface Memory extends Controller { + void setMaxBytes(long bytes) throws IOException; + long getMaxBytes() throws IOException; + } + interface Cpu extends Controller { + void setCpus(float cpus) throws IOException; + int getCpus() throws IOException; + } +} \ No newline at end of file diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Hierarchy.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Hierarchy.java new file mode 100644 index 00000000000000..9db168ac410543 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Hierarchy.java @@ -0,0 +1,56 @@ +package com.google.devtools.build.lib.sandbox.cgroups; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.common.io.Files; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.List; +import java.util.Scanner; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static java.nio.charset.StandardCharsets.UTF_8; + +@AutoValue +public abstract class Hierarchy { + public abstract Integer id(); + public abstract List controllers(); + public abstract Path path(); + public boolean isV2() { + return controllers().isEmpty() && id() == 0; + } + + /** + * A regexp that matches entries in {@code /proc/self/cgroup}. + * + * The format is documented in https://man7.org/linux/man-pages/man7/cgroups.7.html + */ + private static final Pattern PROC_CGROUPS_PATTERN = + Pattern.compile("^(?\\d+):(?[^:]*):(?.+)"); + + static Hierarchy create(Integer id, List controllers, String path) { + return new AutoValue_Hierarchy(id, controllers, Paths.get(path)); + } + + static List parse(File procCgroup) throws IOException { + ImmutableList.Builder hierarchies = ImmutableList.builder(); + for (String line : Files.readLines(procCgroup, StandardCharsets.UTF_8)) { + Matcher m = PROC_CGROUPS_PATTERN.matcher(line); + if (!m.matches()) { + continue; + } + + Integer id = Integer.parseInt(m.group("id")); + String path = m.group("file"); + List cs = ImmutableList.copyOf(m.group("controllers").split(",")); + hierarchies.add(Hierarchy.create(id, cs, path)); + } + return hierarchies.build(); + } +} \ No newline at end of file diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Mount.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Mount.java new file mode 100644 index 00000000000000..4e86e1ec235de0 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Mount.java @@ -0,0 +1,53 @@ +package com.google.devtools.build.lib.sandbox.cgroups; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.common.io.Files; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +@AutoValue +public abstract class Mount { + /** + * A regexp that matches cgroups entries in {@code /proc/mounts}. + * + * The format is documented in https://man7.org/linux/man-pages/man5/fstab.5.html + */ + private static final Pattern CGROUPS_MOUNT_PATTERN = + Pattern.compile("^[^\\s#]\\S*\\s+(?\\S*)\\s+(?cgroup2?)\\s+(?\\S*).*"); + + public abstract Path path(); + public abstract String type(); + public abstract List opts(); + public boolean isV2() { + return type().equals("cgroup2"); + } + + static Mount create(String path, String type, List opts) { + return new AutoValue_Mount(Paths.get(path), type, opts); + } + + static List parse(File procMounts) throws IOException { + ImmutableList.Builder mounts = ImmutableList.builder(); + + for (String mount: Files.readLines(procMounts, StandardCharsets.UTF_8)) { + Matcher m = CGROUPS_MOUNT_PATTERN.matcher(mount); + if (!m.matches()) { + continue; + } + + String path = m.group("file"); + String type = m.group("vfstype"); + List opts = ImmutableList.copyOf(m.group("mntops").split(",")); + mounts.add(Mount.create(path, type, opts)); + } + return mounts.build(); + } +} \ No newline at end of file diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java new file mode 100644 index 00000000000000..c1fed31b26c8cd --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java @@ -0,0 +1,214 @@ +package com.google.devtools.build.lib.sandbox.cgroups; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.flogger.GoogleLogger; +import com.google.common.io.CharSink; +import com.google.common.io.Files; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.sandbox.cgroups.v1.LegacyCpu; +import com.google.devtools.build.lib.sandbox.cgroups.v1.LegacyMemory; +import com.google.devtools.build.lib.sandbox.cgroups.v2.UnifiedCpu; +import com.google.devtools.build.lib.sandbox.cgroups.v2.UnifiedMemory; + +import javax.annotation.Nullable; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.Map; +import java.util.Queue; +import java.util.Scanner; +import java.util.concurrent.ConcurrentLinkedQueue; + + +/** + * This class creates and exposes a virtual cgroup for the bazel process and allows creating + * child cgroups. Resources are exposed as {@link Controller}s, each representing a + * subsystem within the virtual cgroup and that could in theory belong to different real cgroups. + */ +@AutoValue +public abstract class VirtualCGroup { + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + private static final File PROC_SELF_MOUNTS_PATH = new File("/proc/self/mounts"); + private static final File PROC_SELF_CGROUP_PATH = new File("/proc/self/cgroup"); + + @Nullable + private static volatile VirtualCGroup instance; + + public abstract Controller.Cpu cpu(); + public abstract Controller.Memory memory(); + public abstract ImmutableSet paths(); + + private final Queue children = new ConcurrentLinkedQueue<>(); + + public static VirtualCGroup getInstance(EventHandler reporter) throws IOException { + if (instance == null) { + synchronized (VirtualCGroup.class) { + if (instance == null) { + instance = create(reporter); + } + } + } + return instance; + } + + public static void deleteInstance() { + if (instance != null) { + synchronized (VirtualCGroup.class) { + if (instance != null) { + instance.delete(); + instance = null; + } + } + } + } + + public static VirtualCGroup create(EventHandler reporter) throws IOException { + return create(PROC_SELF_MOUNTS_PATH, PROC_SELF_CGROUP_PATH, reporter); + } + + private static void copyControllersToSubtree(Path cgroup) throws IOException { + File subtree = cgroup.resolve("cgroup.subtree_control").toFile(); + File controllers = cgroup.resolve("cgroup.controllers").toFile(); + if (subtree.canWrite() && controllers.canRead()) { + CharSink sink = Files.asCharSink(subtree, StandardCharsets.UTF_8); + Scanner scanner = new Scanner(controllers); + while (scanner.hasNext()) { + sink.write("+" + scanner.next()); + } + } + } + + static VirtualCGroup create(File procMounts, File procCgroup, EventHandler reporter) throws IOException { + final List mounts = Mount.parse(procMounts); + final Map hierarchies = Hierarchy.parse(procCgroup) + .stream() + .flatMap(h -> h.controllers().stream().map(c -> Map.entry(c, h))) + // For cgroup v2, there are no controllers specified in the proc/pid/cgroup file + // So the keep will be empty and unique. For cgroup v1, there could potentially + // be multiple mounting points for the same controller, but they represent a + // "view of the same hierarchy" so it is ok to just keep one. + // Ref. https://man7.org/linux/man-pages/man7/cgroups.7.html + .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); + + Controller.Memory memory = null; + Controller.Cpu cpu = null; + ImmutableSet.Builder paths = ImmutableSet.builder(); + + for (Mount m: mounts) { + if (memory != null && cpu != null) break; + + if (m.isV2()) { + Hierarchy h = hierarchies.get(""); + if (h == null) continue; + Path cgroup = m.path().resolve(Paths.get("/").relativize(h.path())); + if (!cgroup.equals(m.path())) { + // Because of the "no internal processes" rule, it is not possible to + // create a non-empty child cgroups on non-root cgroups with member processes + // Instead, we go up one level in the hierarchy and declare a sibling. + cgroup = cgroup.getParent(); + } + if (!cgroup.toFile().canWrite()) { + reporter.handle(Event.warn("Found non-writable cgroup v2 at " + cgroup)); + continue; + } + try (InputStream s = new FileInputStream(cgroup.resolve("cgroup.procs").toFile())) { + // Check if the cgroup is empty, i.e. there are no member processes + // before modifying the subtree control to respect the "no internal processes" + if (s.read() != -1) { + reporter.handle(Event.warn("Found non-empty cgroup v2 at " + cgroup)); + continue; + } + copyControllersToSubtree(cgroup); + } + + cgroup = cgroup.resolve("bazel_" + ProcessHandle.current().pid() + ".slice"); + cgroup.toFile().mkdirs(); + paths.add(cgroup); + + Scanner scanner = new Scanner(cgroup.resolve("cgroup.controllers").toFile()); + while (scanner.hasNext()) { + switch (scanner.next()) { + case "memory": + if (memory != null) continue; + logger.atInfo().log("Found cgroup v2 memory controller at %s", cgroup); + memory = new UnifiedMemory(cgroup); + break; + case "cpu": + if (cpu != null) continue; + logger.atInfo().log("Found cgroup v2 cpu controller at %s", cgroup); + cpu = new UnifiedCpu(cgroup); + break; + } + } + } else { + for (String opt : m.opts()) { + Hierarchy h = hierarchies.get(opt); + if (h == null) continue; + Path cgroup = m.path().resolve(Paths.get("/").relativize(h.path())); + if (!cgroup.toFile().canWrite()) { + reporter.handle(Event.warn("Found non-writable cgroup v1 at " + cgroup)); + continue; + } + cgroup = cgroup.resolve("bazel_" + ProcessHandle.current().pid() + ".slice"); + cgroup.toFile().mkdirs(); + paths.add(cgroup); + + switch (opt) { + case "memory": + if (memory != null) continue; + logger.atInfo().log("Found cgroup v1 memory controller at %s", cgroup); + memory = new LegacyMemory(cgroup); + break; + case "cpu": + if (cpu != null) continue; + logger.atInfo().log("Found cgroup v1 cpu controller at %s", cgroup); + cpu = new LegacyCpu(cgroup); + break; + } + } + } + } + + cpu = cpu != null ? cpu : Controller.getDefault(Controller.Cpu.class); + memory = memory != null ? memory : Controller.getDefault(Controller.Memory.class); + VirtualCGroup vcgroup = new AutoValue_VirtualCGroup(cpu, memory, paths.build()); + Runtime.getRuntime().addShutdownHook(new Thread(() -> vcgroup.delete())); + return vcgroup; + } + + public void delete() { + this.children.forEach(VirtualCGroup::delete); + this.paths().stream().map(Path::toFile).filter(File::exists).forEach(File::delete); + } + + public VirtualCGroup child(String name) throws IOException { + Controller.Cpu cpu = Controller.getDefault(Controller.Cpu.class); + Controller.Memory memory = Controller.getDefault(Controller.Memory.class); + ImmutableSet.Builder paths = ImmutableSet.builder(); + if (memory() != null && memory().getPath() != null) { + copyControllersToSubtree(memory().getPath()); + Path cgroup = memory().getPath().resolve(name); + cgroup.toFile().mkdirs(); + memory = memory().isLegacy() ? new LegacyMemory(cgroup) : new UnifiedMemory(cgroup); + paths.add(cgroup); + } + if (cpu() != null && cpu().getPath() != null) { + copyControllersToSubtree(cpu().getPath()); + Path cgroup = cpu().getPath().resolve(name); + cgroup.toFile().mkdirs(); + cpu = cpu().isLegacy() ? new LegacyCpu(cgroup) : new UnifiedCpu(cgroup); + paths.add(cgroup); + } + VirtualCGroup child = new AutoValue_VirtualCGroup(cpu, memory, paths.build()); + this.children.add(child); + return child; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpu.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpu.java new file mode 100644 index 00000000000000..cdcff1643ba070 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpu.java @@ -0,0 +1,33 @@ +package com.google.devtools.build.lib.sandbox.cgroups.v1; + +import com.google.devtools.build.lib.sandbox.cgroups.Controller; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +public class LegacyCpu implements Controller.Cpu { + private final Path path; + private final int period; + + public LegacyCpu(Path path) throws IOException { + this.path = path; + this.period = Integer.parseInt(Files.readString(path.resolve("cpu.cfs_period_us")).trim()); + } + + @Override + public Path getPath() { + return path; + } + + @Override + public void setCpus(float cpus) throws IOException { + int quota = Math.round(cpus * period); + Files.writeString(path.resolve("cpu.cfs_quota_us"), Integer.toString(quota)); + } + + @Override + public int getCpus() throws IOException { + return Integer.parseInt(Files.readString(path.resolve("cpu.cfs_quota_us")).trim()); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java new file mode 100644 index 00000000000000..873f728ccfde19 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java @@ -0,0 +1,30 @@ +package com.google.devtools.build.lib.sandbox.cgroups.v1; + +import com.google.devtools.build.lib.sandbox.cgroups.Controller; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +public class LegacyMemory implements Controller.Memory { + private final Path path; + + @Override + public Path getPath() { + return path; + } + + public LegacyMemory(Path path) { + this.path = path; + } + + @Override + public void setMaxBytes(long bytes) throws IOException { + Files.writeString(path.resolve("memory.limit_in_bytes"), Long.toString(bytes)); + } + + @Override + public long getMaxBytes() throws IOException { + return Long.parseLong(Files.readString(path.resolve("memory.limit_in_bytes")).trim()); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedCpu.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedCpu.java new file mode 100644 index 00000000000000..a3711f845c80cb --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedCpu.java @@ -0,0 +1,32 @@ +package com.google.devtools.build.lib.sandbox.cgroups.v2; + +import com.google.devtools.build.lib.sandbox.cgroups.Controller; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +public class UnifiedCpu implements Controller.Cpu { + private final Path path; + public UnifiedCpu(Path path) { + this.path = path; + } + + @Override + public Path getPath() { + return path; + } + + @Override + public void setCpus(float cpus) throws IOException { + int period = 1000_000; + int quota = Math.round(period * cpus); + String limit = String.format("%d %d", quota, period); + Files.writeString(path.resolve("cpu.max"), limit); + } + + @Override + public int getCpus() throws IOException { + return Integer.parseInt(Files.readString(path.resolve("cpu.max")).trim()); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java new file mode 100644 index 00000000000000..3e6e23e1d7e9bb --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java @@ -0,0 +1,29 @@ +package com.google.devtools.build.lib.sandbox.cgroups.v2; + +import com.google.devtools.build.lib.sandbox.cgroups.Controller; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +public class UnifiedMemory implements Controller.Memory { + private final Path path; + public UnifiedMemory(Path path) { + this.path = path; + } + + @Override + public Path getPath() { + return path; + } + + @Override + public void setMaxBytes(long bytes) throws IOException { + Files.writeString(path.resolve("memory.max"), Long.toString(bytes)); + } + + @Override + public long getMaxBytes() throws IOException { + return Long.parseLong(Files.readString(path.resolve("memory.max")).trim()); + } +} 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 284821909ef17e..2e8539a32ee5d9 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/BUILD +++ b/src/main/java/com/google/devtools/build/lib/worker/BUILD @@ -269,12 +269,14 @@ java_library( ":worker_exec_root", ":worker_key", ":worker_protocol", + "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/exec:tree_deleter", "//src/main/java/com/google/devtools/build/lib/sandbox:linux_sandbox", "//src/main/java/com/google/devtools/build/lib/sandbox:linux_sandbox_command_line_builder", "//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/cgroups", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", 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 befba2576a290a..fc3fbd04fb8e5c 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 @@ -23,12 +23,13 @@ import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.exec.TreeDeleter; -import com.google.devtools.build.lib.sandbox.CgroupsInfo; +import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder; import com.google.devtools.build.lib.sandbox.LinuxSandboxUtil; import com.google.devtools.build.lib.sandbox.SandboxHelpers; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; +import com.google.devtools.build.lib.sandbox.cgroups.VirtualCGroup; import com.google.devtools.build.lib.shell.Subprocess; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; @@ -63,6 +64,8 @@ public abstract static class WorkerSandboxOptions { abstract int memoryLimit(); + abstract EventHandler reporter(); + abstract ImmutableSet inaccessiblePaths(); abstract ImmutableList> additionalMountPaths(); @@ -75,6 +78,7 @@ public static WorkerSandboxOptions create( ImmutableList tmpfsPath, ImmutableList writablePaths, int memoryLimit, + EventHandler reporter, ImmutableSet inaccessiblePaths, ImmutableList> sandboxAdditionalMounts) { return new AutoValue_SandboxedWorker_WorkerSandboxOptions( @@ -85,6 +89,7 @@ public static WorkerSandboxOptions create( writablePaths, sandboxBinary, memoryLimit, + reporter, inaccessiblePaths, sandboxAdditionalMounts); } @@ -191,12 +196,13 @@ protected Subprocess createProcess() throws IOException, UserExecException { .setCreateNetworkNamespace(NETNS); if (hardenedSandboxOptions.memoryLimit() > 0) { - CgroupsInfo cgroupsInfo = CgroupsInfo.getInstance(); - // We put the sandbox inside a unique subdirectory using the worker's ID. - cgroupsDir = - cgroupsInfo.createMemoryLimitCgroupDir( - "worker_sandbox_" + workerId, hardenedSandboxOptions.memoryLimit()); - commandLineBuilder.setCgroupsDir(cgroupsDir); + String name = "worker_sandbox_" + workerId; + VirtualCGroup cgroup = + VirtualCGroup.getInstance(hardenedSandboxOptions.reporter()).child(name); + cgroup.memory().setMaxBytes(hardenedSandboxOptions.memoryLimit() * 1024L * 1024L); + ImmutableSet.Builder paths = ImmutableSet.builder(); + cgroup.paths().forEach(p -> paths.add(workDir.getFileSystem().getPath(p.toString()))); + commandLineBuilder.setCgroupsDirs(paths.build()); } if (this.hardenedSandboxOptions.fakeUsername()) { 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 509907e3596e5a..3243819dcac750 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 @@ -107,6 +107,7 @@ public void buildStarting(BuildStartingEvent event) { ImmutableList.copyOf(sandboxOptions.sandboxTmpfsPath), ImmutableList.copyOf(sandboxOptions.sandboxWritablePath), sandboxOptions.memoryLimitMb, + env.getReporter(), sandboxOptions.getInaccessiblePaths(env.getRuntime().getFileSystem()), ImmutableList.copyOf(sandboxOptions.sandboxAdditionalMounts)); } else { diff --git a/src/main/tools/linux-sandbox-options.cc b/src/main/tools/linux-sandbox-options.cc index 6464d72a4bde24..e44eb7212a9b2a 100644 --- a/src/main/tools/linux-sandbox-options.cc +++ b/src/main/tools/linux-sandbox-options.cc @@ -234,7 +234,8 @@ static void ParseCommandLine(unique_ptr> args) { break; case 'C': ValidateIsAbsolutePath(optarg, args->front(), static_cast(c)); - opt.cgroups_dir.assign(optarg); + opt.cgroups_dirs.emplace_back(optarg); + opt.writable_files.emplace_back(optarg); break; case 'P': opt.enable_pty = true; diff --git a/src/main/tools/linux-sandbox-options.h b/src/main/tools/linux-sandbox-options.h index a4ed85d41cc381..3880da16f79db5 100644 --- a/src/main/tools/linux-sandbox-options.h +++ b/src/main/tools/linux-sandbox-options.h @@ -66,8 +66,8 @@ struct Options { bool hermetic; // The sandbox root directory (-s) std::string sandbox_root; - // Directory to use for cgroup control - std::string cgroups_dir; + // Directories to use for cgroup control + std::vector cgroups_dirs; // Command to run (--) std::vector args; }; diff --git a/src/main/tools/linux-sandbox-pid1.cc b/src/main/tools/linux-sandbox-pid1.cc index 97457ed1641d78..f0d4d83bd30e89 100644 --- a/src/main/tools/linux-sandbox-pid1.cc +++ b/src/main/tools/linux-sandbox-pid1.cc @@ -344,7 +344,7 @@ static bool ShouldBeWritable(const std::string &mnt_dir) { return true; } - if (mnt_dir == "/sys/fs/cgroup" && !opt.cgroups_dir.empty()) { + if (mnt_dir == "/sys/fs/cgroup" && !opt.cgroups_dirs.empty()) { return true; } @@ -589,9 +589,12 @@ static int WaitForChild() { } static void AddProcessToCgroup() { - if (!opt.cgroups_dir.empty()) { - PRINT_DEBUG("Adding process to cgroups dir %s", opt.cgroups_dir.c_str()); - WriteFile(opt.cgroups_dir + "/cgroup.procs", "1"); + for(const std::string &cgroups_dir : opt.cgroups_dirs) { + PRINT_DEBUG("Adding process to cgroup dir %s", cgroups_dir.c_str()); + // Writing the value 0 to a cgroup.procs file causes the writing + // process to be moved to the corresponding cgroup. + // Ref. https://man7.org/linux/man-pages/man7/cgroups.7.html + WriteFile(cgroups_dir + "/cgroup.procs", "0"); } } diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java index d1790e620f1c1d..4e3d016d6cece1 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java @@ -28,6 +28,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 java.nio.file.Paths; import java.time.Duration; import java.util.List; import org.junit.Before; @@ -133,7 +135,7 @@ public void testLinuxSandboxCommandLineBuilder_buildsWithOptionalArguments() { .put(bindMountTarget2, bindMountSource2) .buildOrThrow(); - String cgroupsDir = "/sys/fs/cgroups/something"; + Path cgroupsDir = fileSystem.getPath("/sys/fs/cgroups/something"); ImmutableList expectedCommandLine = ImmutableList.builder() @@ -158,7 +160,7 @@ public void testLinuxSandboxCommandLineBuilder_buildsWithOptionalArguments() { .add("-U") .add("-D", sandboxDebugPath.getPathString()) .add("-p") - .add("-C", cgroupsDir) + .add("-C", cgroupsDir.toString()) .add("--") .addAll(commandArguments) .build(); @@ -180,7 +182,7 @@ public void testLinuxSandboxCommandLineBuilder_buildsWithOptionalArguments() { .setUseFakeUsername(useFakeUsername) .setSandboxDebugPath(sandboxDebugPath.getPathString()) .setPersistentProcess(true) - .setCgroupsDir(cgroupsDir) + .setCgroupsDirs(ImmutableSet.of(cgroupsDir)) .buildForCommand(commandArguments); assertThat(commandLine).containsExactlyElementsIn(expectedCommandLine).inOrder(); From db4e1b5b7634e95b5bb8ef37c9879b8f29d5a18b Mon Sep 17 00:00:00 2001 From: Alessandro Patti Date: Wed, 27 Sep 2023 12:36:32 +0200 Subject: [PATCH 2/6] [Databricks][Cgroups] Add cgroups stats --- .../devtools/build/lib/profiler/Profiler.java | 9 ++ .../sandbox/LinuxSandboxedSpawnRunner.java | 1 + .../devtools/build/lib/sandbox/cgroups/BUILD | 2 + .../build/lib/sandbox/cgroups/Controller.java | 20 +++- .../lib/sandbox/cgroups/VirtualCGroup.java | 98 ++++++++++++++++++- .../lib/sandbox/cgroups/v1/LegacyCpu.java | 11 +++ .../lib/sandbox/cgroups/v1/LegacyCpuAcct.java | 30 ++++++ .../lib/sandbox/cgroups/v1/LegacyMemory.java | 23 +++++ .../lib/sandbox/cgroups/v2/UnifiedCpu.java | 14 ++- .../lib/sandbox/cgroups/v2/UnifiedMemory.java | 28 ++++++ 10 files changed, 231 insertions(+), 5 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpuAcct.java diff --git a/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java b/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java index 24bf3b6a8d66d6..d778c938cd79db 100644 --- a/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java +++ b/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.profiler; +import static java.util.Map.entry; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; @@ -714,6 +715,14 @@ public void logEventAtTime(long atTimeNanos, ProfilerTask type, String descripti logTask(atTimeNanos, 0, type, description); } + /** Log arbitrary data. */ + public void logData(TraceData data) { + JsonTraceFileWriter writer = writerRef.get(); + if (writer != null) { + writer.enqueue(data); + } + } + /** Used to log "events" - tasks with zero duration. */ @VisibleForTesting void logEvent(ProfilerTask type, String description) { 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 1850b09d4693c4..5d1c06eb768f5a 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 @@ -540,6 +540,7 @@ public void verifyPostCondition( } Optional cgroup = cgroups.remove(context.getId()); if (cgroup != null && cgroup.isPresent()) { + cgroup.get().logStats(); // We cannot leave the cgroups around and delete them only when we delete the sandboxes // because linux has a hard limit of 65535 memory controllers. // Ref. https://github.com/torvalds/linux/blob/58d4e450a490d5f02183f6834c12550ba26d3b47/include/linux/memcontrol.h#L69 diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/BUILD b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/BUILD index aec4f8b667265c..0ba747beee38e4 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/BUILD +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/BUILD @@ -21,10 +21,12 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib/actions:exec_exception", "//src/main/java/com/google/devtools/build/lib/events", + "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/protobuf:failure_details_java_proto", "//third_party:auto_value", "//third_party:flogger", "//third_party:guava", + "//third_party:gson", "//third_party:jsr305", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java index 23aa9ed2151e87..15a6cf601cdeba 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java @@ -8,7 +8,10 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; +import java.nio.file.Files; import java.nio.file.Path; +import java.util.Map; +import java.util.stream.Collectors; public interface Controller { default boolean isLegacy() throws IOException { @@ -33,12 +36,27 @@ protected FailureDetails.FailureDetail getFailureDetail(String message) { Path getPath() throws IOException; + Path statFile() throws IOException; + + default String getStats() throws IOException { + if (statFile() != null && statFile().toFile().exists()) { + return Files.readString(statFile()); + } + return ""; + } + interface Memory extends Controller { void setMaxBytes(long bytes) throws IOException; long getMaxBytes() throws IOException; + long oomKills() throws IOException; + long maxUsage() throws IOException; } interface Cpu extends Controller { void setCpus(float cpus) throws IOException; int getCpus() throws IOException; + int getPeriod() throws IOException; + } + interface CpuAcct extends Controller { + long getUsage() throws IOException; } -} \ No newline at end of file +} diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java index c1fed31b26c8cd..0d209f1dfcedcc 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java @@ -1,5 +1,6 @@ package com.google.devtools.build.lib.sandbox.cgroups; +import com.google.gson.stream.JsonWriter; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -8,24 +9,32 @@ import com.google.common.io.Files; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.ProfilerTask; +import com.google.devtools.build.lib.profiler.TraceData; import com.google.devtools.build.lib.sandbox.cgroups.v1.LegacyCpu; +import com.google.devtools.build.lib.sandbox.cgroups.v1.LegacyCpuAcct; import com.google.devtools.build.lib.sandbox.cgroups.v1.LegacyMemory; import com.google.devtools.build.lib.sandbox.cgroups.v2.UnifiedCpu; import com.google.devtools.build.lib.sandbox.cgroups.v2.UnifiedMemory; import javax.annotation.Nullable; +import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.StringReader; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Queue; import java.util.Scanner; import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.TimeUnit; /** @@ -44,6 +53,9 @@ public abstract class VirtualCGroup { public abstract Controller.Cpu cpu(); public abstract Controller.Memory memory(); + @Nullable + public abstract Controller.CpuAcct cpuacct(); + public abstract ImmutableSet paths(); private final Queue children = new ConcurrentLinkedQueue<>(); @@ -100,6 +112,7 @@ static VirtualCGroup create(File procMounts, File procCgroup, EventHandler repor Controller.Memory memory = null; Controller.Cpu cpu = null; + Controller.CpuAcct cpuacct = null; ImmutableSet.Builder paths = ImmutableSet.builder(); for (Mount m: mounts) { @@ -172,6 +185,11 @@ static VirtualCGroup create(File procMounts, File procCgroup, EventHandler repor logger.atInfo().log("Found cgroup v1 cpu controller at %s", cgroup); cpu = new LegacyCpu(cgroup); break; + case "cpuacct": + if (cpuacct != null) continue; + logger.atInfo().log("Found cgroup v1 cpuacct controller at %s", cgroup); + cpuacct = new LegacyCpuAcct(cgroup); + break; } } } @@ -179,7 +197,7 @@ static VirtualCGroup create(File procMounts, File procCgroup, EventHandler repor cpu = cpu != null ? cpu : Controller.getDefault(Controller.Cpu.class); memory = memory != null ? memory : Controller.getDefault(Controller.Memory.class); - VirtualCGroup vcgroup = new AutoValue_VirtualCGroup(cpu, memory, paths.build()); + VirtualCGroup vcgroup = new AutoValue_VirtualCGroup(cpu, memory, cpuacct, paths.build()); Runtime.getRuntime().addShutdownHook(new Thread(() -> vcgroup.delete())); return vcgroup; } @@ -192,6 +210,7 @@ public void delete() { public VirtualCGroup child(String name) throws IOException { Controller.Cpu cpu = Controller.getDefault(Controller.Cpu.class); Controller.Memory memory = Controller.getDefault(Controller.Memory.class); + Controller.CpuAcct cpuacct = null; ImmutableSet.Builder paths = ImmutableSet.builder(); if (memory() != null && memory().getPath() != null) { copyControllersToSubtree(memory().getPath()); @@ -207,8 +226,83 @@ public VirtualCGroup child(String name) throws IOException { cpu = cpu().isLegacy() ? new LegacyCpu(cgroup) : new UnifiedCpu(cgroup); paths.add(cgroup); } - VirtualCGroup child = new AutoValue_VirtualCGroup(cpu, memory, paths.build()); + if (cpuacct() != null && cpuacct().getPath() != null) { + Path cgroup = cpuacct().getPath().resolve(name); + cgroup.toFile().mkdirs(); + cpuacct = new LegacyCpuAcct(cgroup); + paths.add(cgroup); + } + VirtualCGroup child = new AutoValue_VirtualCGroup(cpu, memory, cpuacct, paths.build()); this.children.add(child); return child; } + + final class StatsData implements TraceData { + @Override + public void writeTraceData(JsonWriter jsonWriter, long profileStartTimeNanos) throws IOException { + long timestamp = TimeUnit.NANOSECONDS.toMicros(System.nanoTime() - profileStartTimeNanos); + if (cpu() != null || cpuacct() != null) { + var stats = new LinkedHashMap(); + if (cpu() != null) { + try (BufferedReader reader = new BufferedReader(new StringReader(cpu().getStats()))) { + String line; + while ((line = reader.readLine()) != null) { + String[] parts = line.split(" ", 2); + stats.put(parts[0], parts[1]); + } + } + stats.put("quota", String.valueOf(cpu().getCpus())); + stats.put("period", String.valueOf(cpu().getPeriod())); + } + if (cpuacct() != null) { + try (BufferedReader reader = new BufferedReader(new StringReader(cpuacct().getStats()))) { + String line; + while ((line = reader.readLine()) != null) { + String[] parts = line.split(" ", 2); + Double value = Long.parseLong(parts[1]) * 1e6 / LegacyCpuAcct.USER_HZ; + stats.put(parts[0] + "_usec", String.valueOf(value.longValue())); + } + } + stats.put("usage_usec", String.valueOf(cpuacct().getUsage() / 1000)); + } + + writeStats(jsonWriter, timestamp, "CPU stats (Sandbox)", stats); + } + if (memory() != null) { + var stats = new LinkedHashMap(); + Long kills = memory().oomKills(); + Long limit = memory().getMaxBytes(); + Long usage = memory().maxUsage(); + if (usage > 0) stats.put("max_usage_in_bytes", String.valueOf(usage)); + if (limit > 0) stats.put("limit_in_bytes", String.valueOf(limit)); + if (kills > 0) stats.put("oom_kills", String.valueOf(kills)); + writeStats(jsonWriter, timestamp, "Memory stats (Sandbox)", stats); + } + } + + void writeStats(JsonWriter writer, long timestamp, String name, Map stats) throws IOException { + var currentThread = Thread.currentThread(); + var threadId = currentThread.threadId(); + writer.setIndent(" "); + writer.beginObject(); + writer.setIndent(""); + writer.name("cat").value("sandbox info"); + writer.name("name").value(name); + writer.name("args"); + writer.beginObject(); + for (var entry : stats.entrySet()) { + writer.name(entry.getKey()).value(entry.getValue()); + } + writer.endObject(); + writer.name("ph").value("i"); + writer.name("ts").value(timestamp); + writer.name("pid").value(1); + writer.name("tid").value(threadId); + writer.endObject(); + } + } + + public void logStats() throws IOException { + Profiler.instance().logData(new StatsData()); + } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpu.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpu.java index cdcff1643ba070..f5509a23bcd677 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpu.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpu.java @@ -5,6 +5,8 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Map; +import java.util.stream.Collectors; public class LegacyCpu implements Controller.Cpu { private final Path path; @@ -20,6 +22,11 @@ public Path getPath() { return path; } + @Override + public Path statFile() throws IOException { + return path.resolve("cpu.stat"); + } + @Override public void setCpus(float cpus) throws IOException { int quota = Math.round(cpus * period); @@ -30,4 +37,8 @@ public void setCpus(float cpus) throws IOException { public int getCpus() throws IOException { return Integer.parseInt(Files.readString(path.resolve("cpu.cfs_quota_us")).trim()); } + + public int getPeriod() throws IOException { + return this.period; + } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpuAcct.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpuAcct.java new file mode 100644 index 00000000000000..3a144aa751d87d --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpuAcct.java @@ -0,0 +1,30 @@ +package com.google.devtools.build.lib.sandbox.cgroups.v1; + +import com.google.devtools.build.lib.sandbox.cgroups.Controller; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +public class LegacyCpuAcct implements Controller.CpuAcct { + private final Path path; + // TODO get this value from the system + public static long USER_HZ = 100; + + public LegacyCpuAcct(Path path) { + this.path = path; + } + + @Override + public Path getPath() throws IOException { + return path; + } + + @Override + public Path statFile() throws IOException { + return path.resolve("cpuacct.stat"); + } + + public long getUsage() throws IOException { + return Long.parseLong(Files.readString(path.resolve("cpuacct.usage")).trim()); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java index 873f728ccfde19..7691f493461422 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java @@ -5,6 +5,9 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; public class LegacyMemory implements Controller.Memory { private final Path path; @@ -14,6 +17,11 @@ public Path getPath() { return path; } + @Override + public Path statFile() throws IOException { + return path.resolve("memory.stat"); + } + public LegacyMemory(Path path) { this.path = path; } @@ -27,4 +35,19 @@ public void setMaxBytes(long bytes) throws IOException { public long getMaxBytes() throws IOException { return Long.parseLong(Files.readString(path.resolve("memory.limit_in_bytes")).trim()); } + + @Override + public long oomKills() throws IOException { + for (String line: Files.readAllLines(getPath().resolve("memory.oom_control"))) { + if (line.startsWith("oom_kill ")) { + return Long.parseLong(line.substring(line.indexOf(" ") + 1)); + } + } + return -1; + } + + @Override + public long maxUsage() throws IOException { + return Long.parseLong(Files.readString(path.resolve("memory.max_usage_in_bytes")).trim()); + } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedCpu.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedCpu.java index a3711f845c80cb..1b3675d580fa52 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedCpu.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedCpu.java @@ -8,8 +8,10 @@ public class UnifiedCpu implements Controller.Cpu { private final Path path; - public UnifiedCpu(Path path) { + private final int period; + public UnifiedCpu(Path path) throws IOException { this.path = path; + this.period = Integer.parseInt(Files.readString(path.resolve("cpu.max")).split(" ", 2)[1]); } @Override @@ -17,9 +19,13 @@ public Path getPath() { return path; } + @Override + public Path statFile() throws IOException { + return path.resolve("cpu.stat"); + } + @Override public void setCpus(float cpus) throws IOException { - int period = 1000_000; int quota = Math.round(period * cpus); String limit = String.format("%d %d", quota, period); Files.writeString(path.resolve("cpu.max"), limit); @@ -29,4 +35,8 @@ public void setCpus(float cpus) throws IOException { public int getCpus() throws IOException { return Integer.parseInt(Files.readString(path.resolve("cpu.max")).trim()); } + + public int getPeriod() { + return period; + } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java index 3e6e23e1d7e9bb..d8f585c68f94f5 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java @@ -5,6 +5,8 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Map; +import java.util.stream.Collectors; public class UnifiedMemory implements Controller.Memory { private final Path path; @@ -17,6 +19,11 @@ public Path getPath() { return path; } + @Override + public Path statFile() throws IOException { + return path.resolve("memory.stat"); + } + @Override public void setMaxBytes(long bytes) throws IOException { Files.writeString(path.resolve("memory.max"), Long.toString(bytes)); @@ -26,4 +33,25 @@ public void setMaxBytes(long bytes) throws IOException { public long getMaxBytes() throws IOException { return Long.parseLong(Files.readString(path.resolve("memory.max")).trim()); } + + @Override + public long oomKills() throws IOException { + for (String line: Files.readAllLines(getPath().resolve("memory.events"))) { + if (line.startsWith("oom_kill ")) { + return Long.parseLong(line.substring(line.indexOf(" ") + 1)); + } + } + return -1; + } + + @Override + public long maxUsage() throws IOException { + // This file has been added relatively recently, so it might not exist. + // Return -1 in that case, to signal its absence + // Ref. https://github.com/torvalds/linux/commit/8e20d4b332660a32e842e20c34cfc3b3456bc6dc + if (path.resolve("memory.peak").toFile().exists()) { + return Long.parseLong(Files.readString(path.resolve("memory.peak")).trim()); + } + return -1; + } } From 714b2ebc8df0b08cffc86ff29de3763beecbbad8 Mon Sep 17 00:00:00 2001 From: Alessandro Patti Date: Thu, 12 Oct 2023 11:41:36 +0200 Subject: [PATCH 3/6] [Databricks][Cgroups] Monitor memory stats --- .../sandbox/LinuxSandboxedSpawnRunner.java | 1 + .../build/lib/sandbox/SandboxOptions.java | 11 +++ .../build/lib/sandbox/cgroups/Controller.java | 2 +- .../build/lib/sandbox/cgroups/Monitor.java | 94 +++++++++++++++++++ .../lib/sandbox/cgroups/VirtualCGroup.java | 14 ++- .../lib/sandbox/cgroups/v1/LegacyMemory.java | 13 ++- .../lib/sandbox/cgroups/v2/UnifiedMemory.java | 13 ++- 7 files changed, 136 insertions(+), 12 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Monitor.java 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 5d1c06eb768f5a..4960f8e873fa47 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 @@ -409,6 +409,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context Optional cgroup = getCgroup(spawn, context); if (cgroup.isPresent()) { + cgroup.get().memory().monitor().start(sandboxOptions.memoryMonitored); commandLineBuilder.setCgroupsDirs( cgroup.get().paths().stream() .map(p -> fileSystem.getPath(p.toString())) diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java index f5070b2d830bb4..459d242ddb87ee 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.Converter; +import com.google.devtools.common.options.Converters; import com.google.devtools.common.options.Converters.TriStateConverter; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; @@ -371,6 +372,16 @@ public ImmutableSet getInaccessiblePaths(FileSystem fs) { + " --sandbox_add_mount_pair=/tmp to keep seeing the host's /tmp in all sandboxes.") public boolean sandboxHermeticTmp; + @Option( + name = "experimental_sandbox_resources_monitor", + defaultValue = "", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + converter = Converters.CommaSeparatedOptionListConverter.class, + help = + "Enables active monitoring of the values in the memory.stat interface of cgroups.") + public List memoryMonitored; + @Option( name = "experimental_sandbox_memory_limit_mb", defaultValue = "0", diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java index 15a6cf601cdeba..f40387001e5648 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java @@ -45,7 +45,7 @@ default String getStats() throws IOException { return ""; } - interface Memory extends Controller { + interface Memory extends Controller, Monitor.Monitorable { void setMaxBytes(long bytes) throws IOException; long getMaxBytes() throws IOException; long oomKills() throws IOException; diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Monitor.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Monitor.java new file mode 100644 index 00000000000000..334a3d2a5f3206 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Monitor.java @@ -0,0 +1,94 @@ +package com.google.devtools.build.lib.sandbox.cgroups; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.util.concurrent.ThreadFactoryBuilder; + +import java.io.IOException; +import java.time.Duration; +import java.util.HashMap; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + +public class Monitor { + private static String PREFIX = "max_mon_"; + + private volatile boolean active; + + private final HashMap max; + private final Controller controller; + private final ExecutorService executor; + + interface Monitorable { + Monitor monitor(); + } + + public static class MonitorFactory { + private final ThreadPoolExecutor executor; + + public MonitorFactory() { + this.executor = new ThreadPoolExecutor( + 0, + Integer.MAX_VALUE, + 10, + TimeUnit.SECONDS, + new SynchronousQueue<>(), + new ThreadFactoryBuilder().setNameFormat("cgroup-%d").build()); + } + + public Monitor create(Controller controller) { + return new Monitor(controller, executor); + } + + + } + + private Monitor(Controller contoller, ExecutorService executor) { + this.controller = contoller; + this.executor = executor; + this.max = new HashMap<>(); + } + + public void start(List fields) { + if (fields.isEmpty()) { + return; + } + start(Duration.ofSeconds(1), ImmutableSet.copyOf(fields)); + } + + private void start(Duration interval, ImmutableSet fields) { + Preconditions.checkArgument(!active, "Monitoring started twice"); + active = true; + executor.execute(() -> { + synchronized (this.max) { + while (active) { + try { + for (String stat : controller.getStats().split("\n")) { + String[] parts = stat.split(" ", 2); + if (parts.length < 2) continue; + Long value = Long.parseLong(parts[1]); + if (value > 0 && fields.contains(parts[0])) { + Long current = max.getOrDefault(PREFIX + parts[0], 0L); + max.put(PREFIX + parts[0], Math.max(current, value)); + } + } + Thread.sleep(interval.toMillis()); + } catch (IOException | InterruptedException e) { + break; + } + } + } + }); + } + + public ImmutableMap stop() { + this.active = false; + synchronized (this.max) { + return ImmutableMap.copyOf(this.max); + } + } +} \ No newline at end of file diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java index 0d209f1dfcedcc..8831cf1031fb3d 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java @@ -48,6 +48,8 @@ public abstract class VirtualCGroup { private static final File PROC_SELF_MOUNTS_PATH = new File("/proc/self/mounts"); private static final File PROC_SELF_CGROUP_PATH = new File("/proc/self/cgroup"); + private static final Monitor.MonitorFactory factory = new Monitor.MonitorFactory(); + @Nullable private static volatile VirtualCGroup instance; @@ -152,7 +154,7 @@ static VirtualCGroup create(File procMounts, File procCgroup, EventHandler repor case "memory": if (memory != null) continue; logger.atInfo().log("Found cgroup v2 memory controller at %s", cgroup); - memory = new UnifiedMemory(cgroup); + memory = new UnifiedMemory(cgroup, factory); break; case "cpu": if (cpu != null) continue; @@ -178,7 +180,7 @@ static VirtualCGroup create(File procMounts, File procCgroup, EventHandler repor case "memory": if (memory != null) continue; logger.atInfo().log("Found cgroup v1 memory controller at %s", cgroup); - memory = new LegacyMemory(cgroup); + memory = new LegacyMemory(cgroup, factory); break; case "cpu": if (cpu != null) continue; @@ -216,7 +218,9 @@ public VirtualCGroup child(String name) throws IOException { copyControllersToSubtree(memory().getPath()); Path cgroup = memory().getPath().resolve(name); cgroup.toFile().mkdirs(); - memory = memory().isLegacy() ? new LegacyMemory(cgroup) : new UnifiedMemory(cgroup); + memory = memory().isLegacy() ? + new LegacyMemory(cgroup, factory) : + new UnifiedMemory(cgroup, factory); paths.add(cgroup); } if (cpu() != null && cpu().getPath() != null) { @@ -265,7 +269,9 @@ public void writeTraceData(JsonWriter jsonWriter, long profileStartTimeNanos) th } stats.put("usage_usec", String.valueOf(cpuacct().getUsage() / 1000)); } - + for (Map.Entry stat : memory().monitor().stop().entrySet()) { + stats.put(stat.getKey(), String.valueOf(stat.getValue())); + } writeStats(jsonWriter, timestamp, "CPU stats (Sandbox)", stats); } if (memory() != null) { diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java index 7691f493461422..d34b1997a923ef 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java @@ -1,16 +1,15 @@ package com.google.devtools.build.lib.sandbox.cgroups.v1; import com.google.devtools.build.lib.sandbox.cgroups.Controller; +import com.google.devtools.build.lib.sandbox.cgroups.Monitor; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; public class LegacyMemory implements Controller.Memory { private final Path path; + private final Monitor monitor; @Override public Path getPath() { @@ -22,8 +21,9 @@ public Path statFile() throws IOException { return path.resolve("memory.stat"); } - public LegacyMemory(Path path) { + public LegacyMemory(Path path, Monitor.MonitorFactory factory) { this.path = path; + this.monitor = factory.create(this); } @Override @@ -50,4 +50,9 @@ public long oomKills() throws IOException { public long maxUsage() throws IOException { return Long.parseLong(Files.readString(path.resolve("memory.max_usage_in_bytes")).trim()); } + + @Override + public Monitor monitor() { + return monitor; + } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java index d8f585c68f94f5..1d6eb10625ffbb 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java @@ -1,17 +1,19 @@ package com.google.devtools.build.lib.sandbox.cgroups.v2; import com.google.devtools.build.lib.sandbox.cgroups.Controller; +import com.google.devtools.build.lib.sandbox.cgroups.Monitor; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Map; -import java.util.stream.Collectors; public class UnifiedMemory implements Controller.Memory { private final Path path; - public UnifiedMemory(Path path) { + private volatile Monitor monitor; + + public UnifiedMemory(Path path, Monitor.MonitorFactory factory) { this.path = path; + this.monitor = factory.create(this); } @Override @@ -54,4 +56,9 @@ public long maxUsage() throws IOException { } return -1; } + + @Override + public Monitor monitor() { + return this.monitor; + } } From db16db3cf78c0a58d22fb42b8169247177ab27f3 Mon Sep 17 00:00:00 2001 From: Gabriel Russo Date: Fri, 24 Nov 2023 13:41:04 +0100 Subject: [PATCH 4/6] [Databricks][bre] Flag to control pools based on test sizes (#16) * [bre] Flag to control pools based on test sizes * more docs --- .../lib/analysis/test/TestConfiguration.java | 18 +++++ .../lib/analysis/test/TestRunnerAction.java | 23 ++++++ .../packages/DatabricksEngflowTestPool.java | 70 +++++++++++++++++ ...atabricksEngflowTestPoolConverterTest.java | 76 +++++++++++++++++++ src/test/shell/integration/exec_group_test.sh | 31 ++++++++ 5 files changed, 218 insertions(+) create mode 100644 src/main/java/com/google/devtools/build/lib/packages/DatabricksEngflowTestPool.java create mode 100644 src/test/java/com/google/devtools/build/lib/analysis/test/DatabricksEngflowTestPoolConverterTest.java diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java index 562ea3bca15675..fa40b8118ee4f6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.test.CoverageConfiguration.CoverageOptions; import com.google.devtools.build.lib.analysis.test.TestShardingStrategy.ShardingStrategyConverter; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.DatabricksEngflowTestPool; import com.google.devtools.build.lib.packages.TestSize; import com.google.devtools.build.lib.packages.TestTimeout; import com.google.devtools.build.lib.util.Pair; @@ -86,6 +87,19 @@ public static class TestOptions extends FragmentOptions { + "-1 tells blaze to use its default timeouts for that category.") public Map testTimeout; + @Option( + name = "databricks_engflow_test_pools", + defaultValue = "null", + converter = DatabricksEngflowTestPool.DatabricksEngflowTestPoolConverter.class, + documentationCategory = OptionDocumentationCategory.TESTING, + effectTags = {OptionEffectTag.EXECUTION}, + help = "DATABRICKS ONLY: Override the default Engflow Remote Execution worker pool for test sizes. " + + "This is done by setting the Pool execution property of TestRunner actions. This has LESS precedence " + + "than targets and platforms that set the Pool property." + + "If a single string is specified it will override all sizes. If 4 comma-separated strings " + + "are specified, they will override the pools for small, medium, large and enormous (in that order).") + public Map databricksEngflowTestPools; + @Option( name = "default_test_resources", defaultValue = "null", @@ -451,6 +465,10 @@ public boolean checkShardingSupport() { return options.checkShardingSupport; } + public Map getDatabricksEngflowTestPools() { + return options.databricksEngflowTestPools; + } + /** * Option converter that han handle two styles of value for "--runs_per_test": * diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index 01cf8da909fabe..ffa9aca215cf5e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java @@ -60,6 +60,8 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.packages.DatabricksEngflowTestPool; +import com.google.devtools.build.lib.packages.TestSize; import com.google.devtools.build.lib.server.FailureDetails.Execution.Code; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.TestAction; @@ -1066,6 +1068,27 @@ public NestedSet getPossibleInputsForTesting() { return getInputs(); } + private static final String ENGFLOW_POOL_EXEC_PROPERTY = "Pool"; + + /** + * Databricks override: make sure to send actions to the correct Bazel Remote execution pools based on test sizes. + */ + @Override + public ImmutableMap getExecProperties() { + ImmutableMap execProperties = super.getExecProperties(); + Map pools = testConfiguration.getDatabricksEngflowTestPools(); + if (execProperties.containsKey(ENGFLOW_POOL_EXEC_PROPERTY) || pools == null) { + return execProperties; + } + return ImmutableMap.builder() + .putAll(execProperties) + .put( + ENGFLOW_POOL_EXEC_PROPERTY, + pools.get(testProperties.getSize()).getPool() + ) + .build(); + } + /** The same set of paths as the parent test action, resolved against a given exec root. */ public final class ResolvedPaths { private final Path execRoot; diff --git a/src/main/java/com/google/devtools/build/lib/packages/DatabricksEngflowTestPool.java b/src/main/java/com/google/devtools/build/lib/packages/DatabricksEngflowTestPool.java new file mode 100644 index 00000000000000..5d837c14be4821 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/DatabricksEngflowTestPool.java @@ -0,0 +1,70 @@ +package com.google.devtools.build.lib.packages; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Splitter; +import com.google.devtools.common.options.Converter; +import com.google.devtools.common.options.OptionsParsingException; + +import java.util.*; + +/** + * DATABRICKS ONLY: Represents a pool of Engflow Remote Execution worker pools. + * Borrows heavily from {@link TestTimeout}. + */ +public class DatabricksEngflowTestPool { + private final String pool; + + public DatabricksEngflowTestPool(String pool) { + this.pool = pool; + } + + public String getPool() { + return pool; + } + + public static class DatabricksEngflowTestPoolConverter extends Converter.Contextless> { + + @Override + public String getTypeDescription() { + return "a single string or comma-separated list of 4 strings"; + } + + @Override + public Map convert(String input) throws OptionsParsingException { + List values = new ArrayList<>(); + for (String token: Splitter.on(",").limit(6).split(input)) { + if (!token.isEmpty() || values.size() > 1) { + values.add(new DatabricksEngflowTestPool(token)); + } + } + EnumMap pools = new EnumMap<>(TestSize.class); + if (values.size() == 1) { + pools.put(TestSize.SMALL, values.get(0)); + pools.put(TestSize.MEDIUM, values.get(0)); + pools.put(TestSize.LARGE, values.get(0)); + pools.put(TestSize.ENORMOUS, values.get(0)); + } else if (values.size() == 4) { + pools.put(TestSize.SMALL, values.get(0)); + pools.put(TestSize.MEDIUM, values.get(1)); + pools.put(TestSize.LARGE, values.get(2)); + pools.put(TestSize.ENORMOUS, values.get(3)); + } else { + throw new OptionsParsingException("Invalid number of comma-separated entries"); + } + return pools; + } + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DatabricksEngflowTestPool that = (DatabricksEngflowTestPool) o; + return Objects.equals(pool, that.pool); + } + + @Override + public int hashCode() { + return Objects.hash(pool); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/test/DatabricksEngflowTestPoolConverterTest.java b/src/test/java/com/google/devtools/build/lib/analysis/test/DatabricksEngflowTestPoolConverterTest.java new file mode 100644 index 00000000000000..406c09a7d80761 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/analysis/test/DatabricksEngflowTestPoolConverterTest.java @@ -0,0 +1,76 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.analysis.test; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import com.google.devtools.build.lib.packages.DatabricksEngflowTestPool; +import com.google.devtools.build.lib.packages.DatabricksEngflowTestPool.DatabricksEngflowTestPoolConverter; +import com.google.devtools.build.lib.packages.TestSize; +import com.google.devtools.common.options.OptionsParsingException; +import java.util.Map; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * A test for {@link DatabricksEngflowTestPoolConverter}. + */ +@RunWith(JUnit4.class) +public class DatabricksEngflowTestPoolConverterTest { + private Map pools; + + protected void setPools(String option) throws OptionsParsingException { + pools = new DatabricksEngflowTestPoolConverter().convert(option); + } + + protected void assertPool(TestSize size, String expected) { + assertThat(pools).containsEntry(size, new DatabricksEngflowTestPool(expected)); + } + + protected void assertFailure(String option) { + assertThrows( + "Incorrectly parsed '" + option + "'", + OptionsParsingException.class, + () -> setPools(option)); + } + + @Test + public void testUniversalPool() throws Exception { + setPools("same"); + assertPool(TestSize.SMALL, "same"); + assertPool(TestSize.MEDIUM, "same"); + assertPool(TestSize.LARGE, "same"); + assertPool(TestSize.ENORMOUS, "same"); + } + + @Test + public void testSeparatePools() throws Exception { + setPools("small,large,big,why"); + assertPool(TestSize.SMALL, "small"); + assertPool(TestSize.MEDIUM, "large"); + assertPool(TestSize.LARGE, "big"); + assertPool(TestSize.ENORMOUS, "why"); + } + + @Test + public void testIncorrectStrings() { + assertFailure(""); + assertFailure("1,2,3"); + assertFailure("1,2,,3,4"); + assertFailure("1,2,3 4"); + assertFailure("1,2,3,4,5"); + } +} diff --git a/src/test/shell/integration/exec_group_test.sh b/src/test/shell/integration/exec_group_test.sh index 029787cf6b886d..b0e71c3db341d3 100755 --- a/src/test/shell/integration/exec_group_test.sh +++ b/src/test/shell/integration/exec_group_test.sh @@ -988,4 +988,35 @@ EOF grep "testvalue" out.txt || fail "Did not find the platform value" } +function test_databricks_engflow_test_pools() { + local -r pkg=${FUNCNAME[0]} + mkdir $pkg || fail "mkdir $pkg" + + cat > ${pkg}/true.sh < ${pkg}/BUILD < $TEST_log || fail "Test execution failed" + grep "Pool" out.txt || fail "Pool key not found" + grep "manual" out.txt || fail "manual prop not found" + grep "three" out.txt || fail "three prop not found" +} + run_suite "exec group test" From 4530ad2f596ddee3ea3cd60b47976a968b92cc66 Mon Sep 17 00:00:00 2001 From: Alessandro Patti Date: Thu, 22 Aug 2024 05:21:22 -0700 Subject: [PATCH 5/6] Add cgroup startup option This options will have the server start in the user-specifed cgroup. This is useful when using the cgroup features in bazel and thus the server must be started in a user-writable cgroup. Closes #21618. PiperOrigin-RevId: 666305298 Change-Id: Ide3bb3f65bd11f4ecc11d6e843926a16ecb41f2f --- src/main/cpp/blaze.cc | 7 ++ src/main/cpp/blaze_util_posix.cc | 6 ++ src/main/cpp/startup_options.cc | 10 +++ src/main/cpp/startup_options.h | 4 ++ .../runtime/BlazeServerStartupOptions.java | 26 +++++++ src/main/tools/BUILD | 5 +- src/main/tools/{daemonize.c => daemonize.cc} | 71 ++++++++++++++++--- 7 files changed, 118 insertions(+), 11 deletions(-) rename src/main/tools/{daemonize.c => daemonize.cc} (76%) diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index de31219c4fdc55..a9b70122be9a44 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -506,6 +506,13 @@ static vector GetServerExeArgs(const blaze_util::Path &jvm_path, result.push_back("--product_name=" + startup_options.product_name); +#ifdef __linux__ + if (!startup_options.cgroup_parent.empty()) { + result.push_back("--experimental_cgroup_parent=" + + startup_options.cgroup_parent); + } +#endif + startup_options.AddExtraOptions(&result); // The option sources are transmitted in the following format: diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc index 2793572269886a..5145c8331f996d 100644 --- a/src/main/cpp/blaze_util_posix.cc +++ b/src/main/cpp/blaze_util_posix.cc @@ -409,6 +409,12 @@ int ExecuteDaemon(const blaze_util::Path& exe, if (daemon_output_append) { daemonize_args.push_back("-a"); } +#ifdef __linux__ + if (!options.cgroup_parent.empty()) { + daemonize_args.push_back("-c"); + daemonize_args.push_back(options.cgroup_parent); + } +#endif daemonize_args.push_back("--"); daemonize_args.push_back(exe.AsNativePath()); std::copy(args_vector.begin(), args_vector.end(), diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc index b71e974e25164e..98087a9a3fbfef 100644 --- a/src/main/cpp/startup_options.cc +++ b/src/main/cpp/startup_options.cc @@ -97,6 +97,9 @@ StartupOptions::StartupOptions(const string &product_name, macos_qos_class(QOS_CLASS_UNSPECIFIED), #endif unlimit_coredumps(false), +#ifdef __linux__ + cgroup_parent(), +#endif windows_enable_symlinks(false) { // To ensure predictable behavior from PathFragmentConverter in Java, // output_root must be an absolute path. In particular, if we were to return a @@ -169,6 +172,7 @@ StartupOptions::StartupOptions(const string &product_name, RegisterUnaryStartupFlag("output_user_root"); RegisterUnaryStartupFlag("server_jvm_out"); RegisterUnaryStartupFlag("failure_detail_out"); + RegisterUnaryStartupFlag("experimental_cgroup_parent"); } StartupOptions::~StartupOptions() {} @@ -390,6 +394,12 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg( "multiple times."; return blaze_exit_code::BAD_ARGV; } + } else if ((value = GetUnaryOption( + arg, next_arg, "--experimental_cgroup_parent")) != nullptr) { +#ifdef __linux__ + cgroup_parent = value; + option_sources["cgroup_parent"] = rcfile; +#endif } else { bool extra_argument_processed; blaze_exit_code::ExitCode process_extra_arg_exit_code = ProcessArgExtra( diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h index 208e4bc6a96af4..7bc0b1666a048e 100644 --- a/src/main/cpp/startup_options.h +++ b/src/main/cpp/startup_options.h @@ -275,6 +275,10 @@ class StartupOptions { // Whether to raise the soft coredump limit to the hard one or not. bool unlimit_coredumps; +#ifdef __linux__ + std::string cgroup_parent; +#endif + // Whether to create symbolic links on Windows for files. Requires // developer mode to be enabled. bool windows_enable_symlinks; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java index 00b01259d8965b..33b0273bd44dfa 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java @@ -499,4 +499,30 @@ public String getTypeDescription() { "When --noautodetect_server_javabase is passed, Bazel does not fall back to the local " + "JDK for running the bazel server and instead exits.") public boolean autodetectServerJavabase; + + /** + * Note: This option is only used by the C++ client, never by the Java server. It is included here + * to make sure that the option is documented in the help output, which is auto-generated by Java + * code. This also helps ensure that the server is killed if the value of this option changes. + */ + @Option( + name = "experimental_cgroup_parent", + defaultValue = "null", + documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, + effectTags = { + OptionEffectTag.BAZEL_MONITORING, + OptionEffectTag.EXECUTION, + }, + valueHelp = "", + help = + "The cgroup where to start the bazel server as an absolute path. The server " + + "process will be started in the specified cgroup for each supported controller. " + + "For example, if the value of this flag is /build/bazel and the cpu and memory " + + "controllers are mounted respectively on /sys/fs/cgroup/cpu and " + + "/sys/fs/cgroup/memory, the server will be started in the cgroups " + + "/sys/fs/cgroup/cpu/build/bazel and /sys/fs/cgroup/memory/build/bazel." + + "It is not an error if the specified cgroup is not writable for one or more " + + "of the controllers. This options does not have any effect on " + + "platforms that do not support cgroups.") + public String cgroupParent; } diff --git a/src/main/tools/BUILD b/src/main/tools/BUILD index 058a6c5e9f47ae..c1d3db9174b97d 100644 --- a/src/main/tools/BUILD +++ b/src/main/tools/BUILD @@ -2,7 +2,10 @@ package(default_visibility = ["//src:__subpackages__"]) cc_binary( name = "daemonize", - srcs = ["daemonize.c"], + srcs = ["daemonize.cc"], + deps = [ + ":process-tools", + ], ) cc_library( diff --git a/src/main/tools/daemonize.c b/src/main/tools/daemonize.cc similarity index 76% rename from src/main/tools/daemonize.c rename to src/main/tools/daemonize.cc index 1188b67eb19b57..624e5ea8901661 100644 --- a/src/main/tools/daemonize.c +++ b/src/main/tools/daemonize.cc @@ -12,7 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -// daemonize [-a] -l log_path -p pid_path -- binary_path binary_name [args] +// daemonize [-a] -l log_path -p pid_path [-c cgroup] -- binary_path binary_name +// [args] // // daemonize spawns a program as a daemon, redirecting all of its output to the // given log_path and writing the daemon's PID to pid_path. binary_path @@ -35,20 +36,23 @@ // we take the freedom to immediatey exit from anywhere as soon as we // hit an error. -#include - #include #include #include #include #include -#include #include +#include #include #include #include +#include +#include #include +#include "src/main/tools/process-tools.h" +#include "src/main/tools/logging.h" + // Configures std{in,out,err} of the current process to serve as a daemon. // // stdin is configured to read from /dev/null. @@ -89,11 +93,11 @@ static void WritePidFile(pid_t pid, const char* pid_path, int pid_done_fd) { if (pid_file == NULL) { err(EXIT_FAILURE, "Failed to create %s", pid_path); } - if (fprintf(pid_file, "%" PRIdMAX, (intmax_t) pid) < 0) { - err(EXIT_FAILURE, "Failed to write pid %"PRIdMAX" to %s", (intmax_t) pid, pid_path); + if (fprintf(pid_file, "%d", pid) < 0) { + err(EXIT_FAILURE, "Failed to write pid %d to %s", pid, pid_path); } if (fclose(pid_file) < 0) { - err(EXIT_FAILURE, "Failed to write pid %"PRIdMAX" to %s", (intmax_t) pid, pid_path); + err(EXIT_FAILURE, "Failed to write pid %d to %s", pid, pid_path); } char dummy = '\0'; @@ -147,6 +151,42 @@ static void ExecAsDaemon(const char* log_path, bool log_append, int pid_done_fd, err(EXIT_FAILURE, "Failed to execute %s", exe); } +#ifdef __linux__ +// Moves the bazel server into the specified cgroup for all the discovered +// cgroups. This is useful when using the cgroup features in bazel and thus the +// server must be started in a user-writable cgroup. Users can specify a +// pre-setup cgroup where the server will be moved to. This is enabled by +// the --experimental_cgroup_parent startup flag. +static void MoveToCgroup(pid_t pid, const char* cgroup_path) { + FILE* mounts_fp = fopen("/proc/self/mounts", "r"); + if (mounts_fp == NULL) { + err(EXIT_FAILURE, "Failed to open /proc/self/mounts"); + } + + char* line = NULL; + size_t len = 0; + while (getline(&line, &len, mounts_fp) != -1) { + char* saveptr; + strtok_r(line, " ", &saveptr); + char* fs_file = strtok_r(NULL, " ", &saveptr); + char* fs_vfstype = strtok_r(NULL, " ", &saveptr); + if (strcmp(fs_vfstype, "cgroup") == 0 || + strcmp(fs_vfstype, "cgroup2") == 0) { + char* procs_path; + asprintf(&procs_path, "%s%s/cgroup.procs", fs_file, cgroup_path); + FILE* procs = fopen(procs_path, "w"); + if (procs != NULL) { + fprintf(procs, "%d", pid); + fclose(procs); + } + free(procs_path); + } + } + free(line); + fclose(mounts_fp); +} +#endif + // Starts the given process as a daemon. // // This spawns a subprocess that will be configured to run the desired program @@ -154,7 +194,8 @@ static void ExecAsDaemon(const char* log_path, bool log_append, int pid_done_fd, // are given in the NULL-terminated argv. argv[0] must be present and // contain the program name (which may or may not match the basename of exe). static void Daemonize(const char* log_path, bool log_append, - const char* pid_path, const char* exe, char** argv) { + const char* pid_path, const char* cgroup_path, + const char* exe, char** argv) { assert(argv[0] != NULL); int pid_done_fds[2]; @@ -167,6 +208,11 @@ static void Daemonize(const char* log_path, bool log_append, err(EXIT_FAILURE, "fork failed"); } else if (pid == 0) { close(pid_done_fds[1]); +#ifdef __linux__ + if (cgroup_path != NULL) { + MoveToCgroup(pid, cgroup_path); + } +#endif ExecAsDaemon(log_path, log_append, pid_done_fds[0], exe, argv); abort(); // NOLINT Unreachable. } @@ -183,8 +229,9 @@ int main(int argc, char** argv) { bool log_append = false; const char* log_path = NULL; const char* pid_path = NULL; + const char* cgroup_path = NULL; int opt; - while ((opt = getopt(argc, argv, ":al:p:")) != -1) { + while ((opt = getopt(argc, argv, ":al:p:c:")) != -1) { switch (opt) { case 'a': log_append = true; @@ -198,6 +245,10 @@ int main(int argc, char** argv) { pid_path = optarg; break; + case 'c': + cgroup_path = optarg; + break; + case ':': errx(EXIT_FAILURE, "Option -%c requires an argument", optopt); @@ -219,6 +270,6 @@ int main(int argc, char** argv) { if (argc < 2) { errx(EXIT_FAILURE, "Must provide at least an executable name and arg0"); } - Daemonize(log_path, log_append, pid_path, argv[0], argv + 1); + Daemonize(log_path, log_append, pid_path, cgroup_path, argv[0], argv + 1); return EXIT_SUCCESS; } From 3bd39b242807186cff3d87a6e83e385972cbec1a Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 31 Jan 2024 19:16:00 -0800 Subject: [PATCH 6/6] Add flag to skip printing specific aspects in build results We have an internal system which tacks an aspect onto all the builds it runs, but no one really cares about the aspect's outputs, just that with the default --show_result their normal results are suppressed. This lets us avoid that. There are a few ways we could have implemented this, I decided to go with the simplest - non-repeated flag with comma-separated values. Practically, we could evolve this to allow multiple and do something like we do with --output_group with adding/subtracting from the existing/default, but we can cross that bridge when/if we get there. PiperOrigin-RevId: 603229525 Change-Id: I8b1a387eeb7e67cceaf4f8e821bd29c12b136982 --- .../build/lib/buildtool/BuildRequestOptions.java | 12 ++++++++++++ .../build/lib/buildtool/BuildResultPrinter.java | 9 +++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java index 7f2b93f4388724..6fb828e92c9ed4 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java @@ -197,6 +197,18 @@ public class BuildRequestOptions extends OptionsBase { + " under the threshold.") public int maxResultTargets; + @Option( + name = "hide_aspect_results", + converter = Converters.CommaSeparatedOptionListConverter.class, + defaultValue = "", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.TERMINAL_OUTPUT}, + help = + "Comma-separated list of aspect names to not display in results (see --show_result). " + + "Useful for keeping aspects added by wrappers which are typically not interesting " + + "to end users out of console output.") + public List hideAspectResults; + @Option( name = "symlink_prefix", defaultValue = "null", diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java index 950a2a3dc7ec93..8afaeef3ae6cb6 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java @@ -102,10 +102,15 @@ private boolean outputTargets( // Splits aspects based on whether they are validation aspects. final ImmutableSet aspectsToPrint; final ImmutableList validationAspects; + final ImmutableSet aspectsToIgnore = + ImmutableSet.copyOf(request.getBuildOptions().hideAspectResults); + final ImmutableSet filteredAspects = aspects.keySet().stream() + .filter(k -> !aspectsToIgnore.contains(k.getAspectClass().getName())) + .collect(ImmutableSet.toImmutableSet()); if (request.useValidationAspect()) { var aspectsToPrintBuilder = ImmutableSet.builder(); var validationAspectsBuilder = ImmutableList.builder(); - for (AspectKey key : aspects.keySet()) { + for (AspectKey key : filteredAspects) { if (Objects.equals( key.getAspectClass().getName(), AspectCollection.VALIDATION_ASPECT_NAME)) { validationAspectsBuilder.add(key); @@ -116,7 +121,7 @@ private boolean outputTargets( aspectsToPrint = aspectsToPrintBuilder.build(); validationAspects = validationAspectsBuilder.build(); } else { - aspectsToPrint = aspects.keySet(); + aspectsToPrint = filteredAspects; validationAspects = ImmutableList.of(); }