-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60113/
-----------------------------------------------------------
Review request for Ambari, Aravindan Vijayan and Sid Wagle.
Bugs: AMBARI-21248
https://issues.apache.org/jira/browse/AMBARI-21248
Repository: ambari
Description
-------
In org.apache.ambari.server.state.services.AlertNoticeDispatchService, we have
below code snippet:
...
protected void runOneIteration() throws Exception {
…
String targetType = target.getNotificationType();
NotificationDispatcher dispatcher =
m_dispatchFactory.getDispatcher(targetType);
…
if (dispatcher.isDigestSupported()) {
...
{code}
If the database has a change to AlertTargetEntity's targetType column (unlikely
but let's assume it happened), then dispatcher can be null and the subsequent
if clause will throw an exception. In
https://github.com/google/guava/blob/master/guava/src/com/google/common/util/concurrent/AbstractScheduledService.java,
we see that the exception may happen silently since shutDown() and
notifyFailed() does not log anything.
...
{code}
class Task implements Runnable {
@Override
public void run() {
lock.lock();
try {
if (runningTask.isCancelled()) { // task may have been cancelled
while blocked on the lock. return; }
AbstractScheduledService.this.runOneIteration();
} catch (Throwable t) {
try {
shutDown();
} catch (Exception ignored) {
logger.log(Level.WARNING, "Error while attempting to shut
down the service after failure.", ignored);
}
notifyFailed(t);
runningTask.cancel(false); // prevent future invocations.
} finally {
lock.unlock();
}
}
}
}
So, runOneIteration will shutdown the scheduler if it hits an uncaught
exception. We should wrap a try/catch.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java
2972f51
Diff: https://reviews.apache.org/r/60113/diff/1/
Testing
-------
Unit tests passed
Thanks,
Dmytro Sen
