Skip to content

Commit

Permalink
Fix local execution deduplication to work with optional outputs
Browse files Browse the repository at this point in the history
This fixes failures such as the following when a spawn, e.g. a reduced Java compilation spawn, doesn't create an optional output:

```
java.io.FileNotFoundException: /private/var/tmp/_bazel_fmeum/507738cfc7e6cde00e4a0230e9aa0722/execroot/_main/bazel-out/_tmp/actions/remote/175.tmp (No such file or directory)
	at com.google.devtools.build.lib.unix.NativePosixFiles.lstat(Native Method)
	at com.google.devtools.build.lib.unix.UnixFileSystem.statInternal(UnixFileSystem.java:212)
	at com.google.devtools.build.lib.unix.UnixFileSystem.stat(UnixFileSystem.java:201)
	at com.google.devtools.build.lib.vfs.Path.stat(Path.java:290)
	at com.google.devtools.build.lib.vfs.FileSystemUtils.moveFile(FileSystemUtils.java:456)
	at com.google.devtools.build.lib.remote.RemoteExecutionService.moveOutputsToFinalLocation(RemoteExecutionService.java:878)
        ...
```

Also clean up temporary files in case of an exception.

Work towards #23288

Closes #23296.

PiperOrigin-RevId: 665744936
Change-Id: I89a409c7a6b28b2a5fa532bdb233dca9bc5bde73
  • Loading branch information
fmeum authored and copybara-github committed Aug 21, 2024
1 parent 6fabb1f commit 5482fea
Showing 1 changed file with 57 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1422,56 +1422,68 @@ public SpawnResult waitForAndReuseOutputs(RemoteAction action, LocalExecution pr
previousExecution.action.getSpawn().getOutputFiles().stream()
.collect(toImmutableMap(output -> execRoot.getRelative(output.getExecPath()), o -> o));
Map<Path, Path> realToTmpPath = new HashMap<>();
for (String output : action.getCommand().getOutputPathsList()) {
Path sourcePath =
previousExecution
.action
.getRemotePathResolver()
.outputPathToLocalPath(encodeBytestringUtf8(output));
ActionInput outputArtifact = previousOutputs.get(sourcePath);
Path tmpPath = tempPathGenerator.generateTempPath();
tmpPath.getParentDirectory().createDirectoryAndParents();
try {
if (outputArtifact.isDirectory()) {
tmpPath.createDirectory();
FileSystemUtils.copyTreesBelow(sourcePath, tmpPath, Symlinks.NOFOLLOW);
} else if (outputArtifact.isSymlink()) {
FileSystemUtils.ensureSymbolicLink(tmpPath, sourcePath.readSymbolicLink());
} else {
FileSystemUtils.copyFile(sourcePath, tmpPath);
}
} catch (FileNotFoundException e) {
// The spawn this action was deduplicated against failed to create an output file. If the
// output is mandatory, we cannot reuse the previous execution.
if (action.getSpawn().isMandatoryOutput(outputArtifact)) {
return null;
try {
for (String output : action.getCommand().getOutputPathsList()) {
Path sourcePath =
previousExecution
.action
.getRemotePathResolver()
.outputPathToLocalPath(encodeBytestringUtf8(output));
ActionInput outputArtifact = previousOutputs.get(sourcePath);
Path tmpPath = tempPathGenerator.generateTempPath();
tmpPath.getParentDirectory().createDirectoryAndParents();
try {
if (outputArtifact.isDirectory()) {
tmpPath.createDirectory();
FileSystemUtils.copyTreesBelow(sourcePath, tmpPath, Symlinks.NOFOLLOW);
} else if (outputArtifact.isSymlink()) {
FileSystemUtils.ensureSymbolicLink(tmpPath, sourcePath.readSymbolicLink());
} else {
FileSystemUtils.copyFile(sourcePath, tmpPath);
}

Path targetPath =
action.getRemotePathResolver().outputPathToLocalPath(encodeBytestringUtf8(output));
realToTmpPath.put(targetPath, tmpPath);
} catch (FileNotFoundException e) {
// The spawn this action was deduplicated against failed to create an output file. If the
// output is mandatory, we cannot reuse the previous execution.
if (action.getSpawn().isMandatoryOutput(outputArtifact)) {
return null;
}
}
}

Path targetPath =
action.getRemotePathResolver().outputPathToLocalPath(encodeBytestringUtf8(output));
realToTmpPath.put(targetPath, tmpPath);
// TODO: FileOutErr is action-scoped, not spawn-scoped, but this is not a problem for the
// current use case of supporting deduplication of path mapped spawns:
// 1. Starlark and C++ compilation actions always create a single spawn.
// 2. Java compilation actions may run a fallback spawn, but reset the FileOutErr before
// running it.
// If this changes, we will need to introduce a spawn-scoped OutErr.
FileOutErr.dump(
previousExecution.action.getSpawnExecutionContext().getFileOutErr(),
action.getSpawnExecutionContext().getFileOutErr());

action
.getSpawnExecutionContext()
.lockOutputFiles(
previousSpawnResult.exitCode(),
previousSpawnResult.getFailureMessage(),
action.getSpawnExecutionContext().getFileOutErr());
// All outputs are created locally.
moveOutputsToFinalLocation(realToTmpPath.keySet(), realToTmpPath);
} catch (InterruptedException | IOException e) {
// Delete any copied output files.
try {
for (Path tmpPath : realToTmpPath.values()) {
tmpPath.delete();
}
} catch (IOException ignored) {
// Best effort, will be cleaned up at server restart.
}
throw e;
}

// TODO: FileOutErr is action-scoped, not spawn-scoped, but this is not a problem for the
// current use case of supporting deduplication of path mapped spawns:
// 1. Starlark and C++ compilation actions always create a single spawn.
// 2. Java compilation actions may run a fallback spawn, but reset the FileOutErr before
// running it.
// If this changes, we will need to introduce a spawn-scoped OutErr.
FileOutErr.dump(
previousExecution.action.getSpawnExecutionContext().getFileOutErr(),
action.getSpawnExecutionContext().getFileOutErr());

action
.getSpawnExecutionContext()
.lockOutputFiles(
previousSpawnResult.exitCode(),
previousSpawnResult.getFailureMessage(),
action.getSpawnExecutionContext().getFileOutErr());
// All outputs are created locally.
moveOutputsToFinalLocation(realToTmpPath.keySet(), realToTmpPath);

return previousSpawnResult;
}

Expand Down

0 comments on commit 5482fea

Please sign in to comment.