Skip to content

Commit

Permalink
Delete directories asynchronously when cleaning a sandbox stash
Browse files Browse the repository at this point in the history
RELNOTES:none
PiperOrigin-RevId: 595415873
Change-Id: Ia6a41c2193255c34d400e56d1ab255b63a695e96
  • Loading branch information
oquenchil authored and copybara-github committed Jan 3, 2024
1 parent 239b89a commit eac7c96
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public abstract class AbstractContainerizingSandboxedSpawn implements SandboxedS
final SandboxInputs inputs;
final SandboxOutputs outputs;
private final Set<Path> writableDirs;
private final TreeDeleter treeDeleter;
protected final TreeDeleter treeDeleter;
@Nullable private final Path sandboxDebugPath;
@Nullable private final Path statisticsPath;
private final String mnemonic;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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 =
Expand All @@ -56,6 +61,8 @@ class AsynchronousTreeDeleter implements TreeDeleter {
service =
new ThreadPoolExecutor(
1, 1, 0L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), threadFactory);

this.trashBase = trashBase;
}

/**
Expand All @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/sandbox/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ 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",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:auto_value",
"//third_party:flogger",
"//third_party:guava",
"//third_party:jsr305",
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -135,6 +137,17 @@ public static void cleanExisting(
Set<PathFragment> dirsToCreate,
Path workDir)
throws IOException, InterruptedException {
cleanExisting(root, inputs, inputsToCreate, dirsToCreate, workDir, /* treeDeleter= */ null);
}

public static void cleanExisting(
Path root,
SandboxInputs inputs,
Set<PathFragment> inputsToCreate,
Set<PathFragment> 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<PathFragment> prefixDirs = new HashSet<>();
Expand All @@ -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);
}

/**
Expand All @@ -161,7 +174,8 @@ private static void cleanRecursively(
Set<PathFragment> inputsToCreate,
Set<PathFragment> dirsToCreate,
Path workDir,
Set<PathFragment> prefixDirs)
Set<PathFragment> prefixDirs,
@Nullable TreeDeleter treeDeleter)
throws IOException, InterruptedException {
Path execroot = workDir.getParentDirectory();
for (Dirent dirent : root.readdir(Symlinks.NOFOLLOW)) {
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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);
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ public void filterInputsAndDirsToCreate(
inputs,
inputsToCreate,
dirsToCreate,
sandboxExecRoot);
sandboxExecRoot,
treeDeleter);
}
}

Expand Down

0 comments on commit eac7c96

Please sign in to comment.