Incorporate skymeld inconsistencies into the rewinding/node dropping …

…receivers.

Current state: blaze in skymeld + rewinding just uses the `RewindableGraphInconsistencyReceiver`, which doesn't take into account skymeld inconsistencies and is mutually exclusive with `SkymeldInconsistencyReceiver`.

After this CL: `RewindableGraphInconsistencyReceiver` and `NodeDroppingInconsistencyReceiver` handles skymeld-specific inconsistencies as well.

Code changes:

- `SkymeldInconsistencyReceiver` is removed and its content is distributed to the rewinding/node dropping inconsistency receivers.

Behavior changes:

- `RewindableGraphInconsistencyReceiver` and `NodeDroppingInconsistencyReceiver` handles skymeld-specific inconsistencies as well.
- Unexpected skymeld inconsistencies will throw, instead of sending a BugReport like before.

---
Since bazelbuild@5b52f01, rewinding is already compatible with Skymeld.

PiperOrigin-RevId: 631332866
Change-Id: I3d222c23d5e40aa7a86546039bfb795f7b812c2c

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

@@ -187,7 +187,6 @@ java_library(
         ":skyframe_executor_repository_helpers_holder",
         ":skyframe_incremental_build_monitor",
         ":skyframe_stats",
-        ":skymeld_inconsistency_receiver",
         ":starlark_builtins_value",
         ":state_informing_sky_function_environment",
         ":target_completion_value",
@@ -3193,21 +3192,6 @@ java_library(
     ],
 )
 
-java_library(
-    name = "skymeld_inconsistency_receiver",
-    srcs = ["SkymeldInconsistencyReceiver.java"],
-    deps = [
-        ":node_dropping_inconsistency_receiver",
-        ":sky_functions",
-        "//src/main/java/com/google/devtools/build/lib/bugreport",
-        "//src/main/java/com/google/devtools/build/skyframe",
-        "//src/main/java/com/google/devtools/build/skyframe:graph_inconsistency_java_proto",
-        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
-        "//third_party:guava",
-        "//third_party:jsr305",
-    ],
-)
-
 java_library(
     name = "sky_key_stats",
     srcs = ["SkyKeyStats.java"],

src/main/java/com/google/devtools/build/lib/skyframe/NodeDroppingInconsistencyReceiver.java

@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
-import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.FileValue;
@@ -28,31 +27,51 @@
 
 /**
  * {@link GraphInconsistencyReceiver} for evaluations operating on graphs when {@code
- * --heuristically_drop_nodes} flag is applied.
+ * --heuristically_drop_nodes} flag is applied, or when some form of node dropping is done in
+ * combination with skymeld mode.
  *
- * <p>The expected inconsistency caused by heuristically dropping state nodes should be tolerated
- * while all other inconsistencies should result in throwing an exception.
+ * <p>The expected inconsistency should be tolerated while all other inconsistencies should result
+ * in throwing an exception.
  *
  * <p>{@code RewindableGraphInconsistencyReceiver} implements similar logic to handle heuristically
  * dropping state nodes.
  */
-public class NodeDroppingInconsistencyReceiver implements GraphInconsistencyReceiver {
+public final class NodeDroppingInconsistencyReceiver implements GraphInconsistencyReceiver {
 
+  private final boolean heuristicallyDropNodes;
+  private final boolean skymeldInconsistenciesExpected;
   private static final ImmutableMap<SkyFunctionName, SkyFunctionName> EXPECTED_MISSING_CHILDREN =
       ImmutableMap.of(
           FileValue.FILE, FileStateKey.FILE_STATE,
           SkyFunctions.DIRECTORY_LISTING, SkyFunctions.DIRECTORY_LISTING_STATE,
           SkyFunctions.CONFIGURED_TARGET, GenQueryDirectPackageProviderFactory.GENQUERY_SCOPE);
 
+  // TODO: b/290998109#comment60 - After the GLOB nodes are replaced by GLOBS, the missing children
+  // below might be unexpected.
+  // These are only expected when Skymeld is enabled and we're dropping nodes.
+  private static final ImmutableMap<SkyFunctionName, SkyFunctionName>
+      SKYMELD_EXPECTED_MISSING_CHILDREN =
+          ImmutableMap.of(SkyFunctions.ACTION_EXECUTION, SkyFunctions.GLOB);
+
+  public NodeDroppingInconsistencyReceiver(
+      boolean heuristicallyDropNodes, boolean skymeldInconsistenciesExpected) {
+    this.heuristicallyDropNodes = heuristicallyDropNodes;
+    this.skymeldInconsistenciesExpected = skymeldInconsistenciesExpected;
+  }
+
   @Override
   public void noteInconsistencyAndMaybeThrow(
       SkyKey key, @Nullable Collection<SkyKey> otherKeys, Inconsistency inconsistency) {
-    checkState(
-        isExpectedInconsistency(key, otherKeys, inconsistency),
-        "Unexpected inconsistency: %s, %s, %s",
-        key,
-        otherKeys,
-        inconsistency);
+    if (heuristicallyDropNodes && isExpectedInconsistency(key, otherKeys, inconsistency)) {
+      return;
+    }
+    if (skymeldInconsistenciesExpected
+        && isExpectedInconsistencySkymeld(key, otherKeys, inconsistency)) {
+      return;
+    }
+
+    throw new IllegalStateException(
+        String.format("Unexpected inconsistency: %s, %s, %s", key, otherKeys, inconsistency));
   }
 
   /**
@@ -61,19 +80,40 @@ public void noteInconsistencyAndMaybeThrow(
    */
   public static boolean isExpectedInconsistency(
       SkyKey key, @Nullable Collection<SkyKey> otherKeys, Inconsistency inconsistency) {
-    return isExpectedInconsistency(key, otherKeys, inconsistency, EXPECTED_MISSING_CHILDREN);
+    return isExpectedInternal(
+        key,
+        otherKeys,
+        inconsistency,
+        EXPECTED_MISSING_CHILDREN,
+        /* isSkymeldInconsistency= */ false);
+  }
+
+  /**
+   * Checks whether the input inconsistency is an expected scenario caused by skymeld + some form of
+   * node dropping.
+   */
+  public static boolean isExpectedInconsistencySkymeld(
+      SkyKey key, @Nullable Collection<SkyKey> otherKeys, Inconsistency inconsistency) {
+    return isExpectedInternal(
+        key,
+        otherKeys,
+        inconsistency,
+        SKYMELD_EXPECTED_MISSING_CHILDREN,
+        /* isSkymeldInconsistency= */ true);
   }
 
-  static boolean isExpectedInconsistency(
+  private static boolean isExpectedInternal(
       SkyKey key,
       @Nullable Collection<SkyKey> otherKeys,
       Inconsistency inconsistency,
-      ImmutableMap<SkyFunctionName, SkyFunctionName> expectedMissingChildrenTypes) {
-    SkyFunctionName expectedMissingChildType = expectedMissingChildrenTypes.get(key.functionName());
+      ImmutableMap<SkyFunctionName, SkyFunctionName> expectedMissingChildTypes,
+      boolean isSkymeldInconsistency) {
+    SkyFunctionName expectedMissingChildType = expectedMissingChildTypes.get(key.functionName());
     if (expectedMissingChildType == null) {
       return false;
     }
-    if (inconsistency == Inconsistency.RESET_REQUESTED) {
+    // Skymeld shouldn't cause any inconsistency of this type.
+    if (!isSkymeldInconsistency && inconsistency == Inconsistency.RESET_REQUESTED) {
       return otherKeys == null;
     }
     if (inconsistency == Inconsistency.ALREADY_DECLARED_CHILD_MISSING

src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java

@@ -298,28 +298,27 @@ public WorkspaceInfoFromDiff sync(
   }
 
   private GraphInconsistencyReceiver getGraphInconsistencyReceiverForCommand(
-      OptionsProvider options) throws AbruptExitException {
-    if (rewindingEnabled(options)) {
-      // Currently incompatible with Skymeld i.e. this code path won't be run in Skymeld mode. We
-      // may need to combine these GraphInconsistencyReceiver implementations in the future.
-      var rewindableReceiver = new RewindableGraphInconsistencyReceiver();
-      rewindableReceiver.setHeuristicallyDropNodes(heuristicallyDropNodes);
-      return rewindableReceiver;
-    }
-    if (isMergedSkyframeAnalysisExecution()
-        && ((options.getOptions(AnalysisOptions.class) != null
+      OptionsProvider options) {
+    var someNodeDroppingExpected =
+        (options.getOptions(AnalysisOptions.class) != null
                 && options.getOptions(AnalysisOptions.class).discardAnalysisCache)
             || !trackIncrementalState
-            || heuristicallyDropNodes)) {
-      return new SkymeldInconsistencyReceiver(heuristicallyDropNodes);
+            || heuristicallyDropNodes;
+    var skymeldInconsistenciesExpected =
+        someNodeDroppingExpected && isMergedSkyframeAnalysisExecution();
+    if (rewindingEnabled(options)) {
+      return new RewindableGraphInconsistencyReceiver(
+          heuristicallyDropNodes, skymeldInconsistenciesExpected);
     }
-    if (heuristicallyDropNodes) {
-      return new NodeDroppingInconsistencyReceiver();
+
+    if (heuristicallyDropNodes || skymeldInconsistenciesExpected) {
+      return new NodeDroppingInconsistencyReceiver(
+          heuristicallyDropNodes, skymeldInconsistenciesExpected);
     }
     return GraphInconsistencyReceiver.THROWING;
   }
 
-  private boolean rewindingEnabled(OptionsProvider options) throws AbruptExitException {
+  private boolean rewindingEnabled(OptionsProvider options) {
     var buildRequestOptions = options.getOptions(BuildRequestOptions.class);
     return buildRequestOptions != null && buildRequestOptions.rewindLostInputs;
   }

src/main/java/com/google/devtools/build/lib/skyframe/rewinding/RewindableGraphInconsistencyReceiver.java

@@ -52,10 +52,13 @@ public final class RewindableGraphInconsistencyReceiver implements GraphInconsis
   private final Multiset<Inconsistency> selfCounts = ConcurrentHashMultiset.create();
   private final Multiset<Inconsistency> childCounts = ConcurrentHashMultiset.create();
   private boolean rewindingInitiated = false;
-  private boolean heuristicallyDropNodes = false;
+  private final boolean heuristicallyDropNodes;
+  private final boolean skymeldInconsistenciesExpected;
 
-  public void setHeuristicallyDropNodes(boolean heuristicallyDropNodes) {
+  public RewindableGraphInconsistencyReceiver(
+      boolean heuristicallyDropNodes, boolean skymeldInconsistenciesExpected) {
     this.heuristicallyDropNodes = heuristicallyDropNodes;
+    this.skymeldInconsistenciesExpected = skymeldInconsistenciesExpected;
   }
 
   @Override
@@ -69,6 +72,12 @@ public void noteInconsistencyAndMaybeThrow(
       return;
     }
 
+    if (skymeldInconsistenciesExpected
+        && NodeDroppingInconsistencyReceiver.isExpectedInconsistencySkymeld(
+            key, otherKeys, inconsistency)) {
+      return;
+    }
+
     // RESET_REQUESTED and PARENT_FORCE_REBUILD_OF_CHILD may be the first inconsistencies seen with
     // rewinding. BUILDING_PARENT_FOUND_UNDONE_CHILD may also be seen, but it will not be the first.
     switch (inconsistency) {