Skip to content

Commit eac7c96

Browse files
oquenchilcopybara-github
oquenchil
authored and
copybara-github
committed
Delete directories asynchronously when cleaning a sandbox stash
RELNOTES:none PiperOrigin-RevId: 595415873 Change-Id: Ia6a41c2193255c34d400e56d1ab255b63a695e96
1 parent 239b89a commit eac7c96

File tree

7 files changed

+52
-11
lines changed

7 files changed

+52
-11
lines changed

‎src/main/java/com/google/devtools/build/lib/exec/TreeDeleter.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ public interface TreeDeleter {
2727
* not be reported even if they happen. For example, if deletions are asynchronous, there is no
2828
* way to capture their errors.
2929
*
30+
* <p>For asynchronous deleters the directory is moved before being deleted.
31+
*
3032
* @param path the tree to be deleted
3133
* @throws IOException if there are problems deleting the tree
3234
*/

‎src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public abstract class AbstractContainerizingSandboxedSpawn implements SandboxedS
4545
final SandboxInputs inputs;
4646
final SandboxOutputs outputs;
4747
private final Set<Path> writableDirs;
48-
private final TreeDeleter treeDeleter;
48+
protected final TreeDeleter treeDeleter;
4949
@Nullable private final Path sandboxDebugPath;
5050
@Nullable private final Path statisticsPath;
5151
private final String mnemonic;

‎src/main/java/com/google/devtools/build/lib/sandbox/AsynchronousTreeDeleter.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.concurrent.ThreadFactory;
2727
import java.util.concurrent.ThreadPoolExecutor;
2828
import java.util.concurrent.TimeUnit;
29+
import java.util.concurrent.atomic.AtomicInteger;
2930
import javax.annotation.Nullable;
3031

3132
/**
@@ -39,11 +40,15 @@
3940
class AsynchronousTreeDeleter implements TreeDeleter {
4041
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
4142

43+
private final AtomicInteger trashCount = new AtomicInteger(0);
44+
4245
/** Thread pool used to execute asynchronous tree deletions; null in synchronous mode. */
4346
@Nullable private ThreadPoolExecutor service;
4447

48+
private final Path trashBase;
49+
4550
/** Constructs a new asynchronous tree deleter backed by just one thread. */
46-
AsynchronousTreeDeleter() {
51+
AsynchronousTreeDeleter(Path trashBase) {
4752
logger.atInfo().log("Starting async tree deletion pool with 1 thread");
4853

4954
ThreadFactory threadFactory =
@@ -56,6 +61,8 @@ class AsynchronousTreeDeleter implements TreeDeleter {
5661
service =
5762
new ThreadPoolExecutor(
5863
1, 1, 0L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), threadFactory);
64+
65+
this.trashBase = trashBase;
5966
}
6067

6168
/**
@@ -74,12 +81,14 @@ void setThreads(int threads) {
7481
}
7582

7683
@Override
77-
public void deleteTree(Path path) {
84+
public void deleteTree(Path path) throws IOException {
85+
Path trashPath = trashBase.getRelative(Integer.toString(trashCount.getAndIncrement()));
86+
path.renameTo(trashPath);
7887
checkNotNull(service, "Cannot call deleteTree after shutdown")
7988
.execute(
8089
() -> {
8190
try {
82-
path.deleteTree();
91+
trashPath.deleteTree();
8392
} catch (IOException e) {
8493
logger.atWarning().withCause(e).log(
8594
"Failed to delete tree %s asynchronously", path);

‎src/main/java/com/google/devtools/build/lib/sandbox/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@ java_library(
2020
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
2121
"//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
2222
"//src/main/java/com/google/devtools/build/lib/cmdline",
23+
"//src/main/java/com/google/devtools/build/lib/exec:tree_deleter",
2324
"//src/main/java/com/google/devtools/build/lib/vfs",
2425
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
2526
"//src/main/java/com/google/devtools/common/options",
2627
"//src/main/protobuf:failure_details_java_proto",
2728
"//third_party:auto_value",
2829
"//third_party:flogger",
2930
"//third_party:guava",
31+
"//third_party:jsr305",
3032
],
3133
)
3234

‎src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput;
3939
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
4040
import com.google.devtools.build.lib.cmdline.LabelConstants;
41+
import com.google.devtools.build.lib.exec.TreeDeleter;
4142
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
4243
import com.google.devtools.build.lib.server.FailureDetails.Sandbox;
4344
import com.google.devtools.build.lib.server.FailureDetails.Sandbox.Code;
@@ -64,6 +65,7 @@
6465
import java.util.TreeMap;
6566
import java.util.concurrent.atomic.AtomicBoolean;
6667
import java.util.concurrent.atomic.AtomicInteger;
68+
import javax.annotation.Nullable;
6769

6870
/**
6971
* Helper methods that are shared by the different sandboxing strategies.
@@ -135,6 +137,17 @@ public static void cleanExisting(
135137
Set<PathFragment> dirsToCreate,
136138
Path workDir)
137139
throws IOException, InterruptedException {
140+
cleanExisting(root, inputs, inputsToCreate, dirsToCreate, workDir, /* treeDeleter= */ null);
141+
}
142+
143+
public static void cleanExisting(
144+
Path root,
145+
SandboxInputs inputs,
146+
Set<PathFragment> inputsToCreate,
147+
Set<PathFragment> dirsToCreate,
148+
Path workDir,
149+
@Nullable TreeDeleter treeDeleter)
150+
throws IOException, InterruptedException {
138151
// To avoid excessive scanning of dirsToCreate for prefix dirs, we prepopulate this set of
139152
// prefixes.
140153
Set<PathFragment> prefixDirs = new HashSet<>();
@@ -148,7 +161,7 @@ public static void cleanExisting(
148161
parent = parent.getParentDirectory();
149162
}
150163
}
151-
cleanRecursively(root, inputs, inputsToCreate, dirsToCreate, workDir, prefixDirs);
164+
cleanRecursively(root, inputs, inputsToCreate, dirsToCreate, workDir, prefixDirs, treeDeleter);
152165
}
153166

154167
/**
@@ -161,7 +174,8 @@ private static void cleanRecursively(
161174
Set<PathFragment> inputsToCreate,
162175
Set<PathFragment> dirsToCreate,
163176
Path workDir,
164-
Set<PathFragment> prefixDirs)
177+
Set<PathFragment> prefixDirs,
178+
@Nullable TreeDeleter treeDeleter)
165179
throws IOException, InterruptedException {
166180
Path execroot = workDir.getParentDirectory();
167181
for (Dirent dirent : root.readdir(Symlinks.NOFOLLOW)) {
@@ -188,17 +202,28 @@ private static void cleanRecursively(
188202
&& absPath.readSymbolicLink().equals(destination.get())) {
189203
inputsToCreate.remove(pathRelativeToWorkDir);
190204
} else if (absPath.isDirectory()) {
191-
absPath.deleteTree();
205+
if (treeDeleter == null) {
206+
// TODO(bazel-team): Use async tree deleter for workers too
207+
absPath.deleteTree();
208+
} else {
209+
treeDeleter.deleteTree(absPath);
210+
}
192211
} else {
193212
absPath.delete();
194213
}
195214
} else if (DIRECTORY.equals(dirent.getType())) {
196215
if (dirsToCreate.contains(pathRelativeToWorkDir)
197216
|| prefixDirs.contains(pathRelativeToWorkDir)) {
198-
cleanRecursively(absPath, inputs, inputsToCreate, dirsToCreate, workDir, prefixDirs);
217+
cleanRecursively(
218+
absPath, inputs, inputsToCreate, dirsToCreate, workDir, prefixDirs, treeDeleter);
199219
dirsToCreate.remove(pathRelativeToWorkDir);
200220
} else {
201-
absPath.deleteTree();
221+
if (treeDeleter == null) {
222+
// TODO(bazel-team): Use async tree deleter for workers too
223+
absPath.deleteTree();
224+
} else {
225+
treeDeleter.deleteTree(absPath);
226+
}
202227
}
203228
} else if (!inputsToCreate.contains(pathRelativeToWorkDir)) {
204229
absPath.delete();

‎src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ private void setup(CommandEnvironment cmdEnv, SpawnStrategyRegistry.Builder buil
193193
throws IOException, InterruptedException {
194194
SandboxOptions options = checkNotNull(env.getOptions().getOptions(SandboxOptions.class));
195195
sandboxBase = computeSandboxBase(options, env);
196+
Path trashBase = sandboxBase.getRelative("_moved_trash_dir");
196197

197198
SandboxHelpers helpers = new SandboxHelpers();
198199

@@ -210,7 +211,7 @@ private void setup(CommandEnvironment cmdEnv, SpawnStrategyRegistry.Builder buil
210211
}
211212
} else {
212213
if (!(treeDeleter instanceof AsynchronousTreeDeleter)) {
213-
treeDeleter = new AsynchronousTreeDeleter();
214+
treeDeleter = new AsynchronousTreeDeleter(trashBase);
214215
}
215216
}
216217
SandboxStash.initialize(env.getWorkspaceName(), env.getOutputBase(), options);
@@ -225,6 +226,7 @@ private void setup(CommandEnvironment cmdEnv, SpawnStrategyRegistry.Builder buil
225226
}
226227
firstBuild = false;
227228
sandboxBase.createDirectoryAndParents();
229+
trashBase.createDirectory();
228230

229231
PathFragment windowsSandboxPath = PathFragment.create(options.windowsSandboxPath);
230232
boolean windowsSandboxSupported;

‎src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedSandboxedSpawn.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ public void filterInputsAndDirsToCreate(
8282
inputs,
8383
inputsToCreate,
8484
dirsToCreate,
85-
sandboxExecRoot);
85+
sandboxExecRoot,
86+
treeDeleter);
8687
}
8788
}
8889

0 commit comments

Comments
 (0)