----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61665/ -----------------------------------------------------------
(Updated Aug. 29, 2017, 10:37 p.m.) Review request for atlas, Madhan Neethiraj and Nixon Rodrigues. Changes ------- Updates include: - Addressed review comments. - Updated unit tests. 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/4/ Changes: https://reviews.apache.org/r/61665/diff/3-4/ Testing ------- **Unit tests** Updated unit tests to reproduce the scenarios and verify the fix. **Functional tests** Verified regular notification scenarios. Thanks, Ashutosh Mestry