Automated rollback of commit 8b19650b5817a80fce115b3ede20e92a409eabd6.
*** Reason for rollback ***
After the root cause for b/356561371 has been identified, and the fix unknown commit is in, this blunt mitigation is no longer needed.
Independent of this, investigations and considerations on how to prevent this in the future are still ongoing.
*** Original change description ***
Blunt mitigation for blaze crashing with includes_scanning error: Adding retries for the 2 error modes.
The as yet not understood root cause of this crash must be an unhandled race condition where a *.blaze-grepped_includes_CPP file is read before it is present and fully available.
The two fixes are:
1. Waiting for the file to exist if it doesn't. This wait is very cheap, and it covers the majority of error cases.
2. Retry reading the file should an IOError occur while reading the file. This is...
***
PiperOrigin-RevId: 683997262
Change-Id: I6d1f612b1e7ffa146afec8d517e5f28145f12241
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/BUILD b/src/main/java/com/google/devtools/build/lib/includescanning/BUILD
index 7b51eff..363088d 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/BUILD
@@ -48,7 +48,6 @@
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
- "//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:resource_converter",
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java
index bf06571..c0400b3 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java
@@ -48,8 +48,6 @@
import com.google.devtools.build.lib.skyframe.GlobDescriptor;
import com.google.devtools.build.lib.skyframe.GlobValue;
import com.google.devtools.build.lib.skyframe.InvalidGlobPatternException;
-import com.google.devtools.build.lib.util.JavaSleeper;
-import com.google.devtools.build.lib.util.Sleeper;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -103,7 +101,6 @@
}
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
- private static final Sleeper sleeper = new JavaSleeper();
/**
* Immutable object representation of the four columns making up a single Rule in a Hints set. See
@@ -720,11 +717,7 @@
/** Processes the output generated by an auxiliary include-scanning binary stored in a file. */
static List<Inclusion> processIncludes(Path file) throws IOException {
try {
- // TODO(b/356561371): Remove waitForFileToExist() and retryingReadContent() and reactivate
- // FileSystemUtils.readContent() once the root cause
- // for b/356561371 has been identified and fixed.
- waitForFileToExist(file);
- byte[] data = retryingReadContent(file); // FileSystemUtils.readContent(file);
+ byte[] data = FileSystemUtils.readContent(file);
return IncludeParser.processIncludes(Arrays.asList(new String(data, ISO_8859_1).split("\n")));
} catch (IOException e) {
throw new IOException("Error reading include file " + file + ": " + e.getMessage());
@@ -743,59 +736,6 @@
}
}
- // TODO(b/356561371): Remove this method again once the root cause for b/356561371 has been
- // identified and fixed.
- private static void waitForFileToExist(Path file) throws IOException {
- if (sleeper == null) {
- return;
- }
- if (file.exists()) {
- return;
- }
- int[] waitMillis = {1, 5, 10, 20, 50, 100, 200, 500};
- int waitedMillis = 0;
- for (int i = 0; i < waitMillis.length; i++) {
- try {
- sleeper.sleepMillis(waitMillis[i]);
- waitedMillis += waitMillis[i];
- } catch (InterruptedException e) {
- throw new IOException("Interrupted while waiting for " + file + " to exist", e);
- }
- if (file.exists()) {
- logger.atFine().log("File %s exists after sleeping %s ms", file, waitedMillis);
- return;
- }
- }
- logger.atInfo().log("File %s still missing after waiting %s ms", file, waitedMillis);
- }
-
- // TODO(b/356561371): Remove this method again once the root cause for b/356561371 has been
- // identified and fixed.
- private static byte[] retryingReadContent(Path file) throws IOException {
- int[] waitMillis = {1, 5, 10, 20, 50, 100, 200, 500};
- int waitedMillis = 0;
- if (sleeper != null) {
- for (int i = 0; i < waitMillis.length; i++) {
- try {
- byte[] data = FileSystemUtils.readContent(file);
- logger.atFine().log("File %s readable after sleeping %s ms", file, waitedMillis);
- return data;
- } catch (IOException e) {
- logger.atFine().log("File %s not readable after sleeping %s ms.", file, waitedMillis);
- }
- try {
- sleeper.sleepMillis(waitMillis[i]);
- waitedMillis += waitMillis[i];
- } catch (InterruptedException e) {
- throw new IOException("Interrupted while waiting for " + file + " to be readable", e);
- }
- }
- logger.atInfo().log(
- "File %s not readable after waiting %s ms. Will try once more.", file, waitedMillis);
- }
- return FileSystemUtils.readContent(file);
- }
-
@VisibleForTesting
Inclusion extractInclusion(String line) {
return extractInclusion(line.getBytes(ISO_8859_1), 0, line.length());
