Handle CancellationExceptions in BES module instead of crashing.
Each BEP transport has a close future that we wrap with a Futures.withTimeout in order to handle timeouts. TiemoutFutures sometimes (1/100) throw CancellationException when the timeout is reached so we're currently crashing when that happens causing flakiness in our tests. With this change the flakiness due to this problem is removed.
Catching CancellationException seems weird to me since it's an unchecked exception but after spending quite a bit of time debugging this I couldn't quite figure out why Futures.withTimeout doesn't consistently throw a TimeoutException when timing out so I went with this solution.
PiperOrigin-RevId: 246537642
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
index 145848c..ce4d4d2 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
@@ -67,6 +67,7 @@
import java.time.Duration;
import java.time.Instant;
import java.util.Set;
+import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
@@ -178,6 +179,13 @@
Futures.allAsList(futureMap.values()),
getMaxWaitForPreviousInvocation().getSeconds(),
TimeUnit.SECONDS);
+ } catch (CancellationException e) {
+ String msg =
+ "Previous invocation failed to finish Build Event Protocol upload. "
+ + "The upload was cancelled. "
+ + "Ignoring the failure and starting a new invocation...";
+ cmdLineReporter.handle(Event.warn(msg));
+ googleLogger.atWarning().withCause(e).log(msg);
} catch (TimeoutException exception) {
String msg =
String.format(
@@ -301,7 +309,7 @@
// passed. We should fix this by waiting at most the value set by bes_timeout.
googleLogger.atInfo().log("Closing pending build event transports");
Uninterruptibles.getUninterruptibly(Futures.allAsList(closeFuturesMap.values()));
- } catch (ExecutionException e) {
+ } catch (CancellationException | ExecutionException e) {
googleLogger.atSevere().withCause(e).log("Failed to close a build event transport");
LoggingUtil.logToRemote(Level.SEVERE, "Failed to close a build event transport", e);
} finally {
@@ -328,7 +336,7 @@
Futures.allAsList(closeFuturesMap.values()),
getMaxWaitForPreviousInvocation().getSeconds(),
TimeUnit.SECONDS);
- } catch (TimeoutException | ExecutionException exception) {
+ } catch (CancellationException | TimeoutException | ExecutionException exception) {
googleLogger.atWarning().withCause(exception).log(
"Encountered Exception when closing BEP transports in Blaze's shutting down sequence");
} finally {
@@ -378,6 +386,11 @@
// passed. We should fix this by waiting at most the value set by bes_timeout.
Uninterruptibles.getUninterruptibly(Futures.allAsList(closeFuturesMap.values()));
}
+ } catch (CancellationException e) {
+ throw new AbruptExitException(
+ "The Build Event Protocol upload was cancelled",
+ ExitCode.TRANSIENT_BUILD_EVENT_SERVICE_UPLOAD_ERROR,
+ e);
} catch (ExecutionException e) {
// Futures.withTimeout wraps the TimeoutException in an ExecutionException when the future
// times out.
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java
index e3c8e80..66ce615 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java
@@ -739,3 +739,4 @@
}
}
}
+
