-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61665/
-----------------------------------------------------------
(Updated Aug. 17, 2017, 5:04 a.m.)
Review request for atlas, Madhan Neethiraj and Nixon Rodrigues.
Changes
-------
Updates include:
- Additional analysis.
- Implementation details.
Bugs: ATLAS-2047
https://issues.apache.org/jira/browse/ATLAS-2047
Repository: atlas
Description (updated)
-------
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
-----
webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
ef64c3b
webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java
a6f58e8
Diff: https://reviews.apache.org/r/61665/diff/2/
Testing
-------
**Unit tests**
Updated unit tests to reproduce the scenarios and verify the fix.
**Functional tests**
Verified regular notification scenarios.
Thanks,
Ashutosh Mestry
