Repository: bazelbuild/bazel
Issue: Remote-cache not work in 5.2.0
Labels: type: bug
untriaged
team-Remote-Exec

### Description of the bug:


When upgrade to the latest version of bazel. Bazel remote seems not to work. Bazel can only put but can not get. When use bazel 5.1.1 bazel remote works again. More strange, bazel 5.2.0 can use the cache of 5.1.1. It seems that the artifact put by bazel 5.2 has problem.

### What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
Use bazel 5.2.0 and use `https://github.com/buchgr/bazel-remote` as remote cache like `bazel build --remote_cache=http://localhost:9090`

### Which operating system are you running Bazel on?


Ubuntu 20.04

### What is the output of `bazel info release`?


release 5.2.0

### If `bazel info release` returns `development version` or `(@non-git)`, tell us how you built Bazel.


_No response_

### What's the output of `git remote get-url origin; git rev-parse master; git rev-parse HEAD` ?


_No response_

### Have you found anything relevant by searching the web?

_No response_

### Any other information, logs, or outputs that you want to share?

_No response_

Issue comments:
people are reporting this is causing the issue

https://bazelbuild.slack.com/archives/CA31HN1T3/p1655638147979559?thread_ts=1655544708.176529&cid=CA31HN1T3

https://github.com/bazelbuild/bazel/commit/4d900ceea12919ad62012830a95e51f9ec1a48bb
@coeuvre 
What's error message looks like? Does it also the case for HEAD?
I am not sure, it seems the cache is not uploading artifacts in certain scenarios, there are no diagnostic messages. I can't tell if people reporting the issues are only using http caching though, but the person in the slack thread and this report are both using http cache.
I also confirm this is a problem. We didn't notice at the beginning of the upgrade as we were using the previously populated remote cache but after a couple of days we were having mostly cache misses and less and less cache hits. The problem happens both when using the buildbuddy grpc cache or a simple GCS Bucket. Reverting to 5.1.1 makes everything working again.
I am not able to reproduce this locally. Do you know which actions are not uploading artifacts? 
@coeuvre it actually happens in rules from `rules_nodejs` we are using like `TsProject` https://github.com/bazelbuild/rules_nodejs/blob/stable/packages/typescript/index.bzl#L109
We reverted back to 5.1.1 due to this, however we do not use `rules_nodejs` (mainly `java_library` and `genrule`).

We noticed because the bazel-remote logs were **all** GET 404.
> 

Same error.
@nickbreen Are you using directory inputs/outputs? `rules_nodejs` does and 5.2 includes https://github.com/bazelbuild/bazel/commit/26f878325e915e0905626a0e4c8bbacffd72f875, but if you are facing the same issues without relying on TreeArtifacts, then that commit is less likely to be the culprit (which would make it rather likely it's actually https://github.com/bazelbuild/bazel/commit/4d900ceea12919ad62012830a95e51f9ec1a48bb).
We have various rules using TreeArtifacts (`ctx.actions.declare_directory`) and rules that produce a directory output using `ctx.outputs` and genrules doing the same.
We have also noticed this similar bug where caching seems to have broken on 5.2.0. When I was tracing the request logs something seemed odd in terms of the action cache `GET` missing and content addressable `PUT`. I could be missing a detail here, maybe this will help narrow down to what other folks might be seeing.


Before doing a build there is a miss on the action cache `GET 404       127.0.0.1 /ac/877dcafd4dc610e6cc734aa39f91aaf9a8e9a7f730e5bd1021c557c5c74bfda5` 
https://gist.github.com/adam-singer/0c73500e47d1a2410924dc5be4da0a3a#file-gistfile1-txt-L1 

Bazel/rules go along their business and build/calculate results, then I would assume that the next time `877dcafd4dc610e6cc734aa39f91aaf9a8e9a7f730e5bd1021c557c5c74bfda5` is seen in the request logs it would be a `PUT` to `/ac/877dcafd4dc610e6cc734aa39f91aaf9a8e9a7f730e5bd1021c557c5c74bfda5`, but what I've seen in our request logs is the remote uploader is making the request to `PUT 200       127.0.0.1 /cas/877dcafd4dc610e6cc734aa39f91aaf9a8e9a7f730e5bd1021c557c5c74bfda5` https://gist.github.com/adam-singer/0c73500e47d1a2410924dc5be4da0a3a#file-gistfile1-txt-L62 the content addressable cache.

Could someone confirm or verify seeing similar behavior and if that is expected? 

**Edit:**

https://github.com/bazelbuild/bazel/commit/4d900ceea12919ad62012830a95e51f9ec1a48bb#diff-5556e28152f91eac5403663774979225b86e64133da164366b56f91d1daa641cR536 The `Step` is set `action.getRemoteActionExecutionContext().setStep(Step.CHECK_ACTION_CACHE);` but when comes time to upload via `DiskAndRemoteCacheClient.uploadActionResult` https://github.com/bazelbuild/bazel/commit/4d900ceea12919ad62012830a95e51f9ec1a48bb#diff-f826b1e1a71d4a2d9f9524894df3069dcb54964998277721021093ff7d55d194R62 the check is for `Step.UPLOAD_OUTPUTS`, that seems to be the reason uploading to action cache isn't happening.

<img width="1469" alt="Screen Shot 2022-07-05 at 10 55 07 PM" src="https://user-images.githubusercontent.com/654526/177479745-75124c77-c2ef-4dd5-9153-dc31b35ce19e.png">

This patch fixed uploading to action cache https://gist.github.com/adam-singer/6500f268885f1c2c604fd4f9f1fec439, would prefer if owners verify/confirm bug.

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
index 2b92eac6832932..e94875ec9e3f6a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -1240,6 +1240,8 @@ public void uploadOutputs(RemoteAction action, SpawnResult spawnResult)
         SpawnResult.Status.SUCCESS.equals(spawnResult.status()) && spawnResult.exitCode() == 0,
         "shouldn't upload outputs of failed local action");
 
+    action.getRemoteActionExecutionContext().setStep(Step.UPLOAD_OUTPUTS);
+
     if (remoteOptions.remoteCacheAsync) {
       Single.using(
               remoteCache::retain,


Test patch:
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index c215615c12255a..f45f93dbb2f361 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -2429,6 +2429,33 @@ EOF
   rm -rf $cache
 }
 
+function test_combined_cache_upload() {
+  mkdir -p a
+  echo 'bar' > a/bar
+  cat > a/BUILD <<'EOF'
+genrule(
+  name = "foo",
+  srcs = [":bar"],
+  outs = ["foo.txt"],
+  cmd = """
+    echo $(location :bar) > "$@"
+  """,
+)
+EOF
+
+  CACHEDIR=$(mktemp -d)
+
+  bazel build \
+    --disk_cache=$CACHEDIR \
+    --remote_cache=grpc://localhost:${worker_port} \
+    //a:foo >& $TEST_log || "Failed to build //a:foo"
+
+  remote_ac_files="$(count_remote_ac_files)"
+  [[ "$remote_ac_files" == 1 ]] || fail "Expected 1 remote action cache entries, not $remote_ac_files"
+  remote_cas_files="$(count_remote_cas_files)"
+  [[ "$remote_cas_files" == 3 ]] || fail "Expected 3 remote cas entries, not $remote_cas_files"
+}
+
 function test_combined_cache_with_no_remote_cache_tag_remote_cache() {
   # Test that actions with no-remote-cache tag can hit disk cache of a combined cache but
   # remote cache is disabled.