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/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/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/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/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(); } 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/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/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/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..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 @@ -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,13 @@ 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()) { + cgroup.get().memory().monitor().start(sandboxOptions.memoryMonitored); + commandLineBuilder.setCgroupsDirs( + cgroup.get().paths().stream() + .map(p -> fileSystem.getPath(p.toString())) + .collect(ImmutableSet.toImmutableSet())); } if (!timeout.isZero()) { @@ -447,6 +539,14 @@ public void verifyPostCondition( if (getSandboxOptions().useHermetic) { checkForConcurrentModifications(context); } + 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 + cgroup.get().delete(); + } } private void checkForConcurrentModifications(SpawnExecutionContext context) @@ -511,9 +611,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..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 @@ -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; @@ -25,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; @@ -370,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", @@ -381,6 +393,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..0ba747beee38e4 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/BUILD @@ -0,0 +1,32 @@ +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/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 new file mode 100644 index 00000000000000..f40387001e5648 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/Controller.java @@ -0,0 +1,62 @@ +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.Files; +import java.nio.file.Path; +import java.util.Map; +import java.util.stream.Collectors; + +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; + + Path statFile() throws IOException; + + default String getStats() throws IOException { + if (statFile() != null && statFile().toFile().exists()) { + return Files.readString(statFile()); + } + return ""; + } + + interface Memory extends Controller, Monitor.Monitorable { + 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; + } +} 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/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/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..8831cf1031fb3d --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/VirtualCGroup.java @@ -0,0 +1,314 @@ +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; +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.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; + + +/** + * 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"); + + private static final Monitor.MonitorFactory factory = new Monitor.MonitorFactory(); + + @Nullable + private static volatile VirtualCGroup instance; + + 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<>(); + + 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; + Controller.CpuAcct cpuacct = 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, factory); + 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, factory); + break; + case "cpu": + if (cpu != null) continue; + 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; + } + } + } + } + + 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, cpuacct, 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); + Controller.CpuAcct cpuacct = null; + 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, factory) : + new UnifiedMemory(cgroup, factory); + 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); + } + 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)); + } + 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) { + 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 new file mode 100644 index 00000000000000..f5509a23bcd677 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyCpu.java @@ -0,0 +1,44 @@ +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; +import java.util.Map; +import java.util.stream.Collectors; + +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 Path statFile() throws IOException { + return path.resolve("cpu.stat"); + } + + @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()); + } + + 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 new file mode 100644 index 00000000000000..d34b1997a923ef --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v1/LegacyMemory.java @@ -0,0 +1,58 @@ +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; + +public class LegacyMemory implements Controller.Memory { + private final Path path; + private final Monitor monitor; + + @Override + public Path getPath() { + return path; + } + + @Override + public Path statFile() throws IOException { + return path.resolve("memory.stat"); + } + + public LegacyMemory(Path path, Monitor.MonitorFactory factory) { + this.path = path; + this.monitor = factory.create(this); + } + + @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()); + } + + @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()); + } + + @Override + public Monitor monitor() { + return monitor; + } +} 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..1b3675d580fa52 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedCpu.java @@ -0,0 +1,42 @@ +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; + 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 + 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(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()); + } + + 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 new file mode 100644 index 00000000000000..1d6eb10625ffbb --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/sandbox/cgroups/v2/UnifiedMemory.java @@ -0,0 +1,64 @@ +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; + +public class UnifiedMemory implements Controller.Memory { + private final Path path; + private volatile Monitor monitor; + + public UnifiedMemory(Path path, Monitor.MonitorFactory factory) { + this.path = path; + this.monitor = factory.create(this); + } + + @Override + 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)); + } + + @Override + 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; + } + + @Override + public Monitor monitor() { + return this.monitor; + } +} 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/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; } 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/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/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(); 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"