Write the action digest to the execution log also for cache misses.

Specifically, introduce a getter/setter pair to store the digest in the SpawnExecutionContext when looking up the disk or remote cache, and retrieve it later when constructing the SpawnResult.

Fixes bazelbuild#16741.

PiperOrigin-RevId: 577799982
Change-Id: I570db74f4dbeee95e00853883eda68b25a84934c

src/main/java/com/google/devtools/build/lib/actions/BUILD

@@ -208,6 +208,7 @@ java_library(
         "//src/main/protobuf:action_cache_java_proto",
         "//src/main/protobuf:extra_actions_base_java_proto",
         "//src/main/protobuf:failure_details_java_proto",
+        "//src/main/protobuf:spawn_java_proto",
         "//third_party:auto_value",
         "//third_party:checker_framework_annotations",
         "//third_party:flogger",

src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java

@@ -13,12 +13,12 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
-import com.google.auto.value.AutoValue;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.devtools.build.lib.bugreport.BugReport;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.exec.Protos.Digest;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
 import com.google.devtools.build.lib.shell.TerminationStatus;
 import com.google.devtools.build.lib.util.DetailedExitCode;
@@ -275,19 +275,11 @@ String getDetailMessage(
   /** Whether the spawn result was obtained through remote strategy. */
   boolean wasRemote();
 
-  /** A unique identifier for the spawn. */
-  @AutoValue
-  @Immutable
-  public abstract class Digest {
-    public abstract String getHash();
-
-    public abstract Long getSizeBytes();
-
-    public static Digest of(String hash, Long sizeBytes) {
-      return new AutoValue_SpawnResult_Digest(hash, sizeBytes);
-    }
-  }
-
+  /**
+   * Returns the remote or disk cache digest.
+   *
+   * <p>Only available when remote execution, remote cache or disk cache was enabled for the spawn.
+   */
   @Nullable
   default Digest getDigest() {
     return null;

src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java

@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.exec;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.util.concurrent.Futures.immediateVoidFuture;
 import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
 
@@ -44,6 +46,7 @@
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.exec.Protos.Digest;
 import com.google.devtools.build.lib.exec.SpawnCache.CacheHandle;
 import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
 import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
@@ -229,6 +232,8 @@ private final class SpawnExecutionContextImpl implements SpawnExecutionContext {
     private SortedMap<PathFragment, ActionInput> lazyInputMapping;
     private PathFragment inputMappingBaseDirectory;
 
+    @Nullable private Digest digest;
+
     SpawnExecutionContextImpl(
         Spawn spawn,
         ActionExecutionContext actionExecutionContext,
@@ -245,6 +250,24 @@ public int getId() {
       return id;
     }
 
+    @Override
+    public void setDigest(Digest digest) {
+      if (this.digest != null) {
+        checkArgument(
+            this.digest.equals(digest),
+            "setDigest was called more than once with different digests: %s vs %s",
+            this.digest,
+            digest);
+      }
+      this.digest = checkNotNull(digest);
+    }
+
+    @Override
+    @Nullable
+    public Digest getDigest() {
+      return digest;
+    }
+
     @Override
     public ListenableFuture<Void> prefetchInputs()
         throws IOException, ForbiddenActionInputException {

src/main/java/com/google/devtools/build/lib/exec/BUILD

@@ -33,6 +33,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/protobuf:failure_details_java_proto",
+        "//src/main/protobuf:spawn_java_proto",
         "//third_party:guava",
         "//third_party:jsr305",
     ],
@@ -298,6 +299,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/util/io",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
+        "//src/main/protobuf:spawn_java_proto",
         "//third_party:auto_value",
         "//third_party:guava",
         "//third_party:jsr305",

src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java

@@ -56,7 +56,7 @@ public void store(SpawnResult result) throws InterruptedException, IOException {
   /**
    * Helper method to create a {@link CacheHandle} from a successful {@link SpawnResult} instance.
    */
-  public static CacheHandle success(final SpawnResult result) {
+  public static CacheHandle success(SpawnResult result) {
     return new CacheHandle() {
       @Override
       public boolean hasResult() {

src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java

@@ -171,11 +171,9 @@ public void logSpawn(
     builder.setExitCode(result.exitCode());
     builder.setRemoteCacheHit(result.isCacheHit());
     builder.setRunner(result.getRunnerName());
+
     if (result.getDigest() != null) {
-      builder
-          .getDigestBuilder()
-          .setHash(result.getDigest().getHash())
-          .setSizeBytes(result.getDigest().getSizeBytes());
+      builder.setDigest(result.getDigest());
     }
 
     String progressMessage = spawn.getResourceOwner().getProgressMessage();

src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java

@@ -28,6 +28,7 @@
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.exec.Protos.Digest;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -128,12 +129,33 @@ interface ProgressStatus {
    */
   interface SpawnExecutionContext {
     /**
-     * Returns a unique id for this spawn, to be used for logging. Note that a single spawn may be
-     * passed to multiple {@link SpawnRunner} implementations, so any log entries should also
-     * contain the identity of the spawn runner implementation.
+     * Returns an id for this spawn, unique within the context of this Bazel server instance, to be
+     * used for logging. Note that a single spawn may be passed to multiple {@link SpawnRunner}
+     * implementations, so any log entries should also contain the identity of the spawn runner
+     * implementation.
      */
     int getId();
 
+    /**
+     * Sets the remote or disk cache digest for this spawn.
+     *
+     * <p>This is the digest that identifies a spawn result stored in a remote or disk cache. It
+     * should be set whenever the spawn is looked up in the cache, and later retrieved via {@link
+     * #getDigest} to be incorporated in the {@link SpawnResult} for a spawn that was executed due
+     * to a cache miss.
+     *
+     * @throws IllegalStateException if called multiple times with different digests.
+     */
+    void setDigest(Digest digest);
+
+    /**
+     * Returns the remote or disk cache digest for this spawn.
+     *
+     * <p>Only available if {@link #setDigest} has been previously called.
+     */
+    @Nullable
+    Digest getDigest();
+
     /**
      * Prefetches the Spawns input files to the local machine. There are cases where Bazel runs on a
      * network file system, and prefetching the files in parallel is a significant performance win.
@@ -316,4 +338,11 @@ default boolean canExecWithLegacyFallback(Spawn spawn) {
    * @throws IOException if there are problems deleting the entries
    */
   default void cleanupSandboxBase(Path sandboxBase, TreeDeleter treeDeleter) throws IOException {}
+
+  /**
+   * Returns a {@link SpawnResult.Builder} prepopulated with the runner name and the spawn digest.
+   */
+  default SpawnResult.Builder getSpawnResultBuilder(SpawnExecutionContext context) {
+    return new SpawnResult.Builder().setRunnerName(getName()).setDigest(context.getDigest());
+  }
 }

src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java

@@ -326,7 +326,7 @@ private SpawnResult start()
       logger.atInfo().log("starting local subprocess #%d, argv: %s", id, debugCmdString());
 
       SpawnResult.Builder spawnResultBuilder =
-          new SpawnResult.Builder().setRunnerName(getName()).setExecutorHostname(hostName);
+          getSpawnResultBuilder(context).setExecutorHostname(hostName);
 
       FileOutErr outErr = context.getFileOutErr();
       String actionType = spawn.getResourceOwner().getMnemonic();

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

@@ -195,7 +195,8 @@ public void registerRemoteSpawnStrategy(SpawnStrategyRegistry.Builder registryBu
             env.getReporter(),
             retryScheduler,
             logDir,
-            getRemoteExecutionService());
+            getRemoteExecutionService(),
+            digestUtil);
     registryBuilder.registerStrategy(
         new RemoteSpawnStrategy(env.getExecRoot(), spawnRunner, executionOptions), "remote");
   }
@@ -211,7 +212,8 @@ public void registerSpawnCache(ModuleActionContextRegistry.Builder registryBuild
             env.getExecRoot(),
             checkNotNull(env.getOptions().getOptions(RemoteOptions.class)),
             checkNotNull(env.getOptions().getOptions(ExecutionOptions.class)).verboseFailures,
-            getRemoteExecutionService());
+            getRemoteExecutionService(),
+            digestUtil);
     registryBuilder.register(SpawnCache.class, spawnCache, "remote-cache");
   }
 

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

@@ -40,6 +40,7 @@
 import com.google.devtools.build.lib.remote.common.BulkTransferException;
 import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
+import com.google.devtools.build.lib.remote.util.DigestUtil;
 import com.google.devtools.build.lib.remote.util.Utils;
 import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput;
 import com.google.devtools.build.lib.vfs.Path;
@@ -55,18 +56,21 @@ final class RemoteSpawnCache implements SpawnCache {
 
   private final Path execRoot;
   private final RemoteOptions options;
-  private final boolean verboseFailures;
   private final RemoteExecutionService remoteExecutionService;
+  private final DigestUtil digestUtil;
+  private final boolean verboseFailures;
 
   RemoteSpawnCache(
       Path execRoot,
       RemoteOptions options,
       boolean verboseFailures,
-      RemoteExecutionService remoteExecutionService) {
+      RemoteExecutionService remoteExecutionService,
+      DigestUtil digestUtil) {
     this.execRoot = execRoot;
     this.options = options;
     this.verboseFailures = verboseFailures;
     this.remoteExecutionService = remoteExecutionService;
+    this.digestUtil = digestUtil;
   }
 
   @VisibleForTesting
@@ -93,6 +97,8 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
             .setInputBytes(action.getInputBytes())
             .setInputFiles(action.getInputFiles());
 
+    context.setDigest(digestUtil.asSpawnLogProto(action.getActionKey()));
+
     Profiler prof = Profiler.instance();
     if (shouldAcceptCachedResult) {
       context.report(SPAWN_CHECKING_CACHE_EVENT);
@@ -119,9 +125,10 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
               .setNetworkTimeInMs((int) action.getNetworkTime().getDuration().toMillis());
           SpawnResult spawnResult =
               createSpawnResult(
+                  digestUtil,
                   action.getActionKey(),
                   result.getExitCode(),
-                  /*cacheHit=*/ true,
+                  /* cacheHit= */ true,
                   result.cacheName(),
                   inMemoryOutput,
                   result.getExecutionMetadata().getExecutionStartTimestamp(),

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

@@ -61,6 +61,7 @@
 import com.google.devtools.build.lib.remote.common.BulkTransferException;
 import com.google.devtools.build.lib.remote.common.OperationObserver;
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
+import com.google.devtools.build.lib.remote.util.DigestUtil;
 import com.google.devtools.build.lib.remote.util.Utils;
 import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput;
 import com.google.devtools.build.lib.server.FailureDetails;
@@ -104,6 +105,7 @@ public class RemoteSpawnRunner implements SpawnRunner {
   private final RemoteRetrier retrier;
   private final Path logDir;
   private final RemoteExecutionService remoteExecutionService;
+  private final DigestUtil digestUtil;
 
   // Used to ensure that a warning is reported only once.
   private final AtomicBoolean warningReported = new AtomicBoolean();
@@ -116,7 +118,8 @@ public class RemoteSpawnRunner implements SpawnRunner {
       @Nullable Reporter cmdlineReporter,
       ListeningScheduledExecutorService retryService,
       Path logDir,
-      RemoteExecutionService remoteExecutionService) {
+      RemoteExecutionService remoteExecutionService,
+      DigestUtil digestUtil) {
     this.execRoot = execRoot;
     this.remoteOptions = remoteOptions;
     this.executionOptions = executionOptions;
@@ -125,6 +128,7 @@ public class RemoteSpawnRunner implements SpawnRunner {
     this.retrier = createExecuteRetrier(remoteOptions, retryService);
     this.logDir = logDir;
     this.remoteExecutionService = remoteExecutionService;
+    this.digestUtil = digestUtil;
   }
 
   @VisibleForTesting
@@ -187,6 +191,8 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
 
     RemoteAction action = remoteExecutionService.buildRemoteAction(spawn, context);
 
+    context.setDigest(digestUtil.asSpawnLogProto(action.getActionKey()));
+
     SpawnMetrics.Builder spawnMetrics =
         SpawnMetrics.Builder.forRemoteExec()
             .setInputBytes(action.getInputBytes())
@@ -435,6 +441,7 @@ private SpawnResult downloadAndFinalizeSpawnResult(
     // subtract network time consumed here to ensure wall clock during fetch is not double
     // counted, and metrics time computation does not exceed total time
     return createSpawnResult(
+        digestUtil,
         action.getActionKey(),
         result.getExitCode(),
         cacheHit,

src/main/java/com/google/devtools/build/lib/remote/util/BUILD

@@ -18,7 +18,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
-        "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
         "//src/main/java/com/google/devtools/build/lib/authandtls",
         "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper",
@@ -31,6 +30,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/protobuf:failure_details_java_proto",
+        "//src/main/protobuf:spawn_java_proto",
         "//third_party:guava",
         "//third_party:jsr305",
         "//third_party:rxjava3",

src/main/java/com/google/devtools/build/lib/remote/util/DigestUtil.java

@@ -103,6 +103,14 @@ public ActionKey asActionKey(Digest digest) {
     return new ActionKey(digest);
   }
 
+  public com.google.devtools.build.lib.exec.Protos.Digest asSpawnLogProto(ActionKey actionKey) {
+    return com.google.devtools.build.lib.exec.Protos.Digest.newBuilder()
+        .setHash(actionKey.getDigest().getHash())
+        .setSizeBytes(actionKey.getDigest().getSizeBytes())
+        .setHashFunctionName(getDigestFunction().toString())
+        .build();
+  }
+
   /** Returns the hash of {@code data} in binary. */
   public byte[] hash(byte[] data) {
     return hashFn.getHashFunction().hashBytes(data).asBytes();

src/main/java/com/google/devtools/build/lib/remote/util/Utils.java

@@ -143,6 +143,7 @@ public static PathFragment getInMemoryOutputPath(Spawn spawn) {
 
   /** Constructs a {@link SpawnResult}. */
   public static SpawnResult createSpawnResult(
+      DigestUtil digestUtil,
       ActionKey actionKey,
       int exitCode,
       boolean cacheHit,
@@ -168,9 +169,7 @@ public static SpawnResult createSpawnResult(
                         .toMillis())
             .setSpawnMetrics(spawnMetrics)
             .setRemote(true)
-            .setDigest(
-                SpawnResult.Digest.of(
-                    actionKey.getDigest().getHash(), actionKey.getDigest().getSizeBytes()));
+            .setDigest(digestUtil.asSpawnLogProto(actionKey));
     if (exitCode != 0) {
       builder.setFailureDetail(
           FailureDetail.newBuilder()