atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ashutosh Mestry <ames...@hortonworks.com>
Subject Re: Review Request 61665: ATLAS-2047: Exception Thrown by Kafka Consumer Ends up Filling Logs Due to Incorrect Handling
Date Wed, 16 Aug 2017 22:45:44 GMT


> On Aug. 16, 2017, 9:59 a.m., Nixon Rodrigues wrote:
> > Ship It!

Can you please review? I have updated the patch with retry logic.


- Ashutosh


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


On Aug. 15, 2017, 4:58 p.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61665/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2017, 4:58 p.m.)
> 
> 
> Review request for atlas, Madhan Neethiraj and Nixon Rodrigues.
> 
> 
> 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.
> 
> **Implementation**
> 
> 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 ")
>   }
> ```
> 
> Moved _try...catch_ block to leave exception handling to _ShutdownableThread_.
> 
> 
> 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/1/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> Updated unit tests to reproduce the scenarios and verify the fix.
> 
> **Functional tests**
> Verified regular notification scenarios.
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message