Store TTL in RemoteFileArtifactValue

When building without the bytes, Bazel stores `RemoteFileArtifactValue` in skyframe (inmemory) and in local action cache which represents a file that is stored remotely. Bazel assumes that the remote file will never expire which is wrong. In practice, remote cache often evict files due to space constraint, and when it happens, the builds could fail.

This PR introduces flag `--experimental_remote_cache_ttl` which tells Bazel at least how long the remote cache could store a file after returning a reference of it to Bazel. Bazel calculates the TTL of the file and store it in the `RemoteFileArtifactValue`. In an incremental build, Bazel will discard the `RemoteFileArtifactValue` and rerun the generating actions if it finds out that the `RemoteFileArtifactValue` is expired. The new field `expireAtEpochMilli` replaces `actionId` (deleted by f62a8b9), so there shouldn't be memory regression.

There are two places Bazel checks the TTL:
1. If the skyframe has in-memory state about previous builds (e.g. incremental builds), the `SkyValue`s are marked as dirty if the `RemoteFileArtifactValue` is expired.
2. When checking local action cache, if the `RemoteFileArtifactValue` is expired, the cache entry is ignored.

So that the generating actions can be re-executed.

Part of bazelbuild#16660.

Closes bazelbuild#17639.

RELNOTES: Add flag `--experimental_remote_cache_ttl` and set the default value to 3 hours.
PiperOrigin-RevId: 513819724
Change-Id: I9c9813621d04d5b1b94312be39384962feae2f7b

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

@@ -46,6 +46,7 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -317,6 +318,7 @@ private CachedOutputMetadata(
 
   private static CachedOutputMetadata loadCachedOutputMetadata(
       Action action, ActionCache.Entry entry, MetadataHandler metadataHandler) {
+    Instant now = Instant.now();
     ImmutableMap.Builder<Artifact, RemoteFileArtifactValue> remoteFileMetadata =
         ImmutableMap.builder();
     ImmutableMap.Builder<SpecialArtifact, TreeArtifactValue> mergedTreeMetadata =
@@ -330,6 +332,17 @@ private static CachedOutputMetadata loadCachedOutputMetadata(
           continue;
         }
 
+        // If any child is not alive, discard the entire tree
+        if (cachedTreeMetadata.childValues().values().stream()
+            .anyMatch(metadata -> !metadata.isAlive(now))) {
+          continue;
+        }
+
+        if (cachedTreeMetadata.archivedFileValue().isPresent()
+            && !cachedTreeMetadata.archivedFileValue().get().isAlive(now)) {
+          continue;
+        }
+
         TreeArtifactValue localTreeMetadata;
         try {
           localTreeMetadata = metadataHandler.getTreeArtifactValue(parent);
@@ -377,7 +390,7 @@ private static CachedOutputMetadata loadCachedOutputMetadata(
         mergedTreeMetadata.put(parent, merged.build());
       } else {
         RemoteFileArtifactValue cachedMetadata = entry.getOutputFile(artifact);
-        if (cachedMetadata == null) {
+        if (cachedMetadata == null || !cachedMetadata.isAlive(now)) {
           continue;
         }
 

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

@@ -34,6 +34,7 @@
 import com.google.devtools.build.skyframe.SkyValue;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.time.Instant;
 import java.util.Arrays;
 import java.util.Objects;
 import java.util.Optional;
@@ -542,27 +543,34 @@ public static class RemoteFileArtifactValue extends FileArtifactValue {
     protected final byte[] digest;
     protected final long size;
     protected final int locationIndex;
+    // The time when the remote file expires in milliseconds since epoch. negative value means the
+    // remote file will never expire. This field doesn't contribute to the equality of the metadata.
+    protected final long expireAtEpochMilli;
 
-    private RemoteFileArtifactValue(byte[] digest, long size, int locationIndex) {
+    private RemoteFileArtifactValue(
+        byte[] digest, long size, int locationIndex, long expireAtEpochMilli) {
       this.digest = Preconditions.checkNotNull(digest);
       this.size = size;
       this.locationIndex = locationIndex;
+      this.expireAtEpochMilli = expireAtEpochMilli;
     }
 
-    public static RemoteFileArtifactValue create(byte[] digest, long size, int locationIndex) {
-      return new RemoteFileArtifactValue(digest, size, locationIndex);
+    public static RemoteFileArtifactValue create(
+        byte[] digest, long size, int locationIndex, long expireAtEpochMilli) {
+      return new RemoteFileArtifactValue(digest, size, locationIndex, expireAtEpochMilli);
     }
 
     public static RemoteFileArtifactValue create(
         byte[] digest,
         long size,
         int locationIndex,
+        long expireAtEpochMilli,
         @Nullable PathFragment materializationExecPath) {
       if (materializationExecPath != null) {
         return new RemoteFileArtifactValueWithMaterializationPath(
-            digest, size, locationIndex, materializationExecPath);
+            digest, size, locationIndex, expireAtEpochMilli, materializationExecPath);
       }
-      return new RemoteFileArtifactValue(digest, size, locationIndex);
+      return new RemoteFileArtifactValue(digest, size, locationIndex, expireAtEpochMilli);
     }
 
     @Override
@@ -613,6 +621,18 @@ public int getLocationIndex() {
       return locationIndex;
     }
 
+    public long getExpireAtEpochMilli() {
+      return expireAtEpochMilli;
+    }
+
+    public boolean isAlive(Instant now) {
+      if (expireAtEpochMilli < 0) {
+        return true;
+      }
+
+      return now.toEpochMilli() < expireAtEpochMilli;
+    }
+
     @Override
     public boolean wasModifiedSinceDigest(Path path) {
       return false;
@@ -629,6 +649,7 @@ public String toString() {
           .add("digest", bytesToString(digest))
           .add("size", size)
           .add("locationIndex", locationIndex)
+          .add("expireAtEpochMilli", expireAtEpochMilli)
           .toString();
     }
   }
@@ -644,8 +665,12 @@ public static final class RemoteFileArtifactValueWithMaterializationPath
     private final PathFragment materializationExecPath;
 
     private RemoteFileArtifactValueWithMaterializationPath(
-        byte[] digest, long size, int locationIndex, PathFragment materializationExecPath) {
-      super(digest, size, locationIndex);
+        byte[] digest,
+        long size,
+        int locationIndex,
+        long expireAtEpochMilli,
+        PathFragment materializationExecPath) {
+      super(digest, size, locationIndex, expireAtEpochMilli);
       this.materializationExecPath = Preconditions.checkNotNull(materializationExecPath);
     }
 
@@ -679,6 +704,7 @@ public String toString() {
           .add("digest", bytesToString(digest))
           .add("size", size)
           .add("locationIndex", locationIndex)
+          .add("expireAtEpochMilli", expireAtEpochMilli)
           .add("materializationExecPath", materializationExecPath)
           .toString();
     }

src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java

@@ -76,7 +76,7 @@ public class CompactPersistentActionCache implements ActionCache {
 
   private static final int NO_INPUT_DISCOVERY_COUNT = -1;
 
-  private static final int VERSION = 15;
+  private static final int VERSION = 16;
 
   private static final class ActionMap extends PersistentMap<Integer, byte[]> {
     private final Clock clock;
@@ -466,6 +466,8 @@ private static void encodeRemoteMetadata(
 
     VarInt.putVarInt(value.getLocationIndex(), sink);
 
+    VarInt.putVarLong(value.getExpireAtEpochMilli(), sink);
+
     Optional<PathFragment> materializationExecPath = value.getMaterializationExecPath();
     if (materializationExecPath.isPresent()) {
       VarInt.putVarInt(1, sink);
@@ -476,10 +478,11 @@ private static void encodeRemoteMetadata(
   }
 
   private static final int MAX_REMOTE_METADATA_SIZE =
-      DigestUtils.ESTIMATED_SIZE
-          + VarInt.MAX_VARLONG_SIZE
-          + VarInt.MAX_VARINT_SIZE
-          + VarInt.MAX_VARINT_SIZE;
+      DigestUtils.ESTIMATED_SIZE // digest
+          + VarInt.MAX_VARLONG_SIZE // size
+          + VarInt.MAX_VARINT_SIZE // locationIndex
+          + VarInt.MAX_VARINT_SIZE // expireAtEpochMilli
+          + VarInt.MAX_VARINT_SIZE; // materializationExecPath
 
   private static RemoteFileArtifactValue decodeRemoteMetadata(
       StringIndexer indexer, ByteBuffer source) throws IOException {
@@ -489,6 +492,8 @@ private static RemoteFileArtifactValue decodeRemoteMetadata(
 
     int locationIndex = VarInt.getVarInt(source);
 
+    long expireAtEpochMilli = VarInt.getVarLong(source);
+
     PathFragment materializationExecPath = null;
     int numMaterializationExecPath = VarInt.getVarInt(source);
     if (numMaterializationExecPath > 0) {
@@ -499,7 +504,8 @@ private static RemoteFileArtifactValue decodeRemoteMetadata(
           PathFragment.create(getStringForIndex(indexer, VarInt.getVarInt(source)));
     }
 
-    return RemoteFileArtifactValue.create(digest, size, locationIndex, materializationExecPath);
+    return RemoteFileArtifactValue.create(
+        digest, size, locationIndex, expireAtEpochMilli, materializationExecPath);
   }
 
   /** @return action data encoded as a byte[] array. */

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

@@ -115,11 +115,12 @@ public void updateContext(MetadataInjector metadataInjector) {
     this.metadataInjector = metadataInjector;
   }
 
-  void injectRemoteFile(PathFragment path, byte[] digest, long size) throws IOException {
+  void injectRemoteFile(PathFragment path, byte[] digest, long size, long expireAtEpochMilli)
+      throws IOException {
     if (!isOutput(path)) {
       return;
     }
-    remoteOutputTree.injectRemoteFile(path, digest, size);
+    remoteOutputTree.injectRemoteFile(path, digest, size, expireAtEpochMilli);
   }
 
   void flush() throws IOException {
@@ -206,6 +207,7 @@ private void maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact outpu
               metadata.getDigest(),
               metadata.getSize(),
               metadata.getLocationIndex(),
+              metadata.getExpireAtEpochMilli(),
               // Avoid a double indirection when the target is already materialized as a symlink.
               metadata.getMaterializationExecPath().orElse(targetPath.relativeTo(execRoot)));
 
@@ -215,7 +217,10 @@ private void maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact outpu
 
   private RemoteFileArtifactValue createRemoteMetadata(RemoteFileInfo remoteFile) {
     return RemoteFileArtifactValue.create(
-        remoteFile.getFastDigest(), remoteFile.getSize(), /* locationIndex= */ 1);
+        remoteFile.getFastDigest(),
+        remoteFile.getSize(),
+        /* locationIndex= */ 1,
+        remoteFile.getExpireAtEpochMilli());
   }
 
   @Override
@@ -743,7 +748,8 @@ protected FileInfo newFile(Clock clock, PathFragment path) {
       return new RemoteFileInfo(clock);
     }
 
-    void injectRemoteFile(PathFragment path, byte[] digest, long size) throws IOException {
+    void injectRemoteFile(PathFragment path, byte[] digest, long size, long expireAtEpochMilli)
+        throws IOException {
       createDirectoryAndParents(path.getParentDirectory());
       InMemoryContentInfo node = getOrCreateWritableInode(path);
       // If a node was already existed and is not a remote file node (i.e. directory or symlink node
@@ -753,7 +759,7 @@ void injectRemoteFile(PathFragment path, byte[] digest, long size) throws IOExce
       }
 
       RemoteFileInfo remoteFileInfo = (RemoteFileInfo) node;
-      remoteFileInfo.set(digest, size);
+      remoteFileInfo.set(digest, size, expireAtEpochMilli);
     }
 
     @Nullable
@@ -771,13 +777,16 @@ static class RemoteFileInfo extends FileInfo {
     private byte[] digest;
     private long size;
 
+    private long expireAtEpochMilli;
+
     RemoteFileInfo(Clock clock) {
       super(clock);
     }
 
-    private void set(byte[] digest, long size) {
+    private void set(byte[] digest, long size, long expireAtEpochMilli) {
       this.digest = digest;
       this.size = size;
+      this.expireAtEpochMilli = expireAtEpochMilli;
     }
 
     @Override
@@ -804,5 +813,9 @@ public byte[] getFastDigest() {
     public long getSize() {
       return size;
     }
+
+    public long getExpireAtEpochMilli() {
+      return expireAtEpochMilli;
+    }
   }
 }

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

@@ -128,6 +128,7 @@
 import io.reactivex.rxjava3.disposables.Disposable;
 import io.reactivex.rxjava3.schedulers.Schedulers;
 import java.io.IOException;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -806,7 +807,8 @@ private void createSymlinks(Iterable<SymlinkMetadata> symlinks) throws IOExcepti
     }
   }
 
-  private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata metadata)
+  private void injectRemoteArtifacts(
+      RemoteAction action, ActionResultMetadata metadata, long expireAtEpochMilli)
       throws IOException {
     FileSystem actionFileSystem = action.getSpawnExecutionContext().getActionFileSystem();
     checkState(actionFileSystem instanceof RemoteActionFileSystem);
@@ -825,15 +827,17 @@ private void injectRemoteArtifacts(RemoteAction action, ActionResultMetadata met
         remoteActionFileSystem.injectRemoteFile(
             file.path().asFragment(),
             DigestUtil.toBinaryDigest(file.digest()),
-            file.digest().getSizeBytes());
+            file.digest().getSizeBytes(),
+            expireAtEpochMilli);
       }
     }
 
     for (FileMetadata file : metadata.files()) {
       remoteActionFileSystem.injectRemoteFile(
           file.path().asFragment(),
           DigestUtil.toBinaryDigest(file.digest()),
-          file.digest().getSizeBytes());
+          file.digest().getSizeBytes(),
+          expireAtEpochMilli);
     }
   }
 
@@ -1162,7 +1166,8 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
         }
       }
 
-      injectRemoteArtifacts(action, metadata);
+      var expireAtEpochMilli = Instant.now().plus(remoteOptions.remoteCacheTtl).toEpochMilli();
+      injectRemoteArtifacts(action, metadata, expireAtEpochMilli);
 
       try (SilentCloseable c = Profiler.instance().profile("Remote.downloadInMemoryOutput")) {
         if (inMemoryOutput != null) {

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

@@ -13,11 +13,16 @@
 // limitations under the License.
 package com.google.devtools.build.lib.remote.options;
 
+import com.google.devtools.common.options.Converter;
+import com.google.devtools.common.options.Converters;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionsBase;
+import com.google.devtools.common.options.OptionsParsingException;
+import java.time.Duration;
 import java.util.List;
+import java.util.regex.Pattern;
 
 /** Options for remote execution and distributed caching that shared between Bazel and Blaze. */
 public class CommonRemoteOptions extends OptionsBase {
@@ -33,4 +38,38 @@ public class CommonRemoteOptions extends OptionsBase {
               + " the client to request certain artifacts that might be needed locally (e.g. IDE"
               + " support)")
   public List<String> remoteDownloadRegex;
+
+  @Option(
+      name = "experimental_remote_cache_ttl",
+      defaultValue = "3h",
+      documentationCategory = OptionDocumentationCategory.REMOTE,
+      effectTags = {OptionEffectTag.EXECUTION},
+      converter = RemoteDurationConverter.class,
+      help =
+          "The guaranteed minimal TTL of blobs in the remote cache after their digests are recently"
+              + " referenced e.g. by an ActionResult or FindMissingBlobs. Bazel does several"
+              + " optimizations based on the blobs' TTL e.g. doesn't repeatedly call"
+              + " GetActionResult in an incremental build. The value should be set slightly less"
+              + " than the real TTL since there is a gap between when the server returns the"
+              + " digests and when Bazel receives them.")
+  public Duration remoteCacheTtl;
+
+  /** Returns the specified duration. Assumes seconds if unitless. */
+  public static class RemoteDurationConverter extends Converter.Contextless<Duration> {
+
+    private static final Pattern UNITLESS_REGEX = Pattern.compile("^[0-9]+$");
+
+    @Override
+    public Duration convert(String input) throws OptionsParsingException {
+      if (UNITLESS_REGEX.matcher(input).matches()) {
+        input += "s";
+      }
+      return new Converters.DurationConverter().convert(input, /* conversionContext= */ null);
+    }
+
+    @Override
+    public String getTypeDescription() {
+      return "An immutable length of time.";
+    }
+  }
 }