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

(Updated Aug. 31, 2017, 4:23 a.m.)


Review request for atlas, Madhan Neethiraj and Nixon Rodrigues.


Changes
-------

Updates include:
- Addressed review comments.
- Updated unit test.


Bugs: ATLAS-2047
    https://issues.apache.org/jira/browse/ATLAS-2047


Repository: atlas


Description
-------

Please refer to [ATLAS-2047](https://issues.apache.org/jira/browse/ATLAS-2047) 
for background and analysis.

**Background**

The _IllegalStateException_ is thrown by _KafkaConsumer.aquire_. This method is 
called at the beginning of almost every method in this class. The method checks 
if the consumer is closed, if it is then it throws IllegalStateException.

Scenario may come about in this way:
- Shutdown has been initiated. Close on consumer is called.
- However, the consumer thread is just about to enter another poll cycle.
- Thus acquire sees that consumer is closed and throws the exception (2nd 
bullet above).

Please take a look at this scala code. This is _ShutdownableThread_. The thread 
does the job of handling all exceptions. Upon exception, it manages the 
_shutdownLatch_ (from yesterday’s bug fix) and gets out of the _isRunning_ loop.
```scala
  override def run(): Unit = {
    info("Starting ")
    try{
      while(isRunning.get()){
        doWork()
      }
    } catch{
      case e: Throwable =>
        if(isRunning.get())
          error("Error due to ", e)
    }
    shutdownLatch.countDown()
    info("Stopped ")
  }
```

**Implementation**

Special treatment is given to _IllegalStateException_ by implementing pause & 
retry logic:
- Modified _LOG_ to _debug_. That way logs are not filled during retry.
- _HookConsumer_ is more resilient. It handles exceptions resulting from 
_Kafka_ and entity APIs.


Diffs (updated)
-----

  
webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
 ef64c3b 
  webapp/src/test/java/org/apache/atlas/notification/AdaptiveWaiterTest.java 
PRE-CREATION 
  
webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java
 a6f58e8 


Diff: https://reviews.apache.org/r/61665/diff/5/

Changes: https://reviews.apache.org/r/61665/diff/4-5/


Testing
-------

**Unit tests**
Updated unit tests to reproduce the scenarios and verify the fix.

**Functional tests**
Verified regular notification scenarios.


Thanks,

Ashutosh Mestry

Reply via email to