diff options
author | 2015-10-19 20:08:48 +0000 | |
---|---|---|
committer | 2015-10-20 16:36:07 +0000 | |
commit | 98ea68a20ddd58f1ee4bb67410484a1c8794556a (patch) | |
tree | 298fc1ff455e6ba37a34e15781f4f3e2e9e41b1f /src | |
parent | 853cd73b0025fb86b3c042320d824a4eb4447dc4 (diff) |
Don't log SchedulerExceptions.
RELNOTES: None
--
MOS_MIGRATED_REVID=105787681
Diffstat (limited to 'src')
4 files changed, 47 insertions, 19 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java b/src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java index 5fe85bbf65..7a0ab061ac 100644 --- a/src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java +++ b/src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java @@ -648,25 +648,33 @@ public class AbstractQueueVisitor { } } + /** Classification of an error thrown by an action. */ + protected enum ErrorClassification { + // All running actions should be stopped. + CRITICAL, + // Same as CRITICAL, but also log the error. + CRITICAL_AND_LOG, + // Other running actions should be left alone. + NOT_CRITICAL + } + /** - * If this returns true, that means the exception {@code e} is critical - * and all running actions should be stopped. {@link Error}s are always considered critical. + * Classifies {@code e}. {@link Error}s are always classified as {@code CRITICAL_AND_LOG}. * - * <p>Default value - always false. If different behavior is needed + * <p>Default value - always treat errors as {@code NOT_CRITICAL}. If different behavior is needed * then we should override this method in subclasses. * * @param e the exception object to check */ - protected boolean isCriticalError(Throwable e) { - return false; + protected ErrorClassification classifyError(Throwable e) { + return ErrorClassification.NOT_CRITICAL; } - private boolean isCriticalErrorInternal(Throwable e) { - boolean isCritical = isCriticalError(e) || (e instanceof Error); - if (isCritical) { - LOG.log(Level.WARNING, "Found critical error in queue visitor", e); + private ErrorClassification classifyErrorInternal(Throwable e) { + if (e instanceof Error) { + return ErrorClassification.CRITICAL_AND_LOG; } - return isCritical; + return classifyError(e); } /** @@ -674,7 +682,19 @@ public class AbstractQueueVisitor { * to stop all jobs inside {@link #awaitTermination(boolean)}. */ private synchronized void markToStopAllJobsIfNeeded(Throwable e) { - if (isCriticalErrorInternal(e) && !jobsMustBeStopped) { + boolean critical = false; + switch (classifyErrorInternal(e)) { + case CRITICAL_AND_LOG: + critical = true; + LOG.log(Level.WARNING, "Found critical error in queue visitor", e); + break; + case CRITICAL: + critical = true; + break; + default: + break; + } + if (critical && !jobsMustBeStopped) { jobsMustBeStopped = true; synchronized (zeroRemainingTasks) { zeroRemainingTasks.notify(); diff --git a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java index 459c7ffc7d..cd6a986acd 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java +++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java @@ -114,8 +114,10 @@ public abstract class InvalidatingNodeVisitor<TGraph extends ThinNodeQueryableGr } @Override - protected boolean isCriticalError(Throwable e) { - return e instanceof RuntimeException; + protected ErrorClassification classifyError(Throwable e) { + return e instanceof RuntimeException + ? ErrorClassification.CRITICAL_AND_LOG + : ErrorClassification.NOT_CRITICAL; } protected abstract long count(); diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java index 6dc31eeaf6..243577ecc6 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -607,8 +607,14 @@ public final class ParallelEvaluator implements Evaluator { } @Override - protected boolean isCriticalError(Throwable e) { - return e instanceof RuntimeException; + protected ErrorClassification classifyError(Throwable e) { + if (e instanceof SchedulerException) { + return ErrorClassification.CRITICAL; + } + if (e instanceof RuntimeException) { + return ErrorClassification.CRITICAL_AND_LOG; + } + return ErrorClassification.NOT_CRITICAL; } protected void waitForCompletion() throws InterruptedException { diff --git a/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java b/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java index 19b4274267..e7798bcd85 100644 --- a/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java +++ b/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java @@ -533,8 +533,8 @@ public class AbstractQueueVisitorTest { } @Override - protected boolean isCriticalError(Throwable e) { - return true; + protected ErrorClassification classifyError(Throwable e) { + return ErrorClassification.CRITICAL; } } @@ -545,8 +545,8 @@ public class AbstractQueueVisitorTest { } @Override - protected boolean isCriticalError(Throwable e) { - return false; + protected ErrorClassification classifyError(Throwable e) { + return ErrorClassification.NOT_CRITICAL; } } } |