[8.8.0] Don't fail an action due to spawn log exceptions (#30717)
Instead, log the exception (at most once per spawn) and show a warning
at the end of the build.
Speculatively fixes #22920.
Closes #27313.
PiperOrigin-RevId: 825577891
Change-Id: I91d057fca01b0d1f1413ea1c3210bf4a392ebc55
(cherry picked from commit ceaff2dcc7c213b0eaac82a35903075ab71fe089)
8.8.0 adaptation: the release branch has no `internalToUnicode`
conversion for exec paths and mnemonics, passes the real
`inputMetadataProvider` to `logFile`/`logDirectory`, and
`logUnresolvedSymlink` takes only two arguments; the new output-logging
loop was adapted to those signatures. The test-only reformatting hunks
in `SpawnLogContextTestBase` were dropped as they don't apply to the
branch.
Closes #30715
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java b/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java
index f62c175..f7471db 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java
@@ -112,7 +112,8 @@
env.getOptions().getOptions(RemoteOptions.class),
env.getRuntime().getFileSystem().getDigestFunction(),
env.getXattrProvider(),
- env.getCommandId());
+ env.getCommandId(),
+ env.getReporter());
} catch (InterruptedException e) {
env.getReporter()
.handle(Event.error("Error while setting up the execution log: " + e.getMessage()));
diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD
index 156a5ae..b3f0f1c 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD
@@ -284,6 +284,7 @@
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/concurrent",
+ "//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/remote/util:digest_utils",
diff --git a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
index 5641ee4..c49db49 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
@@ -21,6 +21,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
+import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
@@ -38,6 +39,8 @@
import com.google.devtools.build.lib.concurrent.AbstractQueueVisitor;
import com.google.devtools.build.lib.concurrent.ErrorClassifier;
import com.google.devtools.build.lib.concurrent.NamedForkJoinPool;
+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.Protos.ExecLogEntry;
import com.google.devtools.build.lib.exec.Protos.Platform;
@@ -66,12 +69,15 @@
import java.util.SortedMap;
import java.util.UUID;
import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;
import javax.annotation.concurrent.GuardedBy;
/** A {@link SpawnLogContext} implementation that produces a log in compact format. */
public class CompactSpawnLogContext extends SpawnLogContext {
+ private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
+
private static final Comparator<ExecLogEntry.File> EXEC_LOG_ENTRY_FILE_COMPARATOR =
Comparator.comparing(ExecLogEntry.File::getPath);
@@ -146,6 +152,8 @@
private final DigestHashFunction digestHashFunction;
private final XattrProvider xattrProvider;
private final UUID invocationId;
+ private final ExtendedEventHandler reporter;
+ private final AtomicBoolean outputLoggingFailed = new AtomicBoolean(false);
// Maps a key identifying an entry into its ID.
// Each key is either a NestedSet.Node or the String path of a file, directory, symlink or
@@ -170,7 +178,8 @@
@Nullable RemoteOptions remoteOptions,
DigestHashFunction digestHashFunction,
XattrProvider xattrProvider,
- UUID invocationId)
+ UUID invocationId,
+ ExtendedEventHandler reporter)
throws IOException, InterruptedException {
this.execRoot = execRoot;
this.workspaceName = workspaceName;
@@ -179,6 +188,7 @@
this.digestHashFunction = digestHashFunction;
this.xattrProvider = xattrProvider;
this.invocationId = invocationId;
+ this.reporter = reporter;
this.outputStream = getOutputStream(outputPath);
logInvocation();
@@ -236,19 +246,32 @@
}
builder.setMnemonic(spawn.getMnemonic());
+ boolean warned = false;
for (ActionInput output : spawn.getOutputFiles()) {
- Path path = fileSystem.getPath(execRoot.getRelative(output.getExecPath()));
- if (!output.isDirectory() && !output.isSymlink() && path.isFile()) {
- builder.addOutputsBuilder().setOutputId(logFile(output, path, inputMetadataProvider));
- } else if (output.isDirectory() && path.isDirectory()) {
- builder
- .addOutputsBuilder()
- .setOutputId(logDirectory(output, path, inputMetadataProvider));
- } else if (output.isSymlink() && path.isSymbolicLink()) {
- builder.addOutputsBuilder().setOutputId(logUnresolvedSymlink(output, path));
- } else {
- builder.addOutputsBuilder().setInvalidOutputPath(output.getExecPathString());
+ var path = fileSystem.getPath(execRoot.getRelative(output.getExecPath()));
+ var outputBuilder = ExecLogEntry.Output.newBuilder();
+ try {
+ if (!output.isDirectory() && !output.isSymlink() && path.isFile()) {
+ outputBuilder.setOutputId(logFile(output, path, inputMetadataProvider));
+ } else if (output.isDirectory() && path.isDirectory()) {
+ outputBuilder.setOutputId(logDirectory(output, path, inputMetadataProvider));
+ } else if (output.isSymlink() && path.isSymbolicLink()) {
+ outputBuilder.setOutputId(logUnresolvedSymlink(output, path));
+ } else {
+ outputBuilder.setInvalidOutputPath(output.getExecPathString());
+ }
+ } catch (IOException e) {
+ if (!warned) {
+ outputLoggingFailed.set(true);
+ warned = true;
+ logger.atInfo().withCause(e).log(
+ "Failed to log outputs of spawn with mnemonic %s and primary output %s",
+ spawn.getMnemonic(),
+ Iterables.getFirst(spawn.getOutputFiles(), /* not reached */ null));
+ }
+ outputBuilder.setInvalidOutputPath(output.getExecPathString());
}
+ builder.addOutputs(outputBuilder);
}
builder.setExitCode(result.exitCode());
@@ -727,6 +750,12 @@
@Override
public void close() throws IOException {
+ if (outputLoggingFailed.get()) {
+ reporter.handle(
+ Event.warn(
+ "The compact execution log is incomplete because some outputs could not be read."
+ + " Refer to the server log file for details."));
+ }
outputStream.close();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD
index 0cb9810..b086aad 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD
@@ -97,6 +97,7 @@
"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//src/test/java/com/google/devtools/build/lib/exec/util",
"//src/test/java/com/google/devtools/build/lib/testutil",
+ "//src/test/java/com/google/devtools/build/lib/testutil:JunitUtils",
"//src/test/java/com/google/devtools/build/lib/testutil:TestConstants",
"//third_party:guava",
"//third_party:jsr305",
diff --git a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java
index 7eb6bd1..ce2881f 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.exec;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertContainsEvent;
import static com.google.devtools.build.lib.testutil.TestConstants.PRODUCT_NAME;
import static com.google.devtools.build.lib.testutil.TestConstants.WORKSPACE_NAME;
@@ -239,6 +240,54 @@
.build());
}
+ @Test
+ public void testUnreadableOutputs(@TestParameter OutputsMode outputsMode) throws Exception {
+ Artifact readableFile = ActionsTestUtil.createArtifact(outputDir, "readable");
+ Artifact unreadableFile = ActionsTestUtil.createArtifact(outputDir, "unreadable");
+ Artifact unreadableFileDir =
+ ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputDir, "unreadableFileDir");
+
+ writeFile(readableFile.getPath(), "xyz");
+ // Make the files unreadable.
+ writeFile(unreadableFile.getPath(), "abc");
+ unreadableFile.getPath().setReadable(false);
+ writeFile(unreadableFileDir.getPath().getChild("file"), "def");
+ unreadableFileDir.getPath().getChild("file").setReadable(false);
+
+ SpawnBuilder spawn =
+ defaultSpawnBuilder().withOutputs(readableFile, unreadableFile, unreadableFileDir);
+
+ SpawnLogContext context = createSpawnLogContext();
+
+ context.logSpawn(
+ spawn.build(),
+ createInputMetadataProvider(),
+ createInputMap(),
+ outputsMode.getActionFileSystem(fs),
+ defaultTimeout(),
+ defaultSpawnResult());
+
+ closeAndAssertLog(
+ context,
+ defaultSpawnExecBuilder()
+ .addActualOutputs(
+ File.newBuilder()
+ .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/readable")
+ .setDigest(getDigest("xyz"))
+ .setIsTool(false))
+ .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/readable")
+ .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/unreadable")
+ .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/unreadableFileDir")
+ .build());
+
+ assertContainsEvent(
+ storedEventHandler.getEvents(),
+ "The compact execution log is incomplete because some outputs could not be read. Refer"
+ + " to the server log file for details.");
+ assertThat(storedEventHandler.getEvents()).hasSize(1);
+ assertThat(storedEventHandler.getPosts()).isEmpty();
+ }
+
@Override
protected SpawnLogContext createSpawnLogContext(ImmutableMap<String, String> platformProperties)
throws IOException, InterruptedException {
@@ -253,7 +302,8 @@
remoteOptions,
DigestHashFunction.SHA256,
SyscallCache.NO_CACHE,
- UUID.fromString("00000000-0000-0000-0000-000000000000"));
+ UUID.fromString("00000000-0000-0000-0000-000000000000"),
+ storedEventHandler);
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
index 6150808..d6cfaa6 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
@@ -63,6 +63,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.exec.Protos.Digest;
import com.google.devtools.build.lib.exec.Protos.EnvironmentVariable;
import com.google.devtools.build.lib.exec.Protos.File;
@@ -117,6 +118,7 @@
protected ArtifactRoot externalSourceRoot;
protected ArtifactRoot externalOutputDir;
protected BuildConfigurationValue configuration;
+ protected StoredEventHandler storedEventHandler;
@TestParameter public boolean siblingRepositoryLayout;
@@ -161,6 +163,7 @@
ArtifactRoot.asExternalSourceRoot(
Root.fromPath(externalRoot.getChild(externalRepo.getName())));
externalOutputDir = configuration.getBinDirectory(externalRepo);
+ storedEventHandler = new StoredEventHandler();
}
// A fake action filesystem that provides a fast digest, but refuses to compute it from the
