From f0c32de0823307ec293a882af6a9c7a48631d33b Mon Sep 17 00:00:00 2001
From: chiwang <chiwang@google.com>
Date: Tue, 16 Mar 2021 01:35:05 -0700
Subject: [PATCH] Remote: Don't delete downloaded files for remote build
 without bytes.

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. https://github.com/bazelbuild/bazel/issues/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. https://github.com/bazelbuild/bazel/issues/12665

PiperOrigin-RevId: 363131672
---
 .../build/lib/remote/RemoteModule.java        | 32 -------------------
 .../bazel/remote/remote_execution_test.sh     |  8 ++---
 2 files changed, 4 insertions(+), 36 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index d5f9a1485a21d1..c3489dd730d98d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -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);
@@ -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;
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index 2724c6110861ca..dce00f2b848d26 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -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() {
@@ -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() {