[GitHub] flink issue #5058: [FLINK-5465] [streaming] Wait for pending timer threads t...

2017-11-24 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5058 I was thinking about where to put those options and after all it was a choice between fragmentation and inconsistency. I considered `TaskManagerOptions` but it sounds like a wrong place, if

[GitHub] flink issue #5058: [FLINK-5465] [streaming] Wait for pending timer threads t...

2017-11-24 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5058 I would like to make a few comments for followup: - I think `TimerServiceOptions` should not be an own class, we are getting a crazy fragmentation of options into classes (that have

[GitHub] flink issue #5058: [FLINK-5465] [streaming] Wait for pending timer threads t...

2017-11-23 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5058 Thanks for the reviews @aljoscha and @StephanEwen. Will merge this now. ---

[GitHub] flink issue #5058: [FLINK-5465] [streaming] Wait for pending timer threads t...

2017-11-23 Thread aljoscha
Github user aljoscha commented on the issue: https://github.com/apache/flink/pull/5058 +1 This LGTM now. ---

[GitHub] flink issue #5058: [FLINK-5465] [streaming] Wait for pending timer threads t...

2017-11-23 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5058 Implemented the configurable timeout, so that we can chose between the two options. ---

[GitHub] flink issue #5058: [FLINK-5465] [streaming] Wait for pending timer threads t...

2017-11-23 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5058 So which way can we agree upon? I feel like this is a setting that is very specific and hard to understand for the user. On the other hand, it is nice if we can adjust it by configuration in

[GitHub] flink issue #5058: [FLINK-5465] [streaming] Wait for pending timer threads t...

2017-11-23 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5058 I am actually leaning towards not having a grace period, too. Reason is that actually there are more than one method in the same code path that could also (theoretically) block forever.

[GitHub] flink issue #5058: [FLINK-5465] [streaming] Wait for pending timer threads t...

2017-11-23 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5058 Looks good all in all. I am torn between adding a configuration setting for the timer grace period and not having one. Seems like a setting that is unlikely to ever be used and just

[GitHub] flink issue #5058: [FLINK-5465] [streaming] Wait for pending timer threads t...

2017-11-23 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5058 CC @aljoscha ---