Write MTSVs for analysis nodes to skycache and inject them on `--experimental_skycache_analysis_only` builds. Currently, skycache hits have a minimal MTSV. This causes issues for builds that require correct MTSVs. PiperOrigin-RevId: 981245951 Change-Id: I4809d5d6bff55dff1a25c5d3b0b65269fe0993ab
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 8c604f0..6b88bb8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -752,6 +752,7 @@ "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:remote_analysis_cache_client", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/proto:analysis_cache_service_miss_reason_java_proto", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/main/java/com/google/devtools/build/skyframe:version", "//third_party/java/guava:base", ], )
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueRetrieverUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueRetrieverUtils.java index c96e9aa..21e17aa 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueRetrieverUtils.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueRetrieverUtils.java
@@ -30,6 +30,7 @@ import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.RetrievalResult; import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.RetrievedValue; import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.SerializableSkyKeyComputeState; +import com.google.devtools.build.lib.skyframe.serialization.analysis.AnalysisValueWithMtsv; import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheClient; import com.google.devtools.build.lib.skyframe.serialization.analysis.RemoteAnalysisCacheReaderDepsProvider; import com.google.devtools.build.lib.skyframe.serialization.analysis.SkycacheUploadClient; @@ -37,6 +38,7 @@ import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import com.google.devtools.build.skyframe.Version; import java.util.function.Supplier; /** @@ -88,6 +90,8 @@ try { retrievalResult = retriever.tryRetrieve(env, new DefaultDependOnFutureShim(env), client, key, state); + retrievalResult = + maybeUnwrapVersionedSkyValue(key, retrievalResult, env, analysisCachingDeps); if (retrievalResult instanceof RetrievedValue || retrievalResult instanceof NoCachedData) { analysisCachingDeps.recordRetrievalResult( retrievalResult, key, state.getPhaseDurationMicros()); @@ -96,8 +100,6 @@ // Don't crash the build if deserialization failed. Gracefully fallback to local evaluation. analysisCachingDeps.recordSerializationException(e, key, state.getPhaseDurationMicros()); retrievalResult = new NoCachedData(e.getReason()); - } catch (RuntimeException | InterruptedException e) { - throw e; } finally { if (retrievalResult == Restart.RESTART) { state.addRestart(); @@ -116,6 +118,28 @@ return retrievalResult; } + private static RetrievalResult maybeUnwrapVersionedSkyValue( + SkyKey key, + RetrievalResult retrievalResult, + Environment env, + RemoteAnalysisCacheReaderDepsProvider analysisCachingDeps) { + if (!(key instanceof ActionLookupKey) + || !(retrievalResult instanceof RetrievedValue(SkyValue value))) { + return retrievalResult; + } + + if (value instanceof AnalysisValueWithMtsv(SkyValue innerValue, Version mtsv)) { + if (analysisCachingDeps.getSkycacheAnalysisOnly()) { + env.injectVersion(mtsv); + } + return new RetrievedValue(innerValue); + } + if (analysisCachingDeps.getSkycacheAnalysisOnly()) { + return new NoCachedData(MissReason.MISS_REASON_MISSING_MTSV); + } + return retrievalResult; + } + public static void tryUploadAsync( RemoteAnalysisCacheReaderDepsProvider cachingDeps, SkyKey key,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/AnalysisValueWithMtsv.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/AnalysisValueWithMtsv.java new file mode 100644 index 0000000..ebc4136 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/AnalysisValueWithMtsv.java
@@ -0,0 +1,32 @@ +// Copyright 2026 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.skyframe.serialization.analysis; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.skyframe.SkyValue; +import com.google.devtools.build.skyframe.Version; + +/** + * A wrapper for an analysis value (configured target or aspect) and its max transitive source + * version (MTSV), so that they can be written to storage together. + */ +@AutoCodec +public record AnalysisValueWithMtsv(SkyValue value, Version mtsv) implements SkyValue { + public AnalysisValueWithMtsv { + checkNotNull(value, "value"); + checkNotNull(mtsv, "mtsv"); + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD index 7be03d4..6873b9d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD
@@ -121,6 +121,7 @@ java_library( name = "frontier_serializer", srcs = [ + "AnalysisValueWithMtsv.java", "FrontierSerializer.java", "SelectedEntrySerializer.java", "SkyValueUploadedEvent.java", @@ -165,6 +166,7 @@ "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:error_message_helper", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:frontier_node_version", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:write_status", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:registered_execution_platforms_value", "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:registered_toolchains_value", "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_context_key", @@ -173,6 +175,7 @@ "//src/main/java/com/google/devtools/build/lib/versioning:long_version_getter", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/main/java/com/google/devtools/build/skyframe:version", "//src/main/protobuf:failure_details_java_proto", "//src/main/protobuf:file_invalidation_data_java_proto", "//third_party/java/error_prone:annotations",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/SelectedEntrySerializer.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/SelectedEntrySerializer.java index a360fb2..47beb5c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/SelectedEntrySerializer.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/SelectedEntrySerializer.java
@@ -70,6 +70,7 @@ import com.google.devtools.build.skyframe.InMemoryNodeEntry; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import com.google.devtools.build.skyframe.Version; import com.google.errorprone.annotations.DoNotCall; import com.google.protobuf.ByteString; import com.google.protobuf.CodedOutputStream; @@ -357,7 +358,11 @@ throw new MissingSkyframeEntryException(actionLookupKey); } serializationStats.registerAnalysisNode(); - uploadAnalysisEntry(actionLookupKey, entry.getValue(), entry.getDirectDeps()); + uploadAnalysisEntry( + actionLookupKey, + entry.getValue(), + entry.getDirectDeps(), + entry.getMaxTransitiveSourceVersion()); } case ActionLookupData lookupData -> { serializationStats.registerExecutionNode(); @@ -387,11 +392,14 @@ * Uploads an analysis phase entry to Skycache. * * <p>Direct deps must always be given. + * + * <p>If {@code mtsv} is given, it is stored along with the value in a {@link + * AnalysisValueWithMtsv}. */ public void uploadAnalysisEntry( - ActionLookupKey key, SkyValue value, Iterable<SkyKey> directDeps) { + ActionLookupKey key, SkyValue value, Iterable<SkyKey> directDeps, @Nullable Version mtsv) { // For analysis phase entries, we register their own dependencies in the invalidation data - uploadEntry(key, value, key, directDeps); + uploadEntry(key, value, key, directDeps, mtsv); } /** @@ -421,7 +429,7 @@ // anymore. In this case, FileOpNodeMemoizingLookup will definitely contain an entry for it, // since creating one is a side effect of uploading. If we are not deleting them, it will do // a graph lookup anyway. - uploadEntry(key, value, dependencyKey, null); + uploadEntry(key, value, dependencyKey, null, /* mtsv= */ null); } private static ActionLookupKey getDependencyKey(SkyKey key) { @@ -442,13 +450,15 @@ * @param dependencyKey the {@link SkyKey} whose file system dependencies are to be used * @param dependencyDeps the dependencies to traverse. These should be the direct deps of {@code * dependencyDeps}. If null, Skyframe will be asked for the deps of {@code key} + * @param mtsv the max transitive source version of the node, if applicable */ private void uploadEntry( SkyKey key, SkyValue value, ActionLookupKey dependencyKey, - @Nullable Iterable<SkyKey> dependencyDeps) { - new UploadTask(key, value, dependencyKey, dependencyDeps).submit(); + @Nullable Iterable<SkyKey> dependencyDeps, + @Nullable Version mtsv) { + new UploadTask(key, value, dependencyKey, dependencyDeps, mtsv).submit(); } private final class UploadTask @@ -457,7 +467,7 @@ private final SkyValue value; private final ActionLookupKey dependencyKey; @Nullable private final Iterable<SkyKey> dependencyDeps; - private final boolean isExecutionValue; + @Nullable private final Version mtsv; // Keys are always stored as fingerprints so their detailed profiles are omitted. private AsyncSerializationTask keyResultTask; @@ -467,12 +477,13 @@ SkyKey key, SkyValue value, ActionLookupKey dependencyKey, - @Nullable Iterable<SkyKey> dependencyDeps) { + @Nullable Iterable<SkyKey> dependencyDeps, + @Nullable Version mtsv) { this.key = key; this.value = value; this.dependencyKey = dependencyKey; this.dependencyDeps = dependencyDeps; - this.isExecutionValue = isExecutionValue(key); + this.mtsv = mtsv; } void submit() { @@ -496,9 +507,14 @@ codecs.serializeMemoizedAsync( compressionService, fingerprintValueService, key, /* profileCollector= */ null); fingerprintValueService.getExecutor().execute(keyResultTask); + + SkyValue valueToSerialize = + mtsv != null && key instanceof ActionLookupKey + ? new AnalysisValueWithMtsv(value, mtsv) + : value; this.valueResultTask = codecs.serializeMemoizedAsync( - compressionService, fingerprintValueService, value, profileCollector); + compressionService, fingerprintValueService, valueToSerialize, profileCollector); fingerprintValueService.getExecutor().execute(valueResultTask); keyResultTask.addListener( @@ -511,7 +527,8 @@ // We pass a null value for execution entries to maintain the invariant that value is // non-null only for analysis entries. FileOpNodeOrFuture fileOpNodeOrFuture = - fileOpNodes.computeNode(dependencyKey, isExecutionValue ? null : value, dependencyDeps); + fileOpNodes.computeNode( + dependencyKey, isExecutionValue(key) ? null : value, dependencyDeps); switch (fileOpNodeOrFuture) { case FileOpNodeOrEmpty nodeOrEmpty -> onSuccess(nodeOrEmpty); case FutureFileOpNode future -> @@ -651,7 +668,8 @@ } codedOut.writeEnumNoTag( - (isExecutionValue ? DATA_TYPE_EXECUTION_NODE : DATA_TYPE_ANALYSIS_NODE).getNumber()); + (isExecutionValue(key) ? DATA_TYPE_EXECUTION_NODE : DATA_TYPE_ANALYSIS_NODE) + .getNumber()); node.cacheKey().writeTo(codedOut); writeStatuses.addWriteStatus(node.writeStatus()); codedOut.writeRawBytes(valueResult.getObject());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/SkycacheUploadClient.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/SkycacheUploadClient.java index db4ca5e..f0b5973 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/SkycacheUploadClient.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/SkycacheUploadClient.java
@@ -107,7 +107,8 @@ .addAll(temporaryDirectDeps.getAllElementsAsIterable()) .addAll(newlyRequestedDeps) .build(); - selectedEntrySerializer.uploadAnalysisEntry(analysisKey, value, deps); + selectedEntrySerializer.uploadAnalysisEntry( + analysisKey, value, deps, env.getMaxTransitiveSourceVersionSoFar()); } else { // This is an execution-phase entry. We need the deps of its owner, which should be // available
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/proto/analysis_cache_service_miss_reason.proto b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/proto/analysis_cache_service_miss_reason.proto index 1f81d7d..99ab52c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/proto/analysis_cache_service_miss_reason.proto +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/proto/analysis_cache_service_miss_reason.proto
@@ -31,4 +31,7 @@ MISS_REASON_INFRA_FAILURE = 5; // The cache client is saturated and shed the lookup request. MISS_REASON_CACHE_SATURATED = 6; + // The lookup succeeded, but the retrieved value did not contain a max + // transitive source version during a skycache-analysis-only build. + MISS_REASON_MISSING_MTSV = 7; }