Skip to content

Commit

Permalink
remote: Delete output files created by failed download.
Browse files Browse the repository at this point in the history 
If a remote download fails, delete any output files that might have
already been created. Else, this might intefere with a subsequent
locally executed actions that expects none of its output files to
exist. See #3452.

Change-Id: I467a97d05606c586aa257326213940a37dad9dd5
PiperOrigin-RevId: 163336093
  • Loading branch information
@buchgr
buchgr committed Jul 27, 2017
1 parent 0f3481b commit 815bd63
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 55 deletions.
92 changes: 53 additions & 39 deletions src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import com.google.common.util.concurrent.MoreExecutors;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputFileCache;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.remote.Digests.ActionKey;
import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode;
Expand All @@ -44,7 +46,6 @@
import com.google.devtools.remoteexecution.v1test.FindMissingBlobsRequest;
import com.google.devtools.remoteexecution.v1test.FindMissingBlobsResponse;
import com.google.devtools.remoteexecution.v1test.GetActionResultRequest;
import com.google.devtools.remoteexecution.v1test.OutputDirectory;
import com.google.devtools.remoteexecution.v1test.OutputFile;
import com.google.devtools.remoteexecution.v1test.UpdateActionResultRequest;
import com.google.protobuf.ByteString;
Expand Down Expand Up @@ -182,55 +183,68 @@ public void ensureInputsPresent(
}
}

/**
* Download the entire tree data rooted by the given digest and write it into the given location.
*/
@SuppressWarnings("unused")
private void downloadTree(Digest rootDigest, Path rootLocation) {
throw new UnsupportedOperationException();
}

/**
* Download all results of a remotely executed action locally. TODO(olaola): will need to amend to
* include the {@link com.google.devtools.build.lib.remote.TreeNodeRepository} for updating.
*/
@Override
public void download(ActionResult result, Path execRoot, FileOutErr outErr)
throws IOException, InterruptedException {
for (OutputFile file : result.getOutputFilesList()) {
Path path = execRoot.getRelative(file.getPath());
FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
Digest digest = file.getDigest();
if (digest.getSizeBytes() == 0) {
// Handle empty file locally.
FileSystemUtils.writeContent(path, new byte[0]);
} else {
if (!file.getContent().isEmpty()) {
try (OutputStream stream = path.getOutputStream()) {
file.getContent().writeTo(stream);
}
throws ExecException, IOException, InterruptedException {
try {
for (OutputFile file : result.getOutputFilesList()) {
Path path = execRoot.getRelative(file.getPath());
FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
Digest digest = file.getDigest();
if (digest.getSizeBytes() == 0) {
// Handle empty file locally.
FileSystemUtils.writeContent(path, new byte[0]);
} else {
retrier.execute(
() -> {
try (OutputStream stream = path.getOutputStream()) {
readBlob(digest, stream);
}
return null;
});
Digest receivedDigest = Digests.computeDigest(path);
if (!receivedDigest.equals(digest)) {
throw new IOException(
"Digest does not match " + receivedDigest + " != " + digest);
if (!file.getContent().isEmpty()) {
try (OutputStream stream = path.getOutputStream()) {
file.getContent().writeTo(stream);
}
} else {
retrier.execute(
() -> {
try (OutputStream stream = path.getOutputStream()) {
readBlob(digest, stream);
}
return null;
});
Digest receivedDigest = Digests.computeDigest(path);
if (!receivedDigest.equals(digest)) {
throw new IOException(
"Digest does not match " + receivedDigest + " != " + digest);
}
}
}
path.setExecutable(file.getIsExecutable());
}
path.setExecutable(file.getIsExecutable());
}
for (OutputDirectory directory : result.getOutputDirectoriesList()) {
downloadTree(directory.getDigest(), execRoot.getRelative(directory.getPath()));
if (!result.getOutputDirectoriesList().isEmpty()) {
throw new UnsupportedOperationException();
}
// TODO(ulfjack): use same code as above also for stdout / stderr if applicable.
downloadOutErr(result, outErr);
} catch (IOException downloadException) {
try {
// Delete any (partially) downloaded output files, since any subsequent local execution
// of this action may expect none of the output files to exist.
for (OutputFile file : result.getOutputFilesList()) {
execRoot.getRelative(file.getPath()).delete();
}
outErr.getOutputPath().delete();
outErr.getErrorPath().delete();
} catch (IOException e) {
// If deleting of output files failed, we abort the build with a decent error message as
// any subsequent local execution failure would likely be incomprehensible.

// We don't propagate the downloadException, as this is a recoverable error and the cause
// of the build failure is really that we couldn't delete output files.
throw new EnvironmentalExecException("Failed to delete output files after incomplete "
+ "download. Cannot continue with local execution.", e, true);
}
throw downloadException;
}
// TODO(ulfjack): use same code as above also for stdout / stderr if applicable.
downloadOutErr(result, outErr);
}

private void downloadOutErr(ActionResult result, FileOutErr outErr)
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/com/google/devtools/build/lib/remote/RemoteActionCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package com.google.devtools.build.lib.remote;

import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.remote.Digests.ActionKey;
import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode;
Expand Down Expand Up @@ -56,11 +57,14 @@ void ensureInputsPresent(
* Download the output files and directory trees of a remotely executed action to the local
* machine, as well stdin / stdout to the given files.
*
* <p>In case of failure, this method must delete any output files it might have already created.
*
* @throws CacheNotFoundException in case of a cache miss.
* @throws ExecException in case clean up after a failed download failed.
*/
// TODO(olaola): will need to amend to include the TreeNodeRepository for updating.
void download(ActionResult result, Path execRoot, FileOutErr outErr)
throws IOException, InterruptedException;
throws ExecException, IOException, InterruptedException;
/**
* Attempts to look up the given action in the remote cache and return its result, if present.
* Returns {@code null} if there is no such entry. Note that a successful result from this method
Expand Down
52 changes: 37 additions & 15 deletions src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputFileCache;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.remote.Digests.ActionKey;
Expand All @@ -31,7 +33,6 @@
import com.google.devtools.remoteexecution.v1test.Directory;
import com.google.devtools.remoteexecution.v1test.DirectoryNode;
import com.google.devtools.remoteexecution.v1test.FileNode;
import com.google.devtools.remoteexecution.v1test.OutputDirectory;
import com.google.devtools.remoteexecution.v1test.OutputFile;
import com.google.protobuf.ByteString;
import com.google.protobuf.InvalidProtocolBufferException;
Expand Down Expand Up @@ -110,22 +111,43 @@ private Digest uploadFileContents(

@Override
public void download(ActionResult result, Path execRoot, FileOutErr outErr)
throws IOException, InterruptedException {
for (OutputFile file : result.getOutputFilesList()) {
if (!file.getContent().isEmpty()) {
createFile(
file.getContent().toByteArray(),
execRoot.getRelative(file.getPath()),
file.getIsExecutable());
} else {
downloadFileContents(
file.getDigest(), execRoot.getRelative(file.getPath()), file.getIsExecutable());
throws ExecException, IOException, InterruptedException {
try {
for (OutputFile file : result.getOutputFilesList()) {
if (!file.getContent().isEmpty()) {
createFile(
file.getContent().toByteArray(),
execRoot.getRelative(file.getPath()),
file.getIsExecutable());
} else {
downloadFileContents(
file.getDigest(), execRoot.getRelative(file.getPath()), file.getIsExecutable());
}
}
if (!result.getOutputDirectoriesList().isEmpty()) {
throw new UnsupportedOperationException();
}
downloadOutErr(result, outErr);
} catch (IOException downloadException) {
try {
// Delete any (partially) downloaded output files, since any subsequent local execution
// of this action may expect none of the output files to exist.
for (OutputFile file : result.getOutputFilesList()) {
execRoot.getRelative(file.getPath()).delete();
}
outErr.getOutputPath().delete();
outErr.getErrorPath().delete();
} catch (IOException e) {
// If deleting of output files failed, we abort the build with a decent error message as
// any subsequent local execution failure would likely be incomprehensible.

// We don't propagate the downloadException, as this is a recoverable error and the cause
// of the build failure is really that we couldn't delete output files.
throw new EnvironmentalExecException("Failed to delete output files after incomplete "
+ "download. Cannot continue with local execution.", e, true);
}
throw downloadException;
}
for (OutputDirectory directory : result.getOutputDirectoriesList()) {
downloadTree(directory.getDigest(), execRoot.getRelative(directory.getPath()));
}
downloadOutErr(result, outErr);
}

private void downloadOutErr(ActionResult result, FileOutErr outErr)
Expand Down

0 comments on commit 815bd63

Please sign in to comment.