You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Edward Sargisson <es...@pobox.com> on 2013/04/09 19:59:29 UTC
Review Request: FLUME-1782 Elastic Search sink does not use UTC to determine
the correct index to write to
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/
-----------------------------------------------------------
Review request for Flume and Hari Shreedharan.
Description
-------
This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
The previous code would use the current time and would do it in the timezone of the Flume agent's host.
This addresses bug FLUME-1782.
https://issues.apache.org/jira/browse/FLUME-1782
Diffs
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10379/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Edward Sargisson <es...@pobox.com>.
> On April 11, 2013, 10:28 a.m., Israel Ekpo wrote:
> > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java, line 248
> > <https://reviews.apache.org/r/10379/diff/1/?file=279376#file279376line248>
> >
> > Since you are retrieving the headers twice within the method, you could retrieve it once and store it in a final variable for use later.
> >
> > Also perform null checks before attempting to use the variable.
> >
> > There may be cases where the event has no headers.
Hi Mr Ekpo,
The current structure of the code, where ensureHasTimestamp will create the headers map if required, means that this idea probably won't work as nicely as it sounds. We'd end up having to do the "create map if missing code" in the process method itself - which seems odd.
In my judgment, the small cost of an additional lookup on the object is worth it compared to the memory cost of hanging onto an additional temporary reference. I'd be incredibly surprised if any performance difference were found - and I could argue that the additional temporary reference version is slower because it would leave more garbage requiring longer collection time.
- Edward
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/#review18998
-----------------------------------------------------------
On April 9, 2013, 5:59 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10379/
> -----------------------------------------------------------
>
> (Updated April 9, 2013, 5:59 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
> The previous code would use the current time and would do it in the timezone of the Flume agent's host.
>
>
> This addresses bug FLUME-1782.
> https://issues.apache.org/jira/browse/FLUME-1782
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10379/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/#review18998
-----------------------------------------------------------
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java
<https://reviews.apache.org/r/10379/#comment39479>
Since you are retrieving the headers twice within the method, you could retrieve it once and store it in a final variable for use later.
Also perform null checks before attempting to use the variable.
There may be cases where the event has no headers.
- Israel Ekpo
On April 9, 2013, 5:59 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10379/
> -----------------------------------------------------------
>
> (Updated April 9, 2013, 5:59 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
> The previous code would use the current time and would do it in the timezone of the Flume agent's host.
>
>
> This addresses bug FLUME-1782.
> https://issues.apache.org/jira/browse/FLUME-1782
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10379/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/#review19358
-----------------------------------------------------------
Edward,
Here are some comments Mike Percy left on the JIRA issue:
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
- Israel Ekpo
On April 11, 2013, 7:03 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10379/
> -----------------------------------------------------------
>
> (Updated April 11, 2013, 7:03 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
> The previous code would use the current time and would do it in the timezone of the Flume agent's host.
>
>
> This addresses bug FLUME-1782.
> https://issues.apache.org/jira/browse/FLUME-1782
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10379/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Edward Sargisson <es...@pobox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/
-----------------------------------------------------------
(Updated April 22, 2013, 8:47 p.m.)
Review request for Flume and Hari Shreedharan.
Changes
-------
Updated patch with new interface as requested. Incorporated flume-1972 patch in order to resolve the merge conflicts.
Description
-------
This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
The previous code would use the current time and would do it in the timezone of the Flume agent's host.
This addresses bug FLUME-1782.
https://issues.apache.org/jira/browse/FLUME-1782
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 38f2205
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchDynamicSerializer.java aa7ad39
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer2.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchLogStashEventSerializer.java 3638368
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchDynamicSerializer.java 43a4b12
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchExtractIdFromHeaderIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java 9dff4b0
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10379/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Edward Sargisson <es...@pobox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/
-----------------------------------------------------------
(Updated April 19, 2013, 6:04 p.m.)
Review request for Flume and Hari Shreedharan.
Changes
-------
Updated patch to use new interface and keep compatibility with old interface.
Description
-------
This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
The previous code would use the current time and would do it in the timezone of the Flume agent's host.
This addresses bug FLUME-1782.
https://issues.apache.org/jira/browse/FLUME-1782
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 38f2205
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchDynamicSerializer.java aa7ad39
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer2.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchLogStashEventSerializer.java 3638368
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchDynamicSerializer.java 43a4b12
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java 9dff4b0
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10379/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Mike Percy <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/#review19419
-----------------------------------------------------------
Hmm, I'm not really OK with breaking API compatibility unless it's just impossible not to do it.
I think we should expose a new interface that serializers can implement and do RTTI at configure() time in the sink in order to determine whether the user has specified (via the Flume config file) a plugin that has implemented the old interface or the new interface for their serializer. Then we can put everything we want into the new interface without breaking people who have deployed Flume 1.3.1 in production.
Does that sound doable?
- Mike Percy
On April 18, 2013, 11:39 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10379/
> -----------------------------------------------------------
>
> (Updated April 18, 2013, 11:39 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
> The previous code would use the current time and would do it in the timezone of the Flume agent's host.
>
>
> This addresses bug FLUME-1782.
> https://issues.apache.org/jira/browse/FLUME-1782
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 38f2205
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchDynamicSerializer.java aa7ad39
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchLogStashEventSerializer.java 3638368
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchDynamicSerializer.java 43a4b12
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java 9dff4b0
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10379/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Edward Sargisson <es...@pobox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/
-----------------------------------------------------------
(Updated April 18, 2013, 11:39 p.m.)
Review request for Flume and Hari Shreedharan.
Changes
-------
Rework from Mike Percy's review:
* Event is no longer modified to carry the generated timestamp. This is a BREAKING change to the interface.
* Now use constants for timestamp values in the tests.
* Now use Charsets.UTF_8.
Description
-------
This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
The previous code would use the current time and would do it in the timezone of the Flume agent's host.
This addresses bug FLUME-1782.
https://issues.apache.org/jira/browse/FLUME-1782
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 38f2205
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchDynamicSerializer.java aa7ad39
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchEventSerializer.java dc6a093
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchLogStashEventSerializer.java 3638368
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchDynamicSerializer.java 43a4b12
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java 9dff4b0
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10379/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Edward Sargisson <es...@pobox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/
-----------------------------------------------------------
(Updated April 11, 2013, 7:03 p.m.)
Review request for Flume and Hari Shreedharan.
Changes
-------
* Now handle events with no header map.
Description
-------
This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
The previous code would use the current time and would do it in the timezone of the Flume agent's host.
This addresses bug FLUME-1782.
https://issues.apache.org/jira/browse/FLUME-1782
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10379/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Edward Sargisson <es...@pobox.com>.
> On April 11, 2013, 10:23 a.m., Israel Ekpo wrote:
> > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java, line 150
> > <https://reviews.apache.org/r/10379/diff/1/?file=279376#file279376line150>
> >
> > I think you should
> >
> > 1. Add a null check for the event headers before attempting to retrieve the timestamp header.
> >
> > 2. Provide a strategy for still returning an index using the current time in cases where no valid timestamp header in present in the event.
Hi Mr Ekpo,
The ensureHasTimestamp method already adds the current time to a new timestamp header if it's not there. However, you are correct that it did not handle having no headers at all. I have remedied that.
- Edward
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/#review18997
-----------------------------------------------------------
On April 9, 2013, 5:59 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10379/
> -----------------------------------------------------------
>
> (Updated April 9, 2013, 5:59 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
> The previous code would use the current time and would do it in the timezone of the Flume agent's host.
>
>
> This addresses bug FLUME-1782.
> https://issues.apache.org/jira/browse/FLUME-1782
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10379/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/#review18997
-----------------------------------------------------------
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java
<https://reviews.apache.org/r/10379/#comment39478>
I think you should
1. Add a null check for the event headers before attempting to retrieve the timestamp header.
2. Provide a strategy for still returning an index using the current time in cases where no valid timestamp header in present in the event.
- Israel Ekpo
On April 9, 2013, 5:59 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10379/
> -----------------------------------------------------------
>
> (Updated April 9, 2013, 5:59 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
> The previous code would use the current time and would do it in the timezone of the Flume agent's host.
>
>
> This addresses bug FLUME-1782.
> https://issues.apache.org/jira/browse/FLUME-1782
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10379/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1782 Elastic Search sink does not use UTC to
determine the correct index to write to
Posted by Edward Sargisson <es...@pobox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10379/
-----------------------------------------------------------
(Updated April 9, 2013, 5:59 p.m.)
Review request for Flume and Hari Shreedharan.
Description
-------
This change gets the timestamp from the event and uses it (in UTC) to determine the name of the index to write to. This is required to match the behaviour of Logstash so that Kibana can find the log events.
The previous code would use the current time and would do it in the timezone of the Flume agent's host.
This addresses bug FLUME-1782.
https://issues.apache.org/jira/browse/FLUME-1782
Diffs
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10379/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson