[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-25 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3035 I pushed a fix for that to master in e7cda75b8594417559d6aac6229b5893f5459f0f --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-25 Thread BrainLogic
Github user BrainLogic commented on the issue: https://github.com/apache/flink/pull/3035 I caught yours idea and together with the argument which I mentioned above - users will not extensively use 8 version of kafka connector, I agree with this proposal. Let me finish this jira,

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-25 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3035 I this case, the exception would not be logged, true. It is a very rare corner case that should not affect correctness, and not really distinguishable from the case where an exception is

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-25 Thread BrainLogic
Github user BrainLogic commented on the issue: https://github.com/apache/flink/pull/3035 Distributed systems and multithreading environments make us think in term of logical clock, like Lamport clock, step by step: Thread1 - fetcher is running `running = true` Thread2 performs

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-25 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3035 Quick question: I am not sure if this scenario can happen like that: > But in this approach an unlikely bug will be still occurred: zkHandler.prepareAndCommitOffsets(offsets); throws

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-24 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3035 @BrainLogic I'll incorporate your comment into a followup commit... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-24 Thread BrainLogic
Github user BrainLogic commented on the issue: https://github.com/apache/flink/pull/3035 Thanks for help and explanation. But in this approach an unlikely bug will be still occurred: `zkHandler.prepareAndCommitOffsets(offsets);` throws important exception when running is true

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-23 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3035 Thank you for picking this up @StephanEwen! I've taken a look at your approach in the local branch, +1 to the approach. --- If your project is set up for it, you can reply to this email and have

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-23 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3035 Bit of background how the error happens: - The test throws a `SuccessException` - While being in the finally clause and shutting down the CluratorClient, the containing `Task` has not

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-23 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3035 I would like to pick this fix up. The exception has still occurred a few times for me in the past, and I prefer the above outlined solution, because it adds less locking on

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-23 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3035 I think we can take a very simple approach here. Many other parts of the code follow the approach to tolerate exceptions thrown during cancellation, or during asynchronous calls on closed

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-23 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3035 Hi @BrainLogic, we're still seeing this exception in tests sometimes, and it'll be great to have this fixed soon. Please let us know on how you'd like to proceed with the contribution,

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-06 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3035 Dropping Java 7 has not happened and there is not yet consensus in the community, so it probably will not happen in the next weeks. --- If your project is set up for it, you can reply to this

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-05 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3035 Regarding some of your notes: - The exposure of the checkpoint lock through `SourceContext` is meant for sources to atomically update their state (e.x. Kafka offsets) with record emitting,

[GitHub] flink issue #3035: [ FLINK-4905] Kafka test instability IllegalStateExceptio...

2017-01-05 Thread tzulitai
Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3035 Thank you for working on this @BrainLogic! First, regarding the approach proposed here: The approach should be able to fix the `IllegalStateException` we're encountering. However, I