atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hemanth Yamijala <yhema...@gmail.com>
Subject Re: Review Request 48723: ATLAS-901 Log messages that cannot be sent to Kafka to a specific log configuration
Date Thu, 16 Jun 2016 12:30:10 GMT


> On June 16, 2016, 6:58 a.m., Madhan Neethiraj wrote:
> > notification/src/main/java/org/apache/atlas/hook/FailedMessagesLogger.java, line
57
> > <https://reviews.apache.org/r/48723/diff/1/?file=1419538#file1419538line57>
> >
> >     Consider obtaining the logfile name from configuration - similar to flag "atlas.notification.log.failed.messages"
in AtlasHook.java.
> >     
> >     This will enable users to customize the log file location per deployment needs.
The default log file can be in directory System.getProperty("atlas.log.dir").
> 
> Madhan Neethiraj wrote:
>     Since this runs the host component process, "atlas.log.dir" would not be set. Please
ignore this suggestion; instead we can enable failed messages logging only if the log file
location is specified in the configuration - in a property named like "atlas.hook.notification.failed.messages.filename"
> 
> Hemanth Yamijala wrote:
>     I have made the log file name configurable. However, a slight change to your proposal:
I did retain the boolean to check if the logging is necessary or not. My rationale was mainly
to have a default value of true for logging failed messages in the interest of increased resilience.
Having a log file with no default value would have prevented this. And having a default value
for the file would have meant it is logged always (if we use the presence of the value as
an indicator for logging). Please let me know if you feel strongly about having just one configuration.
I will leave the issue open to hear back from you.

Another thing is that it is probably easier for admins to turn on / off logging and retain
the same file name across runs without having to remember. This way it is easier for tools
built to read these messages and replay them to not have to worry about multiple file names
that the user could configure etc.


- Hemanth


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


On June 16, 2016, 12:21 p.m., Hemanth Yamijala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48723/
> -----------------------------------------------------------
> 
> (Updated June 16, 2016, 12:21 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-901
>     https://issues.apache.org/jira/browse/ATLAS-901
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> The description of the approach is documented on JIRA here: https://issues.apache.org/jira/browse/ATLAS-901?focusedCommentId=15331480&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15331480
> 
> 
> Diffs
> -----
> 
>   docs/src/site/twiki/Configuration.twiki 0e122fe 
>   notification/src/main/java/org/apache/atlas/hook/AtlasHook.java 71029b0 
>   notification/src/main/java/org/apache/atlas/hook/FailedMessagesLogger.java PRE-CREATION

>   notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 1ee62d2 
>   notification/src/main/java/org/apache/atlas/notification/NotificationException.java
d9d89df 
>   notification/src/test/java/org/apache/atlas/hook/AtlasHookTest.java 16cb0f0 
>   notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 219bd70

> 
> Diff: https://reviews.apache.org/r/48723/diff/
> 
> 
> Testing
> -------
> 
> Tested by integrating with hive hook. Brought down Kafka when the hook has to send messages.
Ensured that the message is logged into the log file.
> 
> Added some UTs for the changes.
> 
> 
> Thanks,
> 
> Hemanth Yamijala
> 
>


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