From 1656434ea10ee3f05e895065661e450525ca7a91 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 2 Oct 2024 09:07:51 -0700 Subject: [PATCH] Remove bug report filing for SerializationException. If something pathlogical happens to the serialization stack and causes a lot of SerializationExceptions, it can: 1) File a lot of bug reports, or 2) Make SkyFunctions lock on the static loggers used in the BugReport implementation, causing contention. This can be improved somewhat, but let's remove the suboptimal code path for now. PiperOrigin-RevId: 681475746 Change-Id: Ie1ee4f17a8d6fb521e20cfb79daccc6228b46da3 --- .../devtools/build/lib/buildtool/BuildTool.java | 5 +++++ .../com/google/devtools/build/lib/skyframe/BUILD | 1 - .../build/lib/skyframe/SkyValueRetrieverUtils.java | 3 +-- .../RemoteAnalysisCachingDependenciesProvider.java | 7 +++++++ .../RemoteAnalysisCachingEventListener.java | 14 ++++++++++++++ 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index 27400152ec248d..7cb5062045baf2 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java @@ -1207,6 +1207,11 @@ public void recordRetrievalResult(RetrievalResult retrievalResult, SkyKey key) { listener.recordRetrievalResult(retrievalResult, key); } + @Override + public void recordSerializationException(SerializationException e) { + listener.recordSerializationException(e); + } + @Override public void setTopLevelConfig(BuildConfigurationValue topLevelConfig) { this.topLevelConfig = topLevelConfig; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 832e59a34a7183..0bc7f4a3ff1fa9 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -679,7 +679,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", - "//src/main/java/com/google/devtools/build/lib/bugreport", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:dependencies_provider", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueRetrieverUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueRetrieverUtils.java index 5bc4053d4837ce..b061da645c2708 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueRetrieverUtils.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyValueRetrieverUtils.java @@ -18,7 +18,6 @@ import com.google.devtools.build.lib.actions.ActionLookupData; import com.google.devtools.build.lib.actions.ActionLookupKey; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.skyframe.serialization.SerializationException; import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever; @@ -66,7 +65,7 @@ public static RetrievalResult maybeFetchSkyValueRemotely( /* frontierNodeVersion= */ analysisCachingDeps.getSkyValueVersion()); } catch (SerializationException e) { // Don't crash the build if deserialization failed. Gracefully fallback to local evaluation. - BugReport.sendBugReport(e); + analysisCachingDeps.recordSerializationException(e); return NO_CACHED_DATA; } analysisCachingDeps.recordRetrievalResult(retrievalResult, key); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCachingDependenciesProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCachingDependenciesProvider.java index 49ddf37171a8c8..6728a994f9d1ea 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCachingDependenciesProvider.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCachingDependenciesProvider.java @@ -53,6 +53,8 @@ default boolean enabled() { void recordRetrievalResult(RetrievalResult retrievalResult, SkyKey key); + void recordSerializationException(SerializationException e); + void setTopLevelConfig(BuildConfigurationValue topLevelConfig); /** A stub dependencies provider for when analysis caching is disabled. */ @@ -92,6 +94,11 @@ public void recordRetrievalResult(RetrievalResult retrievalResult, SkyKey key) { throw new UnsupportedOperationException(); } + @Override + public void recordSerializationException(SerializationException e) { + throw new UnsupportedOperationException(); + } + @Override public void setTopLevelConfig(BuildConfigurationValue topLevelConfig) { throw new UnsupportedOperationException(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCachingEventListener.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCachingEventListener.java index 1b06aa06e802e3..2d143f85b7796d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCachingEventListener.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/RemoteAnalysisCachingEventListener.java @@ -21,6 +21,7 @@ import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.concurrent.ThreadSafety; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.skyframe.serialization.SerializationException; import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.NoCachedData; import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.Restart; import com.google.devtools.build.lib.skyframe.serialization.SkyValueRetriever.RetrievalResult; @@ -52,6 +53,7 @@ public record SerializedNodeEvent(SkyKey key) { private final AtomicInteger analysisCacheMisses = new AtomicInteger(); private final AtomicInteger executionCacheHits = new AtomicInteger(); private final AtomicInteger executionCacheMisses = new AtomicInteger(); + private final Set serializationExceptions = ConcurrentHashMap.newKeySet(); @Subscribe @AllowConcurrentEvents @@ -136,4 +138,16 @@ public int getExecutionNodeCacheHits() { public int getExecutionNodeCacheMisses() { return executionCacheMisses.get(); } + + /** Records a {@link SerializationException} encountered during SkyValue retrievals. */ + public void recordSerializationException(SerializationException e) { + serializationExceptions.add(e); + } + + /** + * Returns the number of {@link SerializationException}s that were thrown during this invocation. + */ + public int getSerializationExceptionCounts() { + return serializationExceptions.size(); + } }