Dont query remote cache but always use bytestream protocol

src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java

@@ -105,10 +105,12 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted
   }
 
   public void omitFile(Path file) {
+    Preconditions.checkState(remoteBuildEventUploadMode != RemoteBuildEventUploadMode.MINIMAL, "Cannot omit file in MINIMAL mode");
     omittedFiles.add(file.asFragment());
   }
 
   public void omitTree(Path treeRoot) {
+    Preconditions.checkState(remoteBuildEventUploadMode != RemoteBuildEventUploadMode.MINIMAL, "Cannot omit tree in MINIMAL mode");
     omittedTreeRoots.add(treeRoot.asFragment());
   }
 
@@ -207,10 +209,6 @@ private static void processQueryResult(
     }
   }
 
-  private boolean shouldQuery(PathMetadata path) {
-    return path.getDigest() != null && !path.isRemote() && !path.isDirectory();
-  }
-
   private boolean shouldUpload(PathMetadata path) {
     boolean result =
         path.getDigest() != null && !path.isRemote() && !path.isDirectory() && !path.isOmitted();
@@ -236,7 +234,8 @@ private Single<List<PathMetadata>> queryRemoteCache(
     List<PathMetadata> filesToQuery = new ArrayList<>();
     Set<Digest> digestsToQuery = new HashSet<>();
     for (PathMetadata path : paths) {
-      if (shouldQuery(path)) {
+      // Query remote cache for files even if omitted from uploading
+      if (shouldUpload(path) || path.isOmitted()) {
         filesToQuery.add(path);
         digestsToQuery.add(path.getDigest());
       } else {
@@ -335,7 +334,7 @@ private Single<PathConverter> upload(Set<Path> files) {
                 .collect(Collectors.toList())
                 .flatMap(paths -> queryRemoteCache(remoteCache, context, paths))
                 .flatMap(paths -> uploadLocalFiles(remoteCache, context, paths))
-                .map(paths -> new PathConverterImpl(remoteServerInstanceName, paths)),
+                .map(paths -> new PathConverterImpl(remoteServerInstanceName, paths, remoteBuildEventUploadMode)),
         RemoteCache::release);
   }
 
@@ -374,7 +373,7 @@ private static class PathConverterImpl implements PathConverter {
     private final Set<Path> skippedPaths;
     private final Set<Path> localPaths;
 
-    PathConverterImpl(String remoteServerInstanceName, List<PathMetadata> uploads) {
+    PathConverterImpl(String remoteServerInstanceName, List<PathMetadata> uploads, RemoteBuildEventUploadMode remoteBuildEventUploadMode) {
       Preconditions.checkNotNull(uploads);
       this.remoteServerInstanceName = remoteServerInstanceName;
       pathToDigest = new HashMap<>(uploads.size());
@@ -384,7 +383,10 @@ private static class PathConverterImpl implements PathConverter {
         Path path = pair.getPath();
         Digest digest = pair.getDigest();
         if (digest != null) {
-          if (pair.isRemote()) {
+          // Always use bytestream:// in MINIMAL mode
+          if (remoteBuildEventUploadMode == RemoteBuildEventUploadMode.MINIMAL) {
+            pathToDigest.put(path, digest);
+          } else if (pair.isRemote()) {
             pathToDigest.put(path, digest);
           } else {
             localPaths.add(path);

src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java

@@ -302,9 +302,8 @@ public String getTypeDescription() {
           "If set to 'all', all local outputs referenced by BEP are uploaded to remote cache.\n"
               + "If set to 'minimal', local outputs referenced by BEP are not uploaded to the"
               + " remote cache, except for files that are important to the consumers of BEP (e.g."
-              + " test logs and timing profile).\n"
-              + "file:// scheme is used for the paths of local files and bytestream:// scheme is"
-              + " used for the paths of (already) uploaded files.\n"
+              + " test logs and timing profile). bytestream:// scheme is always used for the uri of"
+              + " files even if they are missing from remote cache.\n"
               + "Default to 'all'.")
   public RemoteBuildEventUploadMode remoteBuildEventUploadMode;
 

src/test/shell/bazel/remote/remote_build_event_uploader_test.sh

@@ -71,6 +71,29 @@ else
   declare -r EXE_EXT=""
 fi
 
+BEP_JSON=bep.json
+
+function expect_bes_file_uploaded() {
+  local file=$1
+  if [[ $(cat $BEP_JSON) =~ ${file}\",\"uri\":\"bytestream://localhost:${worker_port}/blobs/([^/]*) ]]; then
+    remote_cas_file_exist ${BASH_REMATCH[1]} || (cat $BEP_JSON >> $TEST_log && append_remote_cas_files $TEST_log && fail "$file is not uploaded")
+  else
+    cat $BEP_JSON > $TEST_log
+    fail "$file is not converted to bytestream://"
+  fi
+}
+
+function expect_bes_file_not_uploaded() {
+  local file=$1
+  if [[ $(cat $BEP_JSON) =~ ${file}\",\"uri\":\"bytestream://localhost:${worker_port}/blobs/([^/]*) ]]; then
+    remote_cas_file_exist ${BASH_REMATCH[1]} && (cat $BEP_JSON >> $TEST_log && append_remote_cas_files $TEST_log && fail "$file is uploaded")
+    return 0
+  else
+    cat $BEP_JSON > $TEST_log
+    fail "$file is not converted to bytestream://"
+  fi
+}
+
 function test_upload_minimal_convert_paths_for_existed_blobs() {
   mkdir -p a
   cat > a/BUILD <<EOF
@@ -84,12 +107,11 @@ EOF
   bazel build \
       --remote_executor=grpc://localhost:${worker_port} \
       --experimental_remote_build_event_upload=minimal \
-      --build_event_json_file=bep.json \
+      --build_event_json_file=$BEP_JSON \
       //a:foo >& $TEST_log || fail "Failed to build"
 
-  cat bep.json > $TEST_log
-  expect_log "a:foo.*bytestream://" || fail "paths for existed blobs should be converted"
-  expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data"
+  expect_bes_file_uploaded foo.txt
+  expect_bes_file_uploaded command.profile.gz
 }
 
 function test_upload_minimal_doesnt_upload_missing_blobs() {
@@ -106,12 +128,11 @@ EOF
   bazel build \
       --remote_executor=grpc://localhost:${worker_port} \
       --experimental_remote_build_event_upload=minimal \
-      --build_event_json_file=bep.json \
+      --build_event_json_file=$BEP_JSON \
       //a:foo >& $TEST_log || fail "Failed to build"
 
-  cat bep.json > $TEST_log
-  expect_not_log "a:foo.*bytestream://" || fail "local files are uploaded"
-  expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data"
+  expect_bes_file_not_uploaded foo.txt
+  expect_bes_file_uploaded command.profile.gz
 }
 
 function test_upload_minimal_respect_no_upload_results() {
@@ -128,12 +149,11 @@ EOF
       --remote_cache=grpc://localhost:${worker_port} \
       --remote_upload_local_results=false \
       --experimental_remote_build_event_upload=minimal \
-      --build_event_json_file=bep.json \
+      --build_event_json_file=$BEP_JSON \
       //a:foo >& $TEST_log || fail "Failed to build"
 
-  cat bep.json > $TEST_log
-  expect_not_log "a:foo.*bytestream://" || fail "local files are uploaded"
-  expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data"
+  expect_bes_file_not_uploaded "foo.txt"
+  expect_bes_file_uploaded command.profile.gz
 }
 
 function test_upload_minimal_respect_no_upload_results_combined_cache() {
@@ -154,12 +174,11 @@ EOF
       --incompatible_remote_results_ignore_disk \
       --remote_upload_local_results=false \
       --experimental_remote_build_event_upload=minimal \
-      --build_event_json_file=bep.json \
+      --build_event_json_file=$BEP_JSON \
       //a:foo >& $TEST_log || fail "Failed to build"
 
-  cat bep.json > $TEST_log
-  expect_not_log "a:foo.*bytestream://" || fail "local files are uploaded"
-  expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data"
+  expect_bes_file_not_uploaded "foo.txt"
+  expect_bes_file_uploaded command.profile.gz
   remote_cas_files="$(count_remote_cas_files)"
   [[ "$remote_cas_files" == 1 ]] || fail "Expected 1 remote cas entries, not $remote_cas_files"
   disk_cas_files="$(count_disk_cas_files $cache_dir)"
@@ -187,12 +206,11 @@ EOF
   bazel build \
       --remote_executor=grpc://localhost:${worker_port} \
       --experimental_remote_build_event_upload=minimal \
-      --build_event_json_file=bep.json \
+      --build_event_json_file=$BEP_JSON \
       //a:foo-alias >& $TEST_log || fail "Failed to build"
 
-  cat bep.json > $TEST_log
-  expect_not_log "a:foo.*bytestream://"
-  expect_log "command.profile.gz.*bytestream://"
+  expect_bes_file_not_uploaded foo.txt
+  expect_bes_file_uploaded command.profile.gz
 }
 
 function test_upload_minimal_trees_doesnt_upload_missing_blobs() {
@@ -205,7 +223,7 @@ def _gen_output_dir_impl(ctx):
         inputs = [],
         command = """
           mkdir -p $1/sub; \
-          index=0; while ((index<10)); do echo $index >$1/$index.txt; index=$(($index+1)); done
+          index=0; while ((index<2)); do echo $index >$1/$index.txt; index=$(($index+1)); done
           echo "Shuffle, duffle, muzzle, muff" > $1/sub/bar
         """,
         arguments = [output_dir.path],
@@ -233,13 +251,13 @@ EOF
   bazel build \
       --remote_executor=grpc://localhost:${worker_port} \
       --experimental_remote_build_event_upload=minimal \
-      --build_event_json_file=bep.json \
+      --build_event_json_file=$BEP_JSON \
       //a:foo >& $TEST_log || fail "Failed to build"
 
-  cat bep.json > $TEST_log
-  expect_not_log "a:foo.*bytestream://" || fail "local tree files are uploaded"
-  expect_not_log "a/dir/.*bytestream://" || fail "local tree files are uploaded"
-  expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data"
+  expect_bes_file_not_uploaded dir/0.txt
+  expect_bes_file_not_uploaded dir/1.txt
+  expect_bes_file_not_uploaded dir/sub/bar
+  expect_bes_file_uploaded command.profile.gz
 }
 
 function test_upload_minimal_upload_testlogs() {
@@ -259,14 +277,13 @@ EOF
   bazel test \
       --remote_executor=grpc://localhost:${worker_port} \
       --experimental_remote_build_event_upload=minimal \
-      --build_event_json_file=bep.json \
+      --build_event_json_file=$BEP_JSON \
       //a:test >& $TEST_log || fail "Failed to build"
 
-  cat bep.json > $TEST_log
-  expect_not_log "test.sh.*bytestream://" || fail "test script is uploaded"
-  expect_log "test.log.*bytestream://" || fail "should upload test.log"
-  expect_log "test.xml.*bytestream://" || fail "should upload test.xml"
-  expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data"
+  expect_bes_file_not_uploaded test.sh
+  expect_bes_file_uploaded test.log
+  expect_bes_file_uploaded test.xml
+  expect_bes_file_uploaded command.profile.gz
 }
 
 function test_upload_minimal_upload_profile() {
@@ -283,11 +300,10 @@ EOF
       --remote_executor=grpc://localhost:${worker_port} \
       --experimental_remote_build_event_upload=minimal \
       --profile=mycommand.profile.gz \
-      --build_event_json_file=bep.json \
+      --build_event_json_file=$BEP_JSON \
       //a:foo >& $TEST_log || fail "Failed to build"
 
-  cat bep.json > $TEST_log
-  expect_log "mycommand.profile.gz.*bytestream://" || fail "should upload profile data"
+  expect_bes_file_uploaded "mycommand.profile.gz"
 }
 
 run_suite "Remote build event uploader tests"

src/test/shell/bazel/remote/remote_utils.sh

@@ -96,3 +96,12 @@ function count_remote_cas_files() {
     echo 0
   fi
 }
+
+function remote_cas_file_exist() {
+  local file=$1
+  [ -f "$cas_path/cas/${file:0:2}/$file" ]
+}
+
+function append_remote_cas_files() {
+  find "$cas_path/cas" -type f >> $1
+}