From eac7c968f40066aef5b5bf9b6177dd6c1af543ee Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 3 Jan 2024 08:59:27 -0800 Subject: [PATCH] Delete directories asynchronously when cleaning a sandbox stash RELNOTES:none PiperOrigin-RevId: 595415873 Change-Id: Ia6a41c2193255c34d400e56d1ab255b63a695e96 --- .../devtools/build/lib/exec/TreeDeleter.java | 2 ++ .../AbstractContainerizingSandboxedSpawn.java | 2 +- .../lib/sandbox/AsynchronousTreeDeleter.java | 15 ++++++-- .../google/devtools/build/lib/sandbox/BUILD | 2 ++ .../build/lib/sandbox/SandboxHelpers.java | 35 ++++++++++++++++--- .../build/lib/sandbox/SandboxModule.java | 4 ++- .../lib/sandbox/SymlinkedSandboxedSpawn.java | 3 +- 7 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/exec/TreeDeleter.java b/src/main/java/com/google/devtools/build/lib/exec/TreeDeleter.java index 27188c80b13a25..a8fa8496317d46 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/TreeDeleter.java +++ b/src/main/java/com/google/devtools/build/lib/exec/TreeDeleter.java @@ -27,6 +27,8 @@ public interface TreeDeleter { * not be reported even if they happen. For example, if deletions are asynchronous, there is no * way to capture their errors. * + *

For asynchronous deleters the directory is moved before being deleted. + * * @param path the tree to be deleted * @throws IOException if there are problems deleting the tree */ diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java index 0bfe6979a0daf2..9c2ecb761e72c4 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java @@ -45,7 +45,7 @@ public abstract class AbstractContainerizingSandboxedSpawn implements SandboxedS final SandboxInputs inputs; final SandboxOutputs outputs; private final Set writableDirs; - private final TreeDeleter treeDeleter; + protected final TreeDeleter treeDeleter; @Nullable private final Path sandboxDebugPath; @Nullable private final Path statisticsPath; private final String mnemonic; diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AsynchronousTreeDeleter.java b/src/main/java/com/google/devtools/build/lib/sandbox/AsynchronousTreeDeleter.java index 178f7f11588cb2..3ff1ba81c02814 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AsynchronousTreeDeleter.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AsynchronousTreeDeleter.java @@ -26,6 +26,7 @@ import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nullable; /** @@ -39,11 +40,15 @@ class AsynchronousTreeDeleter implements TreeDeleter { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + private final AtomicInteger trashCount = new AtomicInteger(0); + /** Thread pool used to execute asynchronous tree deletions; null in synchronous mode. */ @Nullable private ThreadPoolExecutor service; + private final Path trashBase; + /** Constructs a new asynchronous tree deleter backed by just one thread. */ - AsynchronousTreeDeleter() { + AsynchronousTreeDeleter(Path trashBase) { logger.atInfo().log("Starting async tree deletion pool with 1 thread"); ThreadFactory threadFactory = @@ -56,6 +61,8 @@ class AsynchronousTreeDeleter implements TreeDeleter { service = new ThreadPoolExecutor( 1, 1, 0L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), threadFactory); + + this.trashBase = trashBase; } /** @@ -74,12 +81,14 @@ void setThreads(int threads) { } @Override - public void deleteTree(Path path) { + public void deleteTree(Path path) throws IOException { + Path trashPath = trashBase.getRelative(Integer.toString(trashCount.getAndIncrement())); + path.renameTo(trashPath); checkNotNull(service, "Cannot call deleteTree after shutdown") .execute( () -> { try { - path.deleteTree(); + trashPath.deleteTree(); } catch (IOException e) { logger.atWarning().withCause(e).log( "Failed to delete tree %s asynchronously", path); 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 6d49080690bf08..18ae18e9d37168 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD +++ b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD @@ -20,6 +20,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration", "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/exec:tree_deleter", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/common/options", @@ -27,6 +28,7 @@ java_library( "//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/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java index dfc18bda2c839a..f094294b82b815 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput; import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.cmdline.LabelConstants; +import com.google.devtools.build.lib.exec.TreeDeleter; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.Sandbox; import com.google.devtools.build.lib.server.FailureDetails.Sandbox.Code; @@ -64,6 +65,7 @@ import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import javax.annotation.Nullable; /** * Helper methods that are shared by the different sandboxing strategies. @@ -135,6 +137,17 @@ public static void cleanExisting( Set dirsToCreate, Path workDir) throws IOException, InterruptedException { + cleanExisting(root, inputs, inputsToCreate, dirsToCreate, workDir, /* treeDeleter= */ null); + } + + public static void cleanExisting( + Path root, + SandboxInputs inputs, + Set inputsToCreate, + Set dirsToCreate, + Path workDir, + @Nullable TreeDeleter treeDeleter) + throws IOException, InterruptedException { // To avoid excessive scanning of dirsToCreate for prefix dirs, we prepopulate this set of // prefixes. Set prefixDirs = new HashSet<>(); @@ -148,7 +161,7 @@ public static void cleanExisting( parent = parent.getParentDirectory(); } } - cleanRecursively(root, inputs, inputsToCreate, dirsToCreate, workDir, prefixDirs); + cleanRecursively(root, inputs, inputsToCreate, dirsToCreate, workDir, prefixDirs, treeDeleter); } /** @@ -161,7 +174,8 @@ private static void cleanRecursively( Set inputsToCreate, Set dirsToCreate, Path workDir, - Set prefixDirs) + Set prefixDirs, + @Nullable TreeDeleter treeDeleter) throws IOException, InterruptedException { Path execroot = workDir.getParentDirectory(); for (Dirent dirent : root.readdir(Symlinks.NOFOLLOW)) { @@ -188,17 +202,28 @@ private static void cleanRecursively( && absPath.readSymbolicLink().equals(destination.get())) { inputsToCreate.remove(pathRelativeToWorkDir); } else if (absPath.isDirectory()) { - absPath.deleteTree(); + if (treeDeleter == null) { + // TODO(bazel-team): Use async tree deleter for workers too + absPath.deleteTree(); + } else { + treeDeleter.deleteTree(absPath); + } } else { absPath.delete(); } } else if (DIRECTORY.equals(dirent.getType())) { if (dirsToCreate.contains(pathRelativeToWorkDir) || prefixDirs.contains(pathRelativeToWorkDir)) { - cleanRecursively(absPath, inputs, inputsToCreate, dirsToCreate, workDir, prefixDirs); + cleanRecursively( + absPath, inputs, inputsToCreate, dirsToCreate, workDir, prefixDirs, treeDeleter); dirsToCreate.remove(pathRelativeToWorkDir); } else { - absPath.deleteTree(); + if (treeDeleter == null) { + // TODO(bazel-team): Use async tree deleter for workers too + absPath.deleteTree(); + } else { + treeDeleter.deleteTree(absPath); + } } } else if (!inputsToCreate.contains(pathRelativeToWorkDir)) { absPath.delete(); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java index 0f3b51732bbc6c..6a3075e962e794 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java @@ -193,6 +193,7 @@ private void setup(CommandEnvironment cmdEnv, SpawnStrategyRegistry.Builder buil throws IOException, InterruptedException { SandboxOptions options = checkNotNull(env.getOptions().getOptions(SandboxOptions.class)); sandboxBase = computeSandboxBase(options, env); + Path trashBase = sandboxBase.getRelative("_moved_trash_dir"); SandboxHelpers helpers = new SandboxHelpers(); @@ -210,7 +211,7 @@ private void setup(CommandEnvironment cmdEnv, SpawnStrategyRegistry.Builder buil } } else { if (!(treeDeleter instanceof AsynchronousTreeDeleter)) { - treeDeleter = new AsynchronousTreeDeleter(); + treeDeleter = new AsynchronousTreeDeleter(trashBase); } } SandboxStash.initialize(env.getWorkspaceName(), env.getOutputBase(), options); @@ -225,6 +226,7 @@ private void setup(CommandEnvironment cmdEnv, SpawnStrategyRegistry.Builder buil } firstBuild = false; sandboxBase.createDirectoryAndParents(); + trashBase.createDirectory(); PathFragment windowsSandboxPath = PathFragment.create(options.windowsSandboxPath); boolean windowsSandboxSupported; diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java index 57a09356f92cb2..6141e5fea11c4c 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java @@ -82,7 +82,8 @@ public void filterInputsAndDirsToCreate( inputs, inputsToCreate, dirsToCreate, - sandboxExecRoot); + sandboxExecRoot, + treeDeleter); } }