-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60113/
-----------------------------------------------------------

(Updated Июнь 19, 2017, 9:10 д.п.)


Review request for Ambari, Aravindan Vijayan, Sid Wagle, and Vitalyi Brodetskyi.


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 (updated)
-----

  
ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java
 2972f51 
  
ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java
 42ee366 


Diff: https://reviews.apache.org/r/60113/diff/3/

Changes: https://reviews.apache.org/r/60113/diff/2-3/


Testing
-------

Unit tests passed


Thanks,

Dmytro Sen

Reply via email to