Skip to content

Commit

Permalink
Remote: Don't delete downloaded files for remote build without bytes.
Browse files Browse the repository at this point in the history
Before this change, when remote build without bytes is enabled, intermediate outputs which are also inputs to local actions will be downloaded for local execution. After build finished, these downloaded files are deleted to keep Bazel's view of the output base identical with the output base i.e. files that Bazel thinks exist only remotely actually do. However, these intermediate outputs maybe used later in which case Bazel need to download them again which is a performance issue. bazelbuild#12855

This change fix the issue by removing the code used to delete downloaded files. Bazel should be able to take the local file as the source of truth if it exists (otherwise, it is a bug).

This is also an essential step to implement separation of downloads. bazelbuild#12665

PiperOrigin-RevId: 363131672
  • Loading branch information
coeuvre authored and copybara-github committed Mar 16, 2021
1 parent ce0b2b2 commit f0c32de
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -747,14 +747,6 @@ public void afterCommand() throws AbruptExitException {
logger.atWarning().withCause(e).log(failureMessage);
}

try {
deleteDownloadedInputs();
} catch (IOException e) {
failure = e;
failureCode = Code.DOWNLOADED_INPUTS_DELETION_FAILURE;
failureMessage = "Failed to delete downloaded inputs";
}

buildEventArtifactUploaderFactoryDelegate.reset();
repositoryRemoteExecutorFactoryDelegate.reset();
remoteDownloaderSupplier.set(null);
Expand All @@ -768,30 +760,6 @@ public void afterCommand() throws AbruptExitException {
}
}

/**
* Delete any input files that have been fetched from the remote cache during the build. This is
* so that Bazel's view of the output base is identical with the output base after a build i.e.
* files that Bazel thinks exist only remotely actually do.
*/
private void deleteDownloadedInputs() throws IOException {
if (actionInputFetcher == null) {
return;
}
IOException deletionFailure = null;
for (Path file : actionInputFetcher.downloadedFiles()) {
try {
file.delete();
} catch (IOException e) {
logger.atSevere().withCause(e).log(
"Failed to delete remote output '%s' from the output base.", file);
deletionFailure = e;
}
}
if (deletionFailure != null) {
throw deletionFailure;
}
}

private void closeRpcLogFile() throws IOException {
if (rpcLogFile != null) {
AsynchronousFileOutputStream oldLogFile = rpcLogFile;
Expand Down
8 changes: 4 additions & 4 deletions src/test/shell/bazel/remote/remote_execution_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1011,8 +1011,8 @@ EOF
[[ $(< ${localtxt}) == "remotelocal" ]] \
|| fail "Unexpected contents in " ${localtxt} ": " $(< ${localtxt})

(! [[ -f bazel-bin/a/remote.txt ]]) \
|| fail "Expected bazel-bin/a/remote.txt to have been deleted again"
[[ -f bazel-bin/a/remote.txt ]] \
|| fail "Expected bazel-bin/a/remote.txt to be downloaded"
}

function test_download_outputs_invalidation() {
Expand Down Expand Up @@ -1105,8 +1105,8 @@ EOF
[[ $(< ${outtxt}) == "Hello buchgr!" ]] \
|| fail "Unexpected contents in "${outtxt}":" $(< ${outtxt})

(! [[ -f bazel-bin/a/template.txt ]]) \
|| fail "Expected bazel-bin/a/template.txt to have been deleted again"
[[ -f bazel-bin/a/template.txt ]] \
|| fail "Expected bazel-bin/a/template.txt to be downloaded"
}

function test_downloads_toplevel() {
Expand Down

0 comments on commit f0c32de

Please sign in to comment.