[ 
https://issues.apache.org/jira/browse/FLUME-1782?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13634455#comment-13634455
 ] 

Mike Percy commented on FLUME-1782:
-----------------------------------

Hi Edward,
Thanks for the patch! I have one concern about the approach and a few minor 
requests/questions.

Concern: Since the sink is modifying the event itself, this can cause issues 
with memory channel rollback. i.e. if there is a channel rollback because ES is 
down, then any event processed when it comes back up will actually have the 
timestamp set by a previous run of the process() method. Depending on why you 
might want to have a local timestamp, that could be unexpected and/or desired.

Instead of ensuring that the timestamp is set in the event itself at the sink, 
we could provide some other method of falling back to a local time in the sink 
itself if the event doesn't have a timestamp set.

tl;dr: Don't modify the Event in the sink :)

Other minor feedback:
1. It would be great to post this onto Review Board @ 
https://reviews.apache.org/groups/Flume/ so that individual lines in the patch 
are easy to reference during review
2. In shouldSetAndWarnWhenNoHeaders() I don't think any Event should ever have 
null headers. Have you seen cases where that can happen?
3. Nit: can we use a constant for the values 1355364900011 and 1355364900079 
that are used throughout this test? They are kinda magic numbers and it could 
use a little more explanation and/or ensuring they are used consistently by 
making them private static final variables. :)
4. Nit: consider using Charsets.UTF_8 from Guava rather than 
Charset.forName("UTF-8") ... this is optional and not a big deal.

Best,
Mike

                
> Elastic Search sink does not use UTC to determine the correct index to write 
> to.
> --------------------------------------------------------------------------------
>
>                 Key: FLUME-1782
>                 URL: https://issues.apache.org/jira/browse/FLUME-1782
>             Project: Flume
>          Issue Type: Bug
>          Components: Sinks+Sources
>            Reporter: Edward Sargisson
>             Fix For: v1.4.0
>
>         Attachments: FLUME-1782-1.4.0-1.patch, FLUME-1782-1.4.0-2.patch, 
> FLUME-1782-1.4.0.patch, flume-1782-2.patch, flume-1782-3.patch, 
> flume-1782.patch
>
>
> The GUI for logs in ElasticSearch, Kibana, uses the utc date to determine 
> which index to read for a search. The Flume ElasticSearch sink is using the 
> local timezone to determine which index to write to. This means that events 
> are being placed in the incorrect index and Kibana doesn't always find them.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to