Add a series of lost input targets to the ActionRewindingStats object.
This information is useful for determining what actions are more prone to lost inputs and transitively can help determine which actions have a possibility of being rewound.
Note: The Java field ActionRewindingStats.RewoundAction.rewoundLatency is NOT set in this change and currently defaulted to 0.
PiperOrigin-RevId: 299115144
diff --git a/scripts/bootstrap/compile.sh b/scripts/bootstrap/compile.sh
index 0cb1afe..035aa2a 100755
--- a/scripts/bootstrap/compile.sh
+++ b/scripts/bootstrap/compile.sh
@@ -16,7 +16,7 @@
# Script for building bazel from scratch without bazel
-PROTO_FILES=$(ls src/main/protobuf/*.proto src/main/java/com/google/devtools/build/lib/buildeventstream/proto/*.proto src/main/java/com/google/devtools/build/skyframe/*.proto)
+PROTO_FILES=$(ls src/main/protobuf/*.proto src/main/java/com/google/devtools/build/lib/buildeventstream/proto/*.proto src/main/java/com/google/devtools/build/skyframe/*.proto src/main/java/com/google/devtools/build/lib/skyframe/proto/*.proto)
LIBRARY_JARS=$(find derived/jars third_party -name '*.jar' | grep -Fv JavaBuilder | grep -Fv third_party/guava | grep -Fv third_party/guava | grep -ve 'third_party/grpc/grpc.*jar' | tr "\n" " ")
GRPC_JAVA_VERSION=1.20.0
GRPC_LIBRARY_JARS=$(find third_party/grpc -name '*.jar' | grep -e ".*${GRPC_JAVA_VERSION}.*jar" | tr "\n" " ")
@@ -228,6 +228,7 @@
-I. \
-Isrc/main/protobuf/ \
-Isrc/main/java/com/google/devtools/build/lib/buildeventstream/proto/ \
+ -Isrc/main/java/com/google/devtools/build/lib/skyframe/proto/ \
-Isrc/main/java/com/google/devtools/build/skyframe/ \
--java_out=${OUTPUT_DIR}/src \
--plugin=protoc-gen-grpc="${GRPC_JAVA_PLUGIN-}" \
diff --git a/src/BUILD b/src/BUILD
index 32dd215..1873fd1 100644
--- a/src/BUILD
+++ b/src/BUILD
@@ -479,6 +479,7 @@
"//src/main/protobuf:dist_jars",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:dist_jars",
"//src/main/java/com/google/devtools/build/lib/bazel/debug:dist_jars",
+ "//src/main/java/com/google/devtools/build/lib/skyframe/proto:dist_jars",
"//src/main/java/com/google/devtools/build/lib/skylarkdebug/proto:dist_jars",
"//src/main/java/com/google/devtools/build/skydoc/rendering/proto:dist_jars",
"//src/main/java/com/google/devtools/build/skyframe:dist_jars",
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index e5a3083..c8648ec 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -88,6 +88,7 @@
"//src/main/java/com/google/devtools/build/lib/sandbox:srcs",
"//src/main/java/com/google/devtools/build/lib/shell:srcs",
"//src/main/java/com/google/devtools/build/lib/skyframe/packages:srcs",
+ "//src/main/java/com/google/devtools/build/lib/skyframe/proto:srcs",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:srcs",
"//src/main/java/com/google/devtools/build/lib/skyframe/trimming:srcs",
"//src/main/java/com/google/devtools/build/lib/skylarkbuildapi:srcs",
@@ -550,6 +551,7 @@
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/rules/cpp:cpp_interface",
"//src/main/java/com/google/devtools/build/lib/shell",
+ "//src/main/java/com/google/devtools/build/lib/skyframe/proto:action_rewind_event_java_proto",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/trimming:trimmed_configuration_cache",
"//src/main/java/com/google/devtools/build/lib/skylarkbuildapi",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
index 3665fb1..9744a18 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
@@ -14,11 +14,15 @@
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.Comparators.greatest;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static java.util.Comparator.comparing;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
import com.google.common.collect.ConcurrentHashMultiset;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -39,6 +43,7 @@
import com.google.devtools.build.lib.actions.LostInputsActionExecutionException;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.skyframe.proto.ActionRewind.ActionRewindEvent;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyFunction.Restart;
import com.google.devtools.build.skyframe.SkyKey;
@@ -49,6 +54,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.logging.Logger;
import javax.annotation.Nullable;
@@ -60,12 +66,15 @@
public class ActionRewindStrategy {
private static final Logger logger = Logger.getLogger(ActionRewindStrategy.class.getName());
@VisibleForTesting public static final int MAX_REPEATED_LOST_INPUTS = 20;
+ @VisibleForTesting public static final int MAX_ACTION_REWIND_EVENTS = 5;
+ private static final int MAX_LOST_INPUTS_RECORDED = 5;
- // Note that this reference is mutated only outside of Skyframe evaluations, and accessed only
- // inside of them. Its visibility piggybacks on Skyframe evaluation synchronizations, like
+ // Note that these references are mutated only outside of Skyframe evaluations, and accessed only
+ // inside of them. Their visibility piggybacks on Skyframe evaluation synchronizations, like
// ActionExecutionFunction's stateMap does.
private ConcurrentHashMultiset<LostInputRecord> lostInputRecords =
ConcurrentHashMultiset.create();
+ private ConcurrentLinkedQueue<RewindPlanStats> rewindPlansStats = new ConcurrentLinkedQueue<>();
/**
* Returns a {@link RewindPlan} specifying:
@@ -93,7 +102,8 @@
ActionInputDepOwners inputDepOwners,
Environment env)
throws ActionExecutionException, InterruptedException {
- checkIfActionLostInputTooManyTimes(actionLookupData, failedAction, lostInputsException);
+ ImmutableList<LostInputRecord> lostInputRecordsThisAction =
+ checkIfActionLostInputTooManyTimes(actionLookupData, failedAction, lostInputsException);
ImmutableList<ActionInput> lostInputs = lostInputsException.getLostInputs().values().asList();
@@ -134,23 +144,47 @@
checkActions(newlyVisitedActions, env, rewindGraph, additionalActionsToRestart);
}
+ int lostInputRecordsCount = lostInputRecordsThisAction.size();
+ rewindPlansStats.add(
+ RewindPlanStats.create(
+ failedAction,
+ rewindGraph.nodes().size(),
+ lostInputRecordsCount,
+ lostInputRecordsThisAction.subList(
+ 0, Math.min(lostInputRecordsCount, MAX_LOST_INPUTS_RECORDED))));
return new RewindPlan(
Restart.selfAnd(ImmutableGraph.copyOf(rewindGraph)), additionalActionsToRestart.build());
}
- /** Clear the history of failed actions' lost inputs. */
+ /**
+ * Log the top N action rewind events and clear the history of failed actions' lost inputs and
+ * rewind plans.
+ */
void reset(ExtendedEventHandler eventHandler) {
- eventHandler.post(new ActionRewindingStats(lostInputRecords.size()));
+ ImmutableList<ActionRewindEvent> topActionRewindEvents =
+ rewindPlansStats.stream()
+ .collect(
+ greatest(
+ MAX_ACTION_REWIND_EVENTS, comparing(RewindPlanStats::invalidatedNodesCount)))
+ .stream()
+ .map(ActionRewindingStats::toActionRewindEventProto)
+ .collect(toImmutableList());
+ ActionRewindingStats rewindingStats =
+ new ActionRewindingStats(lostInputRecords.size(), topActionRewindEvents);
+ eventHandler.post(rewindingStats);
lostInputRecords = ConcurrentHashMultiset.create();
+ rewindPlansStats = new ConcurrentLinkedQueue<>();
}
- private void checkIfActionLostInputTooManyTimes(
+ /** Returns all lost input records that will cause the failed action to rewind. */
+ private ImmutableList<LostInputRecord> checkIfActionLostInputTooManyTimes(
ActionLookupData actionLookupData,
Action failedAction,
LostInputsActionExecutionException lostInputsException)
throws ActionExecutionException {
ImmutableMap<String, ActionInput> lostInputsByDigest = lostInputsException.getLostInputs();
- for (String digest : lostInputsByDigest.keySet()) {
+ ImmutableList.Builder<LostInputRecord> lostInputRecordsThisAction = ImmutableList.builder();
+ for (Map.Entry<String, ActionInput> entry : lostInputsByDigest.entrySet()) {
// The same action losing the same input more than once is unexpected [*]. The action should
// have waited until the depended-on action which generates the lost input is (re)run before
// trying again.
@@ -164,9 +198,11 @@
// [*], TODO(b/123993876): To mitigate a race condition (believed to be) caused by
// non-topological Skyframe dirtying of depended-on nodes, this check fails the build only if
// the same input is repeatedly lost.
- int priorLosses =
- lostInputRecords.add(
- LostInputRecord.create(actionLookupData, digest), /*occurrences=*/ 1);
+ String digest = entry.getKey();
+ LostInputRecord lostInputRecord =
+ LostInputRecord.create(actionLookupData, digest, entry.getValue().getExecPathString());
+ lostInputRecordsThisAction.add(lostInputRecord);
+ int priorLosses = lostInputRecords.add(lostInputRecord, /*occurrences=*/ 1);
if (MAX_REPEATED_LOST_INPUTS <= priorLosses) {
BugReport.sendBugReport(
new IllegalStateException(
@@ -184,6 +220,7 @@
priorLosses + 1, lostInputsByDigest.get(digest), digest, failedAction));
}
}
+ return lostInputRecordsThisAction.build();
}
private static Set<Artifact.DerivedArtifact> getLostInputOwningDirectDeps(
@@ -322,7 +359,7 @@
for (ActionLookupData actionLookupData : newlyDiscoveredActions) {
Action additionalAction =
- Preconditions.checkNotNull(
+ checkNotNull(
ActionExecutionFunction.getActionForLookupData(env, actionLookupData),
actionLookupData);
additionalActionsToRestart.add(additionalAction);
@@ -463,9 +500,7 @@
Map<ActionLookupData, Action> actions =
Maps.newHashMapWithExpectedSize(actionExecutionDeps.size());
for (ActionLookupData dep : actionExecutionDeps) {
- actions.put(
- dep,
- Preconditions.checkNotNull(ActionExecutionFunction.getActionForLookupData(env, dep)));
+ actions.put(dep, checkNotNull(ActionExecutionFunction.getActionForLookupData(env, dep)));
}
return actions;
}
@@ -501,23 +536,23 @@
private static void assertSkyframeAwareRewindingGraph(
ImmutableGraph<SkyKey> graph, ActionLookupData actionKey) {
- Preconditions.checkArgument(
+ checkArgument(
graph.isDirected(),
"SkyframeAwareAction's rewinding graph is undirected. graph: %s, actionKey: %s",
graph,
actionKey);
- Preconditions.checkArgument(
+ checkArgument(
!graph.allowsSelfLoops(),
"SkyframeAwareAction's rewinding graph allows self loops. graph: %s, actionKey: %s",
graph,
actionKey);
- Preconditions.checkArgument(
+ checkArgument(
graph.nodes().contains(actionKey),
"SkyframeAwareAction's rewinding graph does not contain its action root. graph: %s, "
+ "actionKey: %s",
graph,
actionKey);
- Preconditions.checkArgument(
+ checkArgument(
Iterables.size(Traverser.forGraph(graph).breadthFirst(actionKey)) == graph.nodes().size(),
"SkyframeAwareAction's rewinding graph has nodes unreachable from its action root. "
+ "graph: %s, actionKey: %s",
@@ -526,14 +561,14 @@
for (EndpointPair<SkyKey> edge : graph.edges()) {
SkyKey target = edge.target();
- Preconditions.checkArgument(
+ checkArgument(
!(target instanceof Artifact && ((Artifact) target).isSourceArtifact()),
"SkyframeAwareAction's rewinding graph contains source artifact. graph: %s, "
+ "rootActionNode: %s, sourceArtifact: %s",
graph,
actionKey,
target);
- Preconditions.checkState(
+ checkState(
!(target instanceof Artifact) || target instanceof Artifact.DerivedArtifact,
"A non-source artifact must be derived. graph: %s, rootActionNode: %s, sourceArtifact:"
+ " %s",
@@ -562,6 +597,32 @@
}
/**
+ * A lite version of the RewindPlan that contains the metrics, failed action, and lost inputs.
+ * This object will persist across the build, so it will be more memory efficient than saving the
+ * entire rewind graph for each rewind plan.
+ */
+ @AutoValue
+ abstract static class RewindPlanStats {
+
+ abstract Action failedAction();
+
+ abstract int invalidatedNodesCount();
+
+ abstract int lostInputRecordsCount();
+
+ abstract ImmutableList<LostInputRecord> sampleLostInputRecords();
+
+ static RewindPlanStats create(
+ Action failedAction,
+ int invalidatedNodesCount,
+ int lostInputRecordsCount,
+ ImmutableList<LostInputRecord> sampleLostInputRecords) {
+ return new AutoValue_ActionRewindStrategy_RewindPlanStats(
+ failedAction, invalidatedNodesCount, lostInputRecordsCount, sampleLostInputRecords);
+ }
+ }
+
+ /**
* A record indicating that a Skyframe action execution failed because it lost an input with the
* specified digest.
*/
@@ -572,9 +633,12 @@
abstract String lostInputDigest();
- static LostInputRecord create(ActionLookupData failedActionLookupData, String lostInputDigest) {
+ abstract String lostInputPath();
+
+ static LostInputRecord create(
+ ActionLookupData failedActionLookupData, String lostInputDigest, String lostInputPath) {
return new AutoValue_ActionRewindStrategy_LostInputRecord(
- failedActionLookupData, lostInputDigest);
+ failedActionLookupData, lostInputDigest, lostInputPath);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindingStats.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindingStats.java
index 21347c6..2d306eb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindingStats.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindingStats.java
@@ -14,17 +14,54 @@
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.skyframe.ActionRewindStrategy.RewindPlanStats;
+import com.google.devtools.build.lib.skyframe.proto.ActionRewind.ActionDescription;
+import com.google.devtools.build.lib.skyframe.proto.ActionRewind.ActionRewindEvent;
+import com.google.devtools.build.lib.skyframe.proto.ActionRewind.LostInput;
/** Event that encapsulates data about action rewinding during a build. */
public class ActionRewindingStats implements ExtendedEventHandler.Postable {
private final int lostInputsCount;
+ private final ImmutableList<ActionRewindEvent> actionRewindEvents;
- ActionRewindingStats(int lostInputsCount) {
+ ActionRewindingStats(int lostInputsCount, ImmutableList<ActionRewindEvent> actionRewindEvents) {
this.lostInputsCount = lostInputsCount;
+ this.actionRewindEvents = actionRewindEvents;
}
public int lostInputsCount() {
return lostInputsCount;
}
+
+ public ImmutableList<ActionRewindEvent> actionRewindEvents() {
+ return actionRewindEvents;
+ }
+
+ public static ActionRewindEvent toActionRewindEventProto(RewindPlanStats rewindPlanStats) {
+ ActionOwner failedActionOwner = rewindPlanStats.failedAction().getOwner();
+ return ActionRewindEvent.newBuilder()
+ .setActionDescription(
+ ActionDescription.newBuilder()
+ .setType(rewindPlanStats.failedAction().getMnemonic())
+ .setRuleLabel(
+ failedActionOwner != null ? failedActionOwner.getLabel().toString() : null)
+ .build())
+ .addAllLostInputs(
+ rewindPlanStats.sampleLostInputRecords().stream()
+ .map(
+ lostInputRecord ->
+ LostInput.newBuilder()
+ .setPath(lostInputRecord.lostInputPath())
+ .setDigest(lostInputRecord.lostInputDigest())
+ .build())
+ .collect(toImmutableList()))
+ .setTotalLostInputsCount(rewindPlanStats.lostInputRecordsCount())
+ .setInvalidatedNodesCount(rewindPlanStats.invalidatedNodesCount())
+ .build();
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/proto/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/proto/BUILD
new file mode 100644
index 0000000..6519042
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/proto/BUILD
@@ -0,0 +1,27 @@
+load("//tools/build_rules:utilities.bzl", "java_library_srcs")
+load("@rules_java//java:defs.bzl", "java_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+
+package(default_visibility = ["//src:__pkg__"])
+
+proto_library(
+ name = "action_rewind_event",
+ srcs = ["action_rewind_event.proto"],
+)
+
+java_proto_library(
+ name = "action_rewind_event_java_proto",
+ visibility = ["//src/main/java/com/google/devtools/build/lib:__pkg__"],
+ deps = [":action_rewind_event"],
+)
+
+java_library_srcs(
+ name = "dist_jars",
+ deps = [":action_rewind_event_java_proto"],
+)
+
+filegroup(
+ name = "srcs",
+ srcs = glob(["**"]),
+ visibility = ["//src/main/java/com/google/devtools/build/lib:__pkg__"],
+)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/proto/action_rewind_event.proto b/src/main/java/com/google/devtools/build/lib/skyframe/proto/action_rewind_event.proto
new file mode 100644
index 0000000..f23edbb
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/proto/action_rewind_event.proto
@@ -0,0 +1,51 @@
+// Copyright 2020 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.
+
+syntax = "proto3";
+
+package lib.skyframe;
+
+option java_package = "com.google.devtools.build.lib.skyframe.proto";
+option java_outer_classname = "ActionRewind";
+
+// Information about an action rewind event.
+message ActionRewindEvent {
+ // A description of the action that failed and will rewind.
+ ActionDescription action_description = 1;
+ // A list of lost inputs that caused the action to rewind. This may not
+ // contain all lost inputs that caused this action to rewind.
+ repeated LostInput lost_inputs = 2;
+ // The total number of lost inputs that caused this action to rewind.
+ int32 total_lost_inputs_count = 3;
+ // The total number of nodes or actions that were invalidated during this
+ // action rewind event.
+ int32 invalidated_nodes_count = 4;
+}
+
+// Information about a lost input that caused an action to rewind.
+message LostInput {
+ // The path of the lost input.
+ string path = 1;
+ // The expected digest of the lost input.
+ string digest = 2;
+}
+
+// A description of an action.
+message ActionDescription {
+ // The action mnemonic that describes the type of action executed.
+ string type = 1;
+ // The string notation of the action owner's label if it exists. Following the
+ // pattern of '//<package>:<target>'.
+ string rule_label = 2;
+}
