Feedback

bazelbuild · Apr 9, 2024 · a424845 · a424845
1 parent 20c3b46
commit a424845
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 10 deletions.
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
@@ -163,8 +163,9 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS
       Duration timeoutKillDelay,
       TreeDeleter treeDeleter) {
     super(cmdEnv);
+    SandboxOptions sandboxOptions = cmdEnv.getOptions().getOptions(SandboxOptions.class);
     this.cgroupFactory =
-        cmdEnv.getOptions().getOptions(SandboxOptions.class).useOldCgroupImplementation ?
+        sandboxOptions == null || !sandboxOptions.useNewCgroupImplementation ?
           null :
           new VirtualCGroupFactory(
               "sandbox_",

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
@@ -382,7 +382,8 @@ public ImmutableSet<Path> getInaccessiblePaths(FileSystem fs) {
       allowMultiple = true,
       help =
           "If > 0, each Linux sandbox will be limited to the given amount"
-              + " for the specified resource. Overrides --experimental_sandbox_memory_limit_mb."
+              + " for the specified resource. Requires --incompatible_use_new_cgroup_implementation"
+              + " and overrides --experimental_sandbox_memory_limit_mb."
               + " Requires cgroups v1 or v2 and permissions for the users to the cgroups dir.")
   public List<Map.Entry<String, Double>> limits;
 
@@ -394,15 +395,15 @@ public ImmutableMap<String, Double> getLimits() {
   }
 
   @Option(
-    name = "incompatible_use_old_cgroup_implementation",
-    defaultValue = "true",
+    name = "incompatible_use_new_cgroup_implementation",
+    defaultValue = "false",
     documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
     effectTags = {OptionEffectTag.EXECUTION},
     converter = BooleanConverter.class,
     help =
-        "If true, use the old implementation for cgroups. The old implementation only supports"
+        "If true, use the new implementation for cgroups. The old implementation only supports"
             + " the memory controller and ignores the value of --experimental_sandbox_limits.")
-  public boolean useOldCgroupImplementation;
+  public boolean useNewCgroupImplementation;
 
   @Option(
       name = "experimental_sandbox_enforce_resources_regexp",

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
@@ -114,7 +114,7 @@ public static WorkerSandboxOptions create(
       WorkerOptions workerOptions,
       @Nullable WorkerSandboxOptions hardenedSandboxOptions,
       TreeDeleter treeDeleter,
-      VirtualCGroupFactory cgroupFactory) {
+      @Nullable VirtualCGroupFactory cgroupFactory) {
     super(workerKey, workerId, workDir, logFile, workerOptions, cgroupFactory);
     this.workerExecRoot =
         new WorkerExecRoot(

diff --git a/src/main/java/com/google/devtools/build/lib/worker/SingleplexWorker.java b/src/main/java/com/google/devtools/build/lib/worker/SingleplexWorker.java
@@ -77,7 +77,7 @@ class SingleplexWorker extends Worker {
   private final VirtualCGroupFactory cgroupFactory;
 
   SingleplexWorker(
-      WorkerKey workerKey, int workerId, final Path workDir, Path logFile, WorkerOptions options, VirtualCGroupFactory cgroupFactory) {
+      WorkerKey workerKey, int workerId, final Path workDir, Path logFile, WorkerOptions options, @Nullable VirtualCGroupFactory cgroupFactory) {
     super(workerKey, workerId, logFile, new WorkerProcessStatus());
     this.workDir = workDir;
     this.options = options;

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
@@ -124,7 +124,7 @@ public void buildStarting(BuildStartingEvent event) {
       }
     }
     VirtualCGroupFactory cgroupFactory =
-      sandboxOptions == null || sandboxOptions.useOldCgroupImplementation ?
+      sandboxOptions == null || !sandboxOptions.useNewCgroupImplementation ?
         null :
         new VirtualCGroupFactory(
           "worker_",
@@ -198,7 +198,7 @@ public void buildStarting(BuildStartingEvent event) {
 
     // Override the flag value if we can't actually use cgroups so that we at least fallback to ps.
     boolean useCgroupsOnLinux = options.useCgroupsOnLinux &&
-        (sandboxOptions == null || sandboxOptions.useOldCgroupImplementation ?
+        (sandboxOptions == null || !sandboxOptions.useNewCgroupImplementation ?
             CgroupsInfo.isSupported() :
             VirtualCGroup.getInstance().memory() != null);
     WorkerProcessMetricsCollector.instance().setUseCgroupsOnLinux(useCgroupsOnLinux);