Skip to content

Commit

Permalink
Remove bug report filing for SerializationException.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jin authored and copybara-github committed Oct 2, 2024
1 parent c5d5d5e commit 1656434
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<SerializationException> serializationExceptions = ConcurrentHashMap.newKeySet();

@Subscribe
@AllowConcurrentEvents
Expand Down Expand Up @@ -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();
}
}

0 comments on commit 1656434

Please sign in to comment.