[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-07 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1045 I assume you are talking to @nickwallen there @mmiklavc ? ---

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-07 Thread mmiklavc
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/1045 I didn't know you were still actively reviewing. The last comment I received besides a +1 was "that looks great, thanks." We do have unit and integration tests around all of this in addition to

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-07 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1045 My comment was just about calling out a possible need for more shutdown orchestration. I am not reviewing. ---

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-07 Thread nickwallen
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/1045 I was still going through it. Not sure where @ottobackwards landed on this. Besides the open TODO comment, I am not sure how much testing we did around the Profiler or testing

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-07 Thread mmiklavc
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/1045 @nickwallen The PR has been up for 7 days, I addressed all community comments days ago, and no comments appeared to be dissenting. Was there a concern or issue you had before this was merged?

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-07 Thread nickwallen
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/1045 @mmiklavc FYI You've got a TODO comment in `ConfiguredIndexingBolt.java` that seems like something you wanted to address before merging. Usually good to at least give a chance for all

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-06 Thread cestella
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1045 +1 by inspection. I ran this up in full-dev and data flowed through just fine. ---

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-06 Thread mmiklavc
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/1045 Went through a few variations of testing enrichments using the new KafkaWriter bulk message writer implementation with @anandsubbu and we are seeing results close to the numbers we see in master

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-04 Thread mmiklavc
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/1045 @ottobackwards I believe this should already be handled by the acking strategy. Anything not acked will be replayed since we're leveraging at least once message processing semantics. ---

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-02 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1045 Maybe we need some kind of orchestration service that you use to shutdown metron without losing things in the pipeline already ---

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-01 Thread nickwallen
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/1045 > This might actually explain why some of the topologies just won't die sometimes. The more I think about it, this is very likely what is happening in the current code base when

[GitHub] metron issue #1045: METRON-1594: KafkaWriter is asynchronous and may lose da...

2018-06-01 Thread nickwallen
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/1045 I noticed that we should probably use a timeout when we call `KafkaProducer.close`. If we get a huge backlog of messages, it will just block until the backlog is cleared. We can get in